From f33fc2528ddeacdb6c924fc01bb42f126c92c1af Mon Sep 17 00:00:00 2001 From: Neek Sandhu Date: Thu, 13 Jul 2023 12:58:39 -0700 Subject: [PATCH 1/3] [LIBWEB-1353] Fix cookie write error --- .changeset/pink-dancers-exist.md | 5 ++++ .../src/core/user/__tests__/index.test.ts | 24 +++++++++++++++---- .../src/core/user/__tests__/storage.test.ts | 15 ++++++++++++ packages/browser/src/core/user/index.ts | 15 ++++++------ .../segmentio/__tests__/normalize.test.ts | 16 ++++++++++++- 5 files changed, 61 insertions(+), 14 deletions(-) create mode 100644 .changeset/pink-dancers-exist.md create mode 100644 packages/browser/src/core/user/__tests__/storage.test.ts diff --git a/.changeset/pink-dancers-exist.md b/.changeset/pink-dancers-exist.md new file mode 100644 index 000000000..b6768098c --- /dev/null +++ b/.changeset/pink-dancers-exist.md @@ -0,0 +1,5 @@ +--- +'@segment/analytics-next': minor +--- + +[LIBWEB-1353] Fix cookie write error diff --git a/packages/browser/src/core/user/__tests__/index.test.ts b/packages/browser/src/core/user/__tests__/index.test.ts index 7f78deab7..968571f99 100644 --- a/packages/browser/src/core/user/__tests__/index.test.ts +++ b/packages/browser/src/core/user/__tests__/index.test.ts @@ -19,6 +19,20 @@ function clear(): void { localStorage.clear() } +/** + * Filters out the calls made for probing cookie availability + */ +const ignoreProbeCookieWrites = ( + fn: jest.SpyInstance< + string | undefined, + [ + name: string, + value: string | object, + options?: jar.CookieAttributes | undefined + ] + > +) => fn.mock.calls.filter((c) => c[0] !== 'ajs_cookies') + let store: LocalStorage beforeEach(function () { store = new LocalStorage() @@ -58,7 +72,7 @@ describe('user', () => { assert(user.anonymousId()?.length === 36) expect(jar.get('ajs_anonymous_id')).toBeUndefined() expect(localStorage.getItem('ajs_anonymous_id')).toBeNull() - expect(setCookieSpy.mock.calls.length).toBe(0) + expect(ignoreProbeCookieWrites(setCookieSpy).length).toBe(0) }) it('should not overwrite anonymous id', () => { @@ -218,7 +232,7 @@ describe('user', () => { user.id('foo') assert(user.anonymousId() === prev) - expect(setCookieSpy.mock.calls.length).toBe(0) + expect(ignoreProbeCookieWrites(setCookieSpy).length).toBe(0) }) it('should reset anonymousId if the user id changed', () => { @@ -227,7 +241,7 @@ describe('user', () => { user.id('baz') assert(user.anonymousId() !== prev) assert(user.anonymousId()?.length === 36) - expect(setCookieSpy.mock.calls.length).toBe(0) + expect(ignoreProbeCookieWrites(setCookieSpy).length).toBe(0) }) it('should not reset anonymousId if the user id changed to null', () => { @@ -236,7 +250,7 @@ describe('user', () => { user.id(null) assert(user.anonymousId() === prev) assert(user.anonymousId()?.length === 36) - expect(setCookieSpy.mock.calls.length).toBe(0) + expect(ignoreProbeCookieWrites(setCookieSpy).length).toBe(0) }) }) @@ -795,7 +809,7 @@ describe('group', () => { expect(group.id()).toBe('gid') expect(jar.get('ajs_group_id')).toBeFalsy() expect(store.get('ajs_group_id')).toBeFalsy() - expect(setCookieSpy.mock.calls.length).toBe(0) + expect(ignoreProbeCookieWrites(setCookieSpy).length).toBe(0) }) it('behaves the same as user', () => { diff --git a/packages/browser/src/core/user/__tests__/storage.test.ts b/packages/browser/src/core/user/__tests__/storage.test.ts new file mode 100644 index 000000000..30100a30c --- /dev/null +++ b/packages/browser/src/core/user/__tests__/storage.test.ts @@ -0,0 +1,15 @@ +import { Cookie } from '..' + +describe('Cookie storage', () => { + it('should report cookie storage available when cookies are accessible', () => { + expect(Cookie.available()).toBe(true) + }) + + it('should report cookie storage unavailable when cookies are not accessible', () => { + ;(document as any).__defineGetter__('cookie', function () { + return '' + }) + + expect(Cookie.available()).toBe(false) + }) +}) diff --git a/packages/browser/src/core/user/index.ts b/packages/browser/src/core/user/index.ts index a778ecd21..ff888ca60 100644 --- a/packages/browser/src/core/user/index.ts +++ b/packages/browser/src/core/user/index.ts @@ -62,15 +62,14 @@ const ONE_YEAR = 365 export class Cookie extends Store { static available(): boolean { - let cookieEnabled = window.navigator.cookieEnabled - - if (!cookieEnabled) { - jar.set('ajs:cookies', 'test') - cookieEnabled = document.cookie.includes('ajs:cookies') - jar.remove('ajs:cookies') + try { + jar.set('ajs_cookies', 'test') + const cookieEnabled = document.cookie.includes('ajs_cookies') + jar.remove('ajs_cookies') + return cookieEnabled + } catch (error) { + return false } - - return cookieEnabled } static get defaults(): CookieOptions { diff --git a/packages/browser/src/plugins/segmentio/__tests__/normalize.test.ts b/packages/browser/src/plugins/segmentio/__tests__/normalize.test.ts index a91a43e5f..8ddc990ba 100644 --- a/packages/browser/src/plugins/segmentio/__tests__/normalize.test.ts +++ b/packages/browser/src/plugins/segmentio/__tests__/normalize.test.ts @@ -7,6 +7,20 @@ import { SegmentEvent } from '../../../core/events' import { JSDOM } from 'jsdom' import { version } from '../../../generated/version' +/** + * Filters out the calls made for probing cookie availability + */ +const ignoreProbeCookieWrites = ( + fn: jest.SpyInstance< + string | undefined, + [ + name: string, + value: string | object, + options?: cookie.CookieAttributes | undefined + ] + > +) => fn.mock.calls.filter((c) => c[0] !== 'ajs_cookies') + describe('before loading', () => { let jsdom: JSDOM @@ -317,7 +331,7 @@ describe('before loading', () => { expect(object.context.referrer.id).toEqual('medium') assert(object.context.referrer.type === 'millennial-media') expect(cookie.get('s:context.referrer')).toBeUndefined() - expect(setCookieSpy).not.toHaveBeenCalled() + expect(ignoreProbeCookieWrites(setCookieSpy).length).toBe(0) }) it('should add .referrer.id and .referrer.type from cookie', () => { From 28546037d486d474c10181b1485cd6d39b5a4f19 Mon Sep 17 00:00:00 2001 From: Neek Sandhu Date: Fri, 14 Jul 2023 11:08:11 -0700 Subject: [PATCH 2/3] refactor --- packages/browser/src/core/user/__tests__/index.test.ts | 2 +- packages/browser/src/core/user/index.ts | 7 ++++--- .../src/plugins/segmentio/__tests__/normalize.test.ts | 2 +- 3 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/browser/src/core/user/__tests__/index.test.ts b/packages/browser/src/core/user/__tests__/index.test.ts index 968571f99..287d2e0dc 100644 --- a/packages/browser/src/core/user/__tests__/index.test.ts +++ b/packages/browser/src/core/user/__tests__/index.test.ts @@ -31,7 +31,7 @@ const ignoreProbeCookieWrites = ( options?: jar.CookieAttributes | undefined ] > -) => fn.mock.calls.filter((c) => c[0] !== 'ajs_cookies') +) => fn.mock.calls.filter((c) => c[0] !== 'ajs_cookies_check') let store: LocalStorage beforeEach(function () { diff --git a/packages/browser/src/core/user/index.ts b/packages/browser/src/core/user/index.ts index ff888ca60..5949d5fad 100644 --- a/packages/browser/src/core/user/index.ts +++ b/packages/browser/src/core/user/index.ts @@ -63,9 +63,10 @@ const ONE_YEAR = 365 export class Cookie extends Store { static available(): boolean { try { - jar.set('ajs_cookies', 'test') - const cookieEnabled = document.cookie.includes('ajs_cookies') - jar.remove('ajs_cookies') + const PROBE_COOKIE = 'ajs_cookies_check' + jar.set(PROBE_COOKIE, 'test') + const cookieEnabled = document.cookie.includes(PROBE_COOKIE) + jar.remove(PROBE_COOKIE) return cookieEnabled } catch (error) { return false diff --git a/packages/browser/src/plugins/segmentio/__tests__/normalize.test.ts b/packages/browser/src/plugins/segmentio/__tests__/normalize.test.ts index 8ddc990ba..cbbd9f5b9 100644 --- a/packages/browser/src/plugins/segmentio/__tests__/normalize.test.ts +++ b/packages/browser/src/plugins/segmentio/__tests__/normalize.test.ts @@ -19,7 +19,7 @@ const ignoreProbeCookieWrites = ( options?: cookie.CookieAttributes | undefined ] > -) => fn.mock.calls.filter((c) => c[0] !== 'ajs_cookies') +) => fn.mock.calls.filter((c) => c[0] !== 'ajs_cookies_check') describe('before loading', () => { let jsdom: JSDOM From 807d8966ba6b7c0468f479168f5a482f4b6c6dbb Mon Sep 17 00:00:00 2001 From: Neek Sandhu Date: Tue, 18 Jul 2023 11:41:29 -0700 Subject: [PATCH 3/3] Update .changeset/pink-dancers-exist.md --- .changeset/pink-dancers-exist.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/pink-dancers-exist.md b/.changeset/pink-dancers-exist.md index b6768098c..b9b3e0a93 100644 --- a/.changeset/pink-dancers-exist.md +++ b/.changeset/pink-dancers-exist.md @@ -1,5 +1,5 @@ --- -'@segment/analytics-next': minor +'@segment/analytics-next': patch --- [LIBWEB-1353] Fix cookie write error