fix: Sync consent with persistence immediately (#2082)

* Sync consent with persistence immediately

* Sync consent with persistence immediately

* Move test to persistence test file

* Bump terser

* Add another test

* Reset between tests

Author: robbie-c

packages/browser/src/__tests__/posthog-persistence.test.ts

@@ -2,10 +2,17 @@
 import { PostHogPersistence } from '../posthog-persistence'
 import { INITIAL_PERSON_INFO, SESSION_ID, USER_STATE } from '../constants'
 import { PostHogConfig } from '../types'
-import Mock = jest.Mock
 import { PostHog } from '../posthog-core'
 import { window } from '../utils/globals'
 import { uuidv7 } from '../uuidv7'
+import {
+    localPlusCookieStore,
+    resetLocalStorageSupported,
+    resetSessionStorageSupported,
+    sessionStore,
+} from '../storage'
+import { defaultPostHog } from './helpers/posthog-instance'
+import Mock = jest.Mock
 
 let referrer = '' // No referrer by default
 Object.defineProperty(document, 'referrer', { get: () => referrer })
@@ -267,3 +274,56 @@ describe('persistence', () => {
         })
     })
 })
+
+describe('posthog instance persistence', () => {
+    beforeEach(() => {
+        resetSessionStorageSupported()
+        resetLocalStorageSupported()
+    })
+    it('should not write to storage if opt_out_persistence_by_default and opt_out_capturing_by_default is true', () => {
+        const sessionSpy = jest.spyOn(sessionStore, '_set')
+        const localPlusCookieSpy = jest.spyOn(localPlusCookieStore, '_set')
+
+        // init posthog while opting out
+        defaultPostHog().init(
+            uuidv7(),
+            {
+                opt_out_persistence_by_default: true,
+                opt_out_capturing_by_default: true,
+                persistence: 'localStorage+cookie',
+            },
+            uuidv7()
+        )
+
+        // we do one call to check if session storage is supported, but don't actually store anything
+        // the important thing is that we don't store the session id or window id, etc. This test was added alongside
+        // a fix which prevented this
+        const sessionCalls = sessionSpy.mock.calls.filter(([key]) => key !== '__support__')
+        const localPlusCookieCalls = localPlusCookieSpy.mock.calls.filter(([key]) => key !== '__support__')
+
+        expect(sessionCalls).toEqual([])
+        expect(localPlusCookieCalls).toEqual([])
+    })
+
+    it('should write to storage if opt_out_persistence_by_default and opt_out_capturing_by_default is false', () => {
+        const sessionSpy = jest.spyOn(sessionStore, '_set')
+        const localPlusCookieSpy = jest.spyOn(localPlusCookieStore, '_set')
+
+        // init posthog while opting out
+        defaultPostHog().init(
+            uuidv7(),
+            {
+                opt_out_persistence_by_default: false,
+                opt_out_capturing_by_default: false,
+                persistence: 'localStorage+cookie',
+            },
+            uuidv7()
+        )
+
+        const sessionCalls = sessionSpy.mock.calls.filter(([key]) => key !== '__support__')
+        const localPlusCookieCalls = localPlusCookieSpy.mock.calls.filter(([key]) => key !== '__support__')
+
+        expect(sessionCalls.length).toBeGreaterThan(0)
+        expect(localPlusCookieCalls.length).toBeGreaterThan(0)
+    })
+})

packages/browser/src/posthog-core.ts

@@ -486,11 +486,13 @@ export class PostHog {
 
         this.compression = config.disable_compression ? undefined : Compression.GZipJS
 
-        this.persistence = new PostHogPersistence(this.config)
+        const persistenceDisabled = this._is_persistence_disabled()
+
+        this.persistence = new PostHogPersistence(this.config, persistenceDisabled)
         this.sessionPersistence =
             this.config.persistence === 'sessionStorage' || this.config.persistence === 'memory'
                 ? this.persistence
-                : new PostHogPersistence({ ...this.config, persistence: 'sessionStorage' })
+                : new PostHogPersistence({ ...this.config, persistence: 'sessionStorage' }, persistenceDisabled)
 
         // should I store the initial person profiles config in persistence?
         const initialPersistenceProps = { ...this.persistence.props }
@@ -555,8 +557,6 @@ export class PostHog {
             })
         }
 
-        this._sync_opt_out_with_persistence()
-
         // isUndefined doesn't provide typehint here so wouldn't reduce bundle as we'd need to assign
         // eslint-disable-next-line posthog-js/no-direct-undefined-check
         if (config.bootstrap?.distinctID !== undefined) {
@@ -2100,11 +2100,12 @@ export class PostHog {
         if (isObject(config)) {
             extend(this.config, configRenames(config))
 
-            this.persistence?.update_config(this.config, oldConfig)
+            const isPersistenceDisabled = this._is_persistence_disabled()
+            this.persistence?.update_config(this.config, oldConfig, isPersistenceDisabled)
             this.sessionPersistence =
                 this.config.persistence === 'sessionStorage' || this.config.persistence === 'memory'
                     ? this.persistence
-                    : new PostHogPersistence({ ...this.config, persistence: 'sessionStorage' })
+                    : new PostHogPersistence({ ...this.config, persistence: 'sessionStorage' }, isPersistenceDisabled)
 
             if (localStore._is_supported() && localStore._get('ph_debug') === 'true') {
                 this.config.debug = true
@@ -2326,19 +2327,24 @@ export class PostHog {
         return true
     }
 
-    private _sync_opt_out_with_persistence(): void {
+    private _is_persistence_disabled(): boolean {
         const isOptedOut = this.consent.isOptedOut()
         const defaultPersistenceDisabled = this.config.opt_out_persistence_by_default
 
         // TRICKY: We want a deterministic state for persistence so that a new pageload has the same persistence
-        const persistenceDisabled = this.config.disable_persistence || (isOptedOut && !!defaultPersistenceDisabled)
+        return this.config.disable_persistence || (isOptedOut && !!defaultPersistenceDisabled)
+    }
+
+    private _sync_opt_out_with_persistence(): boolean {
+        const persistenceDisabled = this._is_persistence_disabled()
 
         if (this.persistence?._disabled !== persistenceDisabled) {
             this.persistence?.set_disabled(persistenceDisabled)
         }
         if (this.sessionPersistence?._disabled !== persistenceDisabled) {
             this.sessionPersistence?.set_disabled(persistenceDisabled)
         }
+        return persistenceDisabled
     }
 
     /**

packages/browser/src/posthog-persistence.ts

@@ -60,7 +60,11 @@ export class PostHogPersistence {
     private _default_expiry: number | undefined
     private _cross_subdomain: boolean | undefined
 
-    constructor(config: PostHogConfig) {
+    /**
+     * @param {PostHogConfig} config initial PostHog configuration
+     * @param {boolean=} isDisabled should persistence be disabled (e.g. because of consent management)
+     */
+    constructor(config: PostHogConfig, isDisabled?: boolean) {
         this._config = config
         this.props = {}
         this._campaign_params_saved = false
@@ -70,10 +74,14 @@ export class PostHogPersistence {
         if (config.debug) {
             logger.info('Persistence loaded', config['persistence'], { ...this.props })
         }
-        this.update_config(config, config)
+        this.update_config(config, config, isDisabled)
         this.save()
     }
 
+    public isDisabled(): boolean {
+        return !!this._disabled
+    }
+
     private _buildStorage(config: PostHogConfig) {
         if (
             CASE_INSENSITIVE_PERSISTENCE_TYPES.indexOf(
@@ -308,9 +316,9 @@ export class PostHogPersistence {
         return props
     }
 
-    update_config(config: PostHogConfig, oldConfig: PostHogConfig): void {
+    update_config(config: PostHogConfig, oldConfig: PostHogConfig, isDisabled?: boolean): void {
         this._default_expiry = this._expire_days = config['cookie_expiration']
-        this.set_disabled(config['disable_persistence'])
+        this.set_disabled(config['disable_persistence'] || !!isDisabled)
         this.set_cross_subdomain(config['cross_subdomain_cookie'])
         this.set_secure(config['secure_cookie'])
 

packages/browser/src/storage.ts

@@ -179,6 +179,9 @@ export const cookieStore: PersistentStore = {
 }
 
 let _localStorage_supported: boolean | null = null
+export const resetLocalStorageSupported = () => {
+    _localStorage_supported = null
+}
 
 export const localStore: PersistentStore = {
     _is_supported: function () {

packages/browser/terser-mangled-names.json

@@ -135,6 +135,7 @@
         "_isSurveyConditionMatched",
         "_isSurveyFeatureFlagEnabled",
         "_isSurveysEnabled",
+        "_is_persistence_disabled",
         "_is_supported",
         "_lastActivityTimestamp",
         "_lastHref",