feat: Swap to localstorage+cookie as default (#927)

PostHog · Dec 8, 2023 · 8327864 · 8327864
1 parent 3eea52a
commit 8327864
Show file tree

Hide file tree

Showing 4 changed files with 25 additions and 9 deletions.
diff --git a/src/__tests__/posthog-persistence.test.ts b/src/__tests__/posthog-persistence.test.ts
@@ -18,7 +18,7 @@ describe('persistence', () => {
     let library: PostHogPersistence
 
     afterEach(() => {
-        library.clear()
+        library?.clear()
         document.cookie = ''
         referrer = ''
     })
@@ -114,13 +114,13 @@ describe('persistence', () => {
         it('should migrate data from cookies to localStorage', () => {
             const lib = new PostHogPersistence(makePostHogConfig('bla', 'cookie'))
             lib.register_once({ distinct_id: 'testy', test_prop: 'test_value' }, undefined, undefined)
-            expect(document.cookie).toContain(
-                'ph__posthog=%7B%22distinct_id%22%3A%22testy%22%2C%22test_prop%22%3A%22test_value%22%7D'
+            expect(document.cookie).toEqual(
+                '; ph__posthog=%7B%22distinct_id%22%3A%22testy%22%2C%22test_prop%22%3A%22test_value%22%7D'
             )
             const lib2 = new PostHogPersistence(makePostHogConfig('bla', 'localStorage+cookie'))
-            expect(document.cookie).toContain('ph__posthog=%7B%22distinct_id%22%3A%22testy%22%7D')
+            expect(document.cookie).toEqual('; ph__posthog=%7B%22distinct_id%22%3A%22testy%22%7D')
             lib2.register({ test_prop2: 'test_val', distinct_id: 'test2' })
-            expect(document.cookie).toContain('ph__posthog=%7B%22distinct_id%22%3A%22test2%22%7D')
+            expect(document.cookie).toEqual('; ph__posthog=%7B%22distinct_id%22%3A%22test2%22%7D')
             expect(lib2.props).toEqual({ distinct_id: 'test2', test_prop: 'test_value', test_prop2: 'test_val' })
             lib2.remove()
             expect(localStorage.getItem('ph__posthog')).toEqual(null)
@@ -139,6 +139,7 @@ describe('persistence', () => {
                     distinct_id: 'test',
                 })}`
             )
+            expect(document.cookie).not.toContain('test_prop')
 
             lib.register({ otherProp: 'prop' })
             expect(document.cookie).toContain(

diff --git a/src/posthog-core.ts b/src/posthog-core.ts
@@ -108,7 +108,7 @@ export const defaultConfig = (): PostHogConfig => ({
     autocapture: true,
     rageclick: true,
     cross_subdomain_cookie: isCrossDomainCookie(document?.location),
-    persistence: 'cookie',
+    persistence: 'localStorage+cookie', // up to 1.92.0 this was 'cookie'. It's easy to migrate as 'localStorage+cookie' will migrate data from cookie storage
     persistence_name: '',
     cookie_name: '',
     loaded: __NOOP,

diff --git a/src/posthog-persistence.ts b/src/posthog-persistence.ts
@@ -64,8 +64,10 @@ export class PostHogPersistence {
                 config['persistence'].toLowerCase() as Lowercase<PostHogConfig['persistence']>
             ) === -1
         ) {
-            logger.critical('Unknown persistence type ' + config['persistence'] + '; falling back to cookie')
-            config['persistence'] = 'cookie'
+            logger.critical(
+                'Unknown persistence type ' + config['persistence'] + '; falling back to localStorage+cookie'
+            )
+            config['persistence'] = 'localStorage+cookie'
         }
         // We handle storage type in a case-insensitive way for backwards compatibility
         const storage_type = config['persistence'].toLowerCase() as Lowercase<PostHogConfig['persistence']>
@@ -77,8 +79,15 @@ export class PostHogPersistence {
             this.storage = sessionStore
         } else if (storage_type === 'memory') {
             this.storage = memoryStore
-        } else {
+        } else if (storage_type === 'cookie') {
             this.storage = cookieStore
+        } else {
+            // selected storage type wasn't supported, fallback to 'localstorage+cookie' if possible
+            if (localPlusCookieStore.is_supported()) {
+                this.storage = localPlusCookieStore
+            } else {
+                this.storage = cookieStore
+            }
         }
 
         this.user_state = 'anonymous'

diff --git a/src/storage.ts b/src/storage.ts
@@ -140,6 +140,12 @@ export const cookieStore: PersistentStore = {
                 '; SameSite=Lax; path=/' +
                 cdomain +
                 secure
+
+            // 4096 bytes is the size at which some browsers (e.g. firefox) will not store a cookie, warn slightly before that
+            if (new_cookie_val.length > 4096 * 0.9) {
+                logger.warn('cookieStore warning: large cookie, len=' + new_cookie_val.length)
+            }
+
             document.cookie = new_cookie_val
             return new_cookie_val
         } catch (err) {