From ab469dc9a0307bcadaef911fd60900c9e1a8319c Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 3 Aug 2023 15:58:58 +0200 Subject: [PATCH 01/16] Handle init values better when unintialised --- src/extensions/sessionrecording.ts | 33 +++++++++---- src/posthog-core.ts | 76 +++++++++++++++++------------- src/posthog-featureflags.ts | 15 +++--- src/posthog-surveys.ts | 2 +- 4 files changed, 77 insertions(+), 49 deletions(-) diff --git a/src/extensions/sessionrecording.ts b/src/extensions/sessionrecording.ts index ba7f6cd48..574b10049 100644 --- a/src/extensions/sessionrecording.ts +++ b/src/extensions/sessionrecording.ts @@ -101,6 +101,15 @@ export class SessionRecording { }) } + private get sessionManager() { + if (!this.instance.sessionManager) { + logger.error('Session recording started without valid sessionManager') + return + } + + return this.instance.sessionManager + } + startRecordingIfEnabled() { if (this.isRecordingEnabled()) { this.startCaptureAndTrySendingQueuedSnapshots() @@ -185,14 +194,17 @@ export class SessionRecording { } private _startCapture() { - // According to the rrweb docs, rrweb is not supported on IE11 and below: - // "rrweb does not support IE11 and below because it uses the MutationObserver API which was supported by these browsers." - // https://github.com/rrweb-io/rrweb/blob/master/guide.md#compatibility-note - // - // However, MutationObserver does exist on IE11, it just doesn't work well and does not detect all changes. - // Instead, when we load "recorder.js", the first JS error is about "Object.assign" being undefined. - // Thus instead of MutationObserver, we look for this function and block recording if it's undefined. + if (!this.sessionManager) { + return + } if (typeof Object.assign === 'undefined') { + // According to the rrweb docs, rrweb is not supported on IE11 and below: + // "rrweb does not support IE11 and below because it uses the MutationObserver API which was supported by these browsers." + // https://github.com/rrweb-io/rrweb/blob/master/guide.md#compatibility-note + // + // However, MutationObserver does exist on IE11, it just doesn't work well and does not detect all changes. + // Instead, when we load "recorder.js", the first JS error is about "Object.assign" being undefined. + // Thus instead of MutationObserver, we look for this function and block recording if it's undefined. return } @@ -203,7 +215,7 @@ export class SessionRecording { this.captureStarted = true // We want to ensure the sessionManager is reset if necessary on load of the recorder - this.instance.sessionManager.checkAndGetSessionAndWindowId() + this.sessionManager.checkAndGetSessionAndWindowId() const recorderJS = this.getRecordingVersion() === 'v2' ? 'recorder-v2.js' : 'recorder.js' @@ -231,6 +243,9 @@ export class SessionRecording { } private _updateWindowAndSessionIds(event: eventWithTime) { + if (!this.sessionManager) { + return + } // Some recording events are triggered by non-user events (e.g. "X minutes ago" text updating on the screen). // We don't want to extend the session or trigger a new session in these cases. These events are designated by event // type -> incremental update, and source -> mutation. @@ -258,7 +273,7 @@ export class SessionRecording { } // We only want to extend the session if it is an interactive event. - const { windowId, sessionId } = this.instance.sessionManager.checkAndGetSessionAndWindowId( + const { windowId, sessionId } = this.sessionManager.checkAndGetSessionAndWindowId( !isUserInteraction, event.timestamp ) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 1d3773b18..348f5d036 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -260,10 +260,7 @@ export class PostHog { __loaded_recorder_version: 'v1' | 'v2' | undefined // flag that keeps track of which version of recorder is loaded config: PostHogConfig - persistence: PostHogPersistence rateLimiter: RateLimiter - sessionPersistence: PostHogPersistence - sessionManager: SessionIdManager pageViewIdManager: PageViewIdManager featureFlags: PostHogFeatureFlags surveys: PostHogSurveys @@ -272,8 +269,12 @@ export class PostHog { webPerformance: WebPerformanceObserver | undefined exceptionAutocapture: ExceptionObserver | undefined - _requestQueue: RequestQueue - _retryQueue: RetryQueue + // These are instance-specific state created after initialisation + persistence?: PostHogPersistence + sessionPersistence?: PostHogPersistence + sessionManager?: SessionIdManager + _requestQueue?: RequestQueue + _retryQueue?: RetryQueue _triggered_notifs: any compression: Partial> @@ -314,13 +315,6 @@ export class PostHog { this.surveys = new PostHogSurveys(this) this.rateLimiter = new RateLimiter() - // these are created in _init() after we have the config - this._requestQueue = undefined as any - this._retryQueue = undefined as any - this.persistence = undefined as any - this.sessionPersistence = undefined as any - this.sessionManager = undefined as any - // NOTE: See the property definition for deprecation notice this.people = { set: (prop: string | Properties, to?: string, callback?: RequestCallback) => { @@ -562,7 +556,7 @@ export class PostHog { _start_queue_if_opted_in(): void { if (!this.has_opted_out_capturing()) { if (this.get_config('request_batching')) { - this._requestQueue.poll() + this._requestQueue?.poll() } } } @@ -625,8 +619,8 @@ export class PostHog { this.capture('$pageleave') } - this._requestQueue.unload() - this._retryQueue.unload() + this._requestQueue?.unload() + this._retryQueue?.unload() } _handle_queued_event(url: string, data: Record, options?: XHROptions): void { @@ -645,6 +639,9 @@ export class PostHog { } _send_request(url: string, data: Record, options: CaptureOptions, callback?: RequestCallback): void { + if (!this.__loaded || !this._retryQueue) { + return + } if (this.rateLimiter.isRateLimited(options._batchKey)) { if (this.get_config('debug')) { console.warn('[PostHog SendRequest] is quota limited. Dropping request.') @@ -837,7 +834,7 @@ export class PostHog { ): CaptureResult | void { // While developing, a developer might purposefully _not_ call init(), // in this case, we would like capture to be a noop. - if (!this.__loaded) { + if (!this.__loaded || !this.sessionPersistence || !this._requestQueue) { return } @@ -922,6 +919,10 @@ export class PostHog { } _calculate_event_properties(event_name: string, event_properties: Properties): Properties { + if (!this.persistence || !this.sessionPersistence) { + return event_properties + } + // set defaults const start_timestamp = this.persistence.remove_event_timer(event_name) let properties = { ...event_properties } @@ -1019,7 +1020,7 @@ export class PostHog { * @param {Number} [days] How many days since the user's last visit to store the super properties */ register(properties: Properties, days?: number): void { - this.persistence.register(properties, days) + this.persistence?.register(properties, days) } /** @@ -1046,7 +1047,7 @@ export class PostHog { * @param {Number} [days] How many days since the users last visit to store the super properties */ register_once(properties: Properties, default_value?: Property, days?: number): void { - this.persistence.register_once(properties, default_value, days) + this.persistence?.register_once(properties, default_value, days) } /** @@ -1073,7 +1074,7 @@ export class PostHog { * @param {Object} properties An associative array of properties to store about the user */ register_for_session(properties: Properties): void { - this.sessionPersistence.register(properties) + this.sessionPersistence?.register(properties) } /** @@ -1082,7 +1083,7 @@ export class PostHog { * @param {String} property The name of the super property to remove */ unregister(property: string): void { - this.persistence.unregister(property) + this.persistence?.unregister(property) } /** @@ -1091,7 +1092,7 @@ export class PostHog { * @param {String} property The name of the session super property to remove */ unregister_for_session(property: string): void { - this.sessionPersistence.unregister(property) + this.sessionPersistence?.unregister(property) } _register_single(prop: string, value: Property) { @@ -1191,7 +1192,7 @@ export class PostHog { * @returns {Function} A function that can be called to unsubscribe the listener. E.g. Used by useEffect when the component unmounts. */ onSessionId(callback: SessionIdChangedCallback): () => void { - return this.sessionManager.onSessionId(callback) + return this.sessionManager?.onSessionId(callback) ?? (() => {}) } /** Get list of all surveys. */ @@ -1251,6 +1252,9 @@ export class PostHog { * @param {Object} [userPropertiesToSetOnce] Optional: An associative array of properties to store about the user. If property is previously set, this does not override that value. */ identify(new_distinct_id?: string, userPropertiesToSet?: Properties, userPropertiesToSetOnce?: Properties): void { + if (!this.__loaded || !this.persistence) { + return + } //if the new_distinct_id has not been set ignore the identify event if (!new_distinct_id) { console.error('Unique user id has not been set in posthog.identify') @@ -1417,11 +1421,14 @@ export class PostHog { * Useful for clearing data when a user logs out. */ reset(reset_device_id?: boolean): void { + if (!this.__loaded) { + return + } const device_id = this.get_property('$device_id') - this.persistence.clear() - this.sessionPersistence.clear() - this.persistence.set_user_state('anonymous') - this.sessionManager.resetSessionId() + this.persistence?.clear() + this.sessionPersistence?.clear() + this.persistence?.set_user_state('anonymous') + this.sessionManager?.resetSessionId() const uuid = this.get_config('get_device_id')(this.uuidFn()) this.register_once( { @@ -1464,7 +1471,7 @@ export class PostHog { */ get_session_id(): string { - return this.sessionManager.checkAndGetSessionAndWindowId(true).sessionId + return this.sessionManager?.checkAndGetSessionAndWindowId(true).sessionId ?? '' } /** @@ -1475,6 +1482,9 @@ export class PostHog { * @param options.timestampLookBack How many seconds to look back for the timestamp (defaults to 10) */ get_session_replay_url(options?: { withTimestamp?: boolean; timestampLookBack?: number }): string { + if (!this.sessionManager) { + return '' + } const host = this.config.ui_host || this.config.api_host const { sessionId, sessionStartTimestamp } = this.sessionManager.checkAndGetSessionAndWindowId(true) let url = host + '/replay/' + sessionId @@ -1765,7 +1775,7 @@ export class PostHog { * @param {String} property_name The name of the super property you want to retrieve */ get_property(property_name: string): Property | undefined { - return this.persistence['props'][property_name] + return this.persistence?.['props'][property_name] } /** @@ -1788,7 +1798,7 @@ export class PostHog { * @param {String} property_name The name of the session super property you want to retrieve */ getSessionProperty(property_name: string): Property | undefined { - return this.sessionPersistence['props'][property_name] + return this.sessionPersistence?.['props'][property_name] } toString(): string { @@ -1851,11 +1861,11 @@ export class PostHog { return } - if (!this.get_config('disable_persistence') && this.persistence.disabled !== disabled) { - this.persistence.set_disabled(disabled) + if (!this.get_config('disable_persistence') && this.persistence?.disabled !== disabled) { + this.persistence?.set_disabled(disabled) } - if (!this.get_config('disable_persistence') && this.sessionPersistence.disabled !== disabled) { - this.sessionPersistence.set_disabled(disabled) + if (!this.get_config('disable_persistence') && this.sessionPersistence?.disabled !== disabled) { + this.sessionPersistence?.set_disabled(disabled) } } diff --git a/src/posthog-featureflags.ts b/src/posthog-featureflags.ts index 5a71a5962..fb3c32cb1 100644 --- a/src/posthog-featureflags.ts +++ b/src/posthog-featureflags.ts @@ -224,7 +224,7 @@ export class PostHogFeatureFlags { } else { flagCallReported[key] = [flagReportValue] } - this.instance.persistence.register({ [FLAG_CALL_REPORTED]: flagCallReported }) + this.instance.persistence?.register({ [FLAG_CALL_REPORTED]: flagCallReported }) this.instance.capture('$feature_flag_called', { $feature_flag: key, $feature_flag_response: flagValue }) } @@ -264,6 +264,9 @@ export class PostHogFeatureFlags { } receivedFeatureFlags(response: Partial): void { + if (!this.instance.persistence) { + return + } this.instance.decideEndpointWasHit = true const currentFlags = this.getFlagVariants() const currentFlagPayloads = this.getFlagPayloads() @@ -286,15 +289,15 @@ export class PostHogFeatureFlags { this._override_warning = false if (flags === false) { - this.instance.persistence.unregister(PERSISTENCE_OVERRIDE_FEATURE_FLAGS) + this.instance.persistence?.unregister(PERSISTENCE_OVERRIDE_FEATURE_FLAGS) } else if (Array.isArray(flags)) { const flagsObj: Record = {} for (let i = 0; i < flags.length; i++) { flagsObj[flags[i]] = true } - this.instance.persistence.register({ [PERSISTENCE_OVERRIDE_FEATURE_FLAGS]: flagsObj }) + this.instance.persistence?.register({ [PERSISTENCE_OVERRIDE_FEATURE_FLAGS]: flagsObj }) } else { - this.instance.persistence.register({ [PERSISTENCE_OVERRIDE_FEATURE_FLAGS]: flags }) + this.instance.persistence?.register({ [PERSISTENCE_OVERRIDE_FEATURE_FLAGS]: flags }) } } /* @@ -330,7 +333,7 @@ export class PostHogFeatureFlags { this.setPersonPropertiesForFlags(enrollmentPersonProp, false) const newFlags = { ...this.getFlagVariants(), [key]: isEnrolled } - this.instance.persistence.register({ + this.instance.persistence?.register({ [PERSISTENCE_ACTIVE_FEATURE_FLAGS]: Object.keys(filterActiveFeatureFlags(newFlags)), [ENABLED_FEATURE_FLAGS]: newFlags, }) @@ -349,7 +352,7 @@ export class PostHogFeatureFlags { { method: 'GET' }, (response) => { const earlyAccessFeatures = (response as EarlyAccessFeatureResponse).earlyAccessFeatures - this.instance.persistence.register({ [PERSISTENCE_EARLY_ACCESS_FEATURES]: earlyAccessFeatures }) + this.instance.persistence?.register({ [PERSISTENCE_EARLY_ACCESS_FEATURES]: earlyAccessFeatures }) return callback(earlyAccessFeatures) } ) diff --git a/src/posthog-surveys.ts b/src/posthog-surveys.ts index 7d98b5875..d7469a73e 100644 --- a/src/posthog-surveys.ts +++ b/src/posthog-surveys.ts @@ -72,7 +72,7 @@ export class PostHogSurveys { { method: 'GET' }, (response) => { const surveys = response.surveys - this.instance.persistence.register({ [SURVEYS]: surveys }) + this.instance.persistence?.register({ [SURVEYS]: surveys }) return callback(surveys) } ) From e6fa0924a3a8f1f1ee5937c813c4946d72285e41 Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 3 Aug 2023 16:14:51 +0200 Subject: [PATCH 02/16] Fix tests --- src/__tests__/posthog-core.identify.js | 10 +++++----- src/__tests__/posthog-core.js | 9 ++++++++- src/posthog-core.ts | 9 +++------ 3 files changed, 16 insertions(+), 12 deletions(-) diff --git a/src/__tests__/posthog-core.identify.js b/src/__tests__/posthog-core.identify.js index f93858323..ee6b51c1c 100644 --- a/src/__tests__/posthog-core.identify.js +++ b/src/__tests__/posthog-core.identify.js @@ -7,7 +7,11 @@ jest.mock('../gdpr-utils', () => ({ })) jest.mock('../decide') -given('lib', () => Object.assign(new PostHog(), given.overrides)) +given('lib', () => { + const posthog = new PostHog() + posthog._init('testtoken', given.config, 'testhog') + return Object.assign(posthog, given.overrides) +}) describe('identify()', () => { given( @@ -339,10 +343,6 @@ describe('reset()', () => { persistence: new PostHogPersistence(given.config), })) - beforeEach(() => { - given.lib._init('testtoken', given.config, 'testhog') - }) - it('clears persistence', () => { given.lib.persistence.register({ $enabled_feature_flags: { flag: 'variant', other: true } }) expect(given.lib.persistence.props['$enabled_feature_flags']).toEqual({ flag: 'variant', other: true }) diff --git a/src/__tests__/posthog-core.js b/src/__tests__/posthog-core.js index 3d211759d..380adea04 100644 --- a/src/__tests__/posthog-core.js +++ b/src/__tests__/posthog-core.js @@ -16,7 +16,11 @@ jest.mock('../utils', () => ({ document: { title: 'test' }, })) -given('lib', () => Object.assign(new PostHog(), given.overrides)) +given('lib', () => { + const posthog = new PostHog() + posthog._init('testtoken', given.config, 'testhog') + return Object.assign(posthog, given.overrides) +}) describe('capture()', () => { given('eventName', () => '$event') @@ -29,6 +33,7 @@ describe('capture()', () => { given('config', () => ({ property_blacklist: [], _onCapture: jest.fn(), + get_device_id: jest.fn().mockReturnValue('device-id'), })) given('overrides', () => ({ @@ -38,11 +43,13 @@ describe('capture()', () => { persistence: { remove_event_timer: jest.fn(), properties: jest.fn(), + update_config: jest.fn(), }, sessionPersistence: { update_search_keyword: jest.fn(), update_campaign_params: jest.fn(), update_referrer_info: jest.fn(), + update_config: jest.fn(), properties: jest.fn(), }, compression: {}, diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 348f5d036..b9daa9859 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -1691,12 +1691,9 @@ export class PostHog { this.config.disable_persistence = this.config.disable_cookie } - if (this.persistence) { - this.persistence.update_config(this.config) - } - if (this.sessionPersistence) { - this.sessionPersistence.update_config(this.config) - } + this.persistence?.update_config(this.config) + this.sessionPersistence?.update_config(this.config) + if (localStore.is_supported() && localStore.get('ph_debug') === 'true') { this.config.debug = true } From d80f4279b3dd2c1abbe3aed5723a0ebff0776afa Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 3 Aug 2023 16:20:04 +0200 Subject: [PATCH 03/16] Fix --- src/extensions/sessionrecording.ts | 12 +++++++----- src/posthog-core.ts | 6 +++--- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/src/extensions/sessionrecording.ts b/src/extensions/sessionrecording.ts index 574b10049..e01970d54 100644 --- a/src/extensions/sessionrecording.ts +++ b/src/extensions/sessionrecording.ts @@ -101,7 +101,7 @@ export class SessionRecording { }) } - private get sessionManager() { + private getSessionManager() { if (!this.instance.sessionManager) { logger.error('Session recording started without valid sessionManager') return @@ -194,7 +194,8 @@ export class SessionRecording { } private _startCapture() { - if (!this.sessionManager) { + const sessionManager = this.getSessionManager() + if (!sessionManager) { return } if (typeof Object.assign === 'undefined') { @@ -215,7 +216,7 @@ export class SessionRecording { this.captureStarted = true // We want to ensure the sessionManager is reset if necessary on load of the recorder - this.sessionManager.checkAndGetSessionAndWindowId() + sessionManager.checkAndGetSessionAndWindowId() const recorderJS = this.getRecordingVersion() === 'v2' ? 'recorder-v2.js' : 'recorder.js' @@ -243,7 +244,8 @@ export class SessionRecording { } private _updateWindowAndSessionIds(event: eventWithTime) { - if (!this.sessionManager) { + const sessionManager = this.getSessionManager() + if (!sessionManager) { return } // Some recording events are triggered by non-user events (e.g. "X minutes ago" text updating on the screen). @@ -273,7 +275,7 @@ export class SessionRecording { } // We only want to extend the session if it is an interactive event. - const { windowId, sessionId } = this.sessionManager.checkAndGetSessionAndWindowId( + const { windowId, sessionId } = sessionManager.checkAndGetSessionAndWindowId( !isUserInteraction, event.timestamp ) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index b9daa9859..11e0ee627 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -265,9 +265,6 @@ export class PostHog { featureFlags: PostHogFeatureFlags surveys: PostHogSurveys toolbar: Toolbar - sessionRecording: SessionRecording | undefined - webPerformance: WebPerformanceObserver | undefined - exceptionAutocapture: ExceptionObserver | undefined // These are instance-specific state created after initialisation persistence?: PostHogPersistence @@ -275,6 +272,9 @@ export class PostHog { sessionManager?: SessionIdManager _requestQueue?: RequestQueue _retryQueue?: RetryQueue + sessionRecording?: SessionRecording + webPerformance?: WebPerformanceObserver + exceptionAutocapture?: ExceptionObserver _triggered_notifs: any compression: Partial> From 10c462b2575cc0be19102895f575792cd716d6bd Mon Sep 17 00:00:00 2001 From: Ben White Date: Fri, 4 Aug 2023 09:11:06 +0200 Subject: [PATCH 04/16] fix: Warn on init without token --- src/posthog-core.ts | 46 +++++++++++++++++++++++---------------------- 1 file changed, 24 insertions(+), 22 deletions(-) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 1d3773b18..8a0d3b8da 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -193,6 +193,12 @@ const create_phlib = function ( } } + if (!token) { + console.warn( + 'PostHog was initialized without a token. This likely indicates a misconfiguration. Please check the first argument passed to posthog.init()' + ) + } + if (target && init_type === InitType.INIT_MODULE) { instance = target as any } else { @@ -2066,12 +2072,12 @@ const extend_mp = function () { const override_ph_init_func = function () { // we override the snippets init function to handle the case where a // user initializes the posthog library after the script loads & runs - posthog_master['init'] = function (token?: string, config?: Partial, name?: string) { + posthog_master['init'] = function (token: string, config?: Partial, name?: string) { if (name) { // initialize a sub library if (!posthog_master[name]) { posthog_master[name] = instances[name] = create_phlib( - token || '', + token, config || {}, name, (instance: PostHog) => { @@ -2081,28 +2087,25 @@ const override_ph_init_func = function () { ) } return posthog_master[name] - } else { - let instance: PostHog = posthog_master as any as PostHog - - if (instances[PRIMARY_INSTANCE_NAME]) { - // main posthog lib already initialized - instance = instances[PRIMARY_INSTANCE_NAME] - } else if (token) { - // intialize the main posthog lib - instance = create_phlib(token, config || {}, PRIMARY_INSTANCE_NAME, (instance: PostHog) => { - instances[PRIMARY_INSTANCE_NAME] = instance - instance._loaded() - }) + } + + let instance: PostHog = posthog_master as any as PostHog + + if (!instances[PRIMARY_INSTANCE_NAME]) { + // intialize the main posthog lib if we haven't already + instance = create_phlib(token, config || {}, PRIMARY_INSTANCE_NAME, (instance: PostHog) => { instances[PRIMARY_INSTANCE_NAME] = instance - } + instance._loaded() + }) + } - ;(posthog_master as any) = instance - if (init_type === InitType.INIT_SNIPPET) { - ;(window as any)[PRIMARY_INSTANCE_NAME] = posthog_master - } - extend_mp() - return instance + instances[PRIMARY_INSTANCE_NAME] = instance + ;(posthog_master as any) = instance + if (init_type === InitType.INIT_SNIPPET) { + ;(window as any)[PRIMARY_INSTANCE_NAME] = posthog_master } + extend_mp() + return instance } } @@ -2174,7 +2177,6 @@ export function init_as_module(): PostHog { ;(posthog_master as any) = new PostHog() override_ph_init_func() - ;(posthog_master['init'] as any)() add_dom_loaded_handler() return posthog_master as any From 1982b80edae082c2975222e07b8d5c238f2a88b9 Mon Sep 17 00:00:00 2001 From: Ben White Date: Fri, 4 Aug 2023 10:25:33 +0200 Subject: [PATCH 05/16] Refactor initialisation --- playground/session-recordings/index.html | 10 + src/posthog-core.ts | 280 +++++++---------------- 2 files changed, 97 insertions(+), 193 deletions(-) diff --git a/playground/session-recordings/index.html b/playground/session-recordings/index.html index 97e514ff6..cd52fc996 100644 --- a/playground/session-recordings/index.html +++ b/playground/session-recordings/index.html @@ -100,6 +100,16 @@ capture_performance: true, }) + posthog.init( + 'phc_other', + { + api_host: 'http://localhost:8000', + disable_session_recording: true, + capture_performance: true, + }, + 'other' + ) + setTimeout(() => { posthog.debug() document.getElementById('current-session-id').innerHTML = posthog.sessionRecording.sessionId diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 8a0d3b8da..8b114d14f 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -68,18 +68,13 @@ this.__x === private - only use within the class Globals should be all caps */ -enum InitType { - INIT_MODULE = 0, - INIT_SNIPPET = 1, -} - -let init_type: InitType - // TODO: the type of this is very loose. Sometimes it's also PostHogLib itself let posthog_master: Record & { init: (token: string, config: Partial, name: string) => void } +const instances: Record = {} + // some globals for comparisons const __NOOP = () => {} const __NOOPTIONS = {} @@ -163,100 +158,6 @@ const defaultConfig = (): PostHogConfig => ({ uuid_version: 'v7', }) -/** - * create_phlib(token:string, config:object, name:string) - * - * This function is used by the init method of PostHogLib objects - * as well as the main initializer at the end of the JSLib (that - * initializes document.posthog as well as any additional instances - * declared before this file has loaded). - */ -const create_phlib = function ( - token: string, - config?: Partial, - name?: string, - createComplete?: (instance: PostHog) => void -): PostHog { - let instance: PostHog - const target = - name === PRIMARY_INSTANCE_NAME || !posthog_master ? posthog_master : name ? posthog_master[name] : undefined - const callbacksHandled = { - initComplete: false, - syncCode: false, - } - const handleCallback = (callbackName: keyof typeof callbacksHandled) => (instance: PostHog) => { - if (!callbacksHandled[callbackName]) { - callbacksHandled[callbackName] = true - if (callbacksHandled.initComplete && callbacksHandled.syncCode) { - createComplete?.(instance) - } - } - } - - if (!token) { - console.warn( - 'PostHog was initialized without a token. This likely indicates a misconfiguration. Please check the first argument passed to posthog.init()' - ) - } - - if (target && init_type === InitType.INIT_MODULE) { - instance = target as any - } else { - if (target && !_isArray(target)) { - console.error('You have already initialized ' + name) - // TODO: throw something instead? - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - return - } - instance = new PostHog() - } - - instance._init(token, config, name, handleCallback('initComplete')) - instance.toolbar.maybeLoadToolbar() - - instance.sessionRecording = new SessionRecording(instance) - instance.sessionRecording.startRecordingIfEnabled() - - instance.webPerformance = new WebPerformanceObserver(instance) - instance.webPerformance.startObservingIfEnabled() - - instance.exceptionAutocapture = new ExceptionObserver(instance) - - instance.__autocapture = instance.get_config('autocapture') - autocapture._setIsAutocaptureEnabled(instance) - if (autocapture._isAutocaptureEnabled) { - instance.__autocapture = instance.get_config('autocapture') - const num_buckets = 100 - const num_enabled_buckets = 100 - if (!autocapture.enabledForProject(instance.get_config('token'), num_buckets, num_enabled_buckets)) { - instance.__autocapture = false - logger.log('Not in active bucket: disabling Automatic Event Collection.') - } else if (!autocapture.isBrowserSupported()) { - instance.__autocapture = false - logger.log('Disabling Automatic Event Collection because this browser is not supported') - } else { - autocapture.init(instance) - } - } - - // if any instance on the page has debug = true, we set the - // global debug to be true - Config.DEBUG = Config.DEBUG || instance.get_config('debug') - - // if target is not defined, we called init after the lib already - // loaded, so there won't be an array of things to execute - if (typeof target !== 'undefined' && _isArray(target)) { - // Crunch through the people queue first - we queue this data up & - // flush on identify, so it's better to do all these operations first - instance._execute_array.call(instance.people, (target as any).people) - instance._execute_array(target) - } - - handleCallback('syncCode')(instance) - return instance -} - /** * PostHog Library Object * @constructor @@ -361,22 +262,18 @@ export class PostHog { * @param {String} [name] The name for the new posthog instance that you want created */ init(token: string, config?: Partial, name?: string): PostHog | void { - if (_isUndefined(name)) { - console.error('You must name your new library: init(token, config, name)') - return - } - if (name === PRIMARY_INSTANCE_NAME) { - console.error('You must initialize the main posthog object right after you include the PostHog js snippet') - return - } - - const instance: PostHog = create_phlib(token, config, name, (instance: PostHog) => { - posthog_master[name] = instance - instance._loaded() - }) - posthog_master[name] = instance + if (!name || name === PRIMARY_INSTANCE_NAME) { + // This means we are initialising the primary instance (i.e. this) + return this._init(token, config, name) + } else { + const namedPosthog = instances[name] ?? new PostHog() + namedPosthog._init(token, config, name) + instances[name] = namedPosthog + // Add as a property to the primary instance (this isn't type-safe but its how it was always done) + ;(instances[PRIMARY_INSTANCE_NAME] as any)[name] = namedPosthog - return instance + return namedPosthog + } } // posthog._init(token:string, config:object, name:string) @@ -397,7 +294,19 @@ export class PostHog { config: Partial = {}, name?: string, initComplete?: (instance: PostHog) => void - ): void { + ): PostHog | void { + if (!token) { + console.warn( + 'PostHog was initialized without a token. This likely indicates a misconfiguration. Please check the first argument passed to posthog.init()' + ) + return + } + + if (this.__loaded) { + console.warn('You have already initialized PostHog! Re-initialising is a no-op') + return + } + this.__loaded = true this.config = {} as PostHogConfig // will be set right below this._triggered_notifs = [] @@ -457,6 +366,35 @@ export class PostHog { ? this.persistence : new PostHogPersistence({ ...this.config, persistence: 'sessionStorage' }) + this.sessionRecording = new SessionRecording(this) + this.sessionRecording.startRecordingIfEnabled() + + this.webPerformance = new WebPerformanceObserver(this) + this.webPerformance.startObservingIfEnabled() + + this.exceptionAutocapture = new ExceptionObserver(this) + + this.__autocapture = this.get_config('autocapture') + autocapture._setIsAutocaptureEnabled(this) + if (autocapture._isAutocaptureEnabled) { + this.__autocapture = this.get_config('autocapture') + const num_buckets = 100 + const num_enabled_buckets = 100 + if (!autocapture.enabledForProject(this.get_config('token'), num_buckets, num_enabled_buckets)) { + this.__autocapture = false + logger.log('Not in active bucket: disabling Automatic Event Collection.') + } else if (!autocapture.isBrowserSupported()) { + this.__autocapture = false + logger.log('Disabling Automatic Event Collection because this browser is not supported') + } else { + autocapture.init(this) + } + } + + // if any instance on the page has debug = true, we set the + // global debug to be true + Config.DEBUG = Config.DEBUG || this.get_config('debug') + this._gdpr_init() if (config.segment) { @@ -527,9 +465,14 @@ export class PostHog { window.addEventListener && window.addEventListener('onpagehide' in self ? 'pagehide' : 'unload', this._handle_unload.bind(this)) + this.toolbar.maybeLoadToolbar() // Make sure that we also call the initComplete callback at the end of // the synchronous code as well. updateInitComplete('syncCode')() + + this._loaded() + + return this } // Private methods @@ -2059,56 +2002,6 @@ export class PostHog { _safewrap_class(PostHog, ['identify']) -const instances: Record = {} -const extend_mp = function () { - // add all the sub posthog instances - _each(instances, function (instance, name) { - if (name !== PRIMARY_INSTANCE_NAME) { - posthog_master[name] = instance - } - }) -} - -const override_ph_init_func = function () { - // we override the snippets init function to handle the case where a - // user initializes the posthog library after the script loads & runs - posthog_master['init'] = function (token: string, config?: Partial, name?: string) { - if (name) { - // initialize a sub library - if (!posthog_master[name]) { - posthog_master[name] = instances[name] = create_phlib( - token, - config || {}, - name, - (instance: PostHog) => { - posthog_master[name] = instances[name] = instance - instance._loaded() - } - ) - } - return posthog_master[name] - } - - let instance: PostHog = posthog_master as any as PostHog - - if (!instances[PRIMARY_INSTANCE_NAME]) { - // intialize the main posthog lib if we haven't already - instance = create_phlib(token, config || {}, PRIMARY_INSTANCE_NAME, (instance: PostHog) => { - instances[PRIMARY_INSTANCE_NAME] = instance - instance._loaded() - }) - } - - instances[PRIMARY_INSTANCE_NAME] = instance - ;(posthog_master as any) = instance - if (init_type === InitType.INIT_SNIPPET) { - ;(window as any)[PRIMARY_INSTANCE_NAME] = posthog_master - } - extend_mp() - return instance - } -} - const add_dom_loaded_handler = function () { // Cross browser DOM Loaded support function dom_loaded_handler() { @@ -2142,42 +2035,43 @@ const add_dom_loaded_handler = function () { } export function init_from_snippet(): void { - init_type = InitType.INIT_SNIPPET - if (_isUndefined((window as any).posthog)) { - ;(window as any).posthog = [] - } - posthog_master = (window as any).posthog + const posthogMaster = (instances[PRIMARY_INSTANCE_NAME] = new PostHog()) - if (posthog_master['__loaded'] || (posthog_master['config'] && posthog_master['persistence'])) { - // lib has already been loaded at least once; we don't want to override the global object this time so bomb early - console.error('PostHog library has already been downloaded at least once.') - return - } + const snippetPostHog = (window as any)['posthog'] - // Load instances of the PostHog Library - _each(posthog_master['_i'], function (item: [token: string, config: Partial, name: string]) { - if (item && _isArray(item)) { - instances[item[2]] = create_phlib(...item) - } - }) + if (snippetPostHog) { + // Call all pre-loaded init calls properly + _each(snippetPostHog['_i'], function (item: [token: string, config: Partial, name: string]) { + if (item && _isArray(item)) { + const instance = posthogMaster.init(item[0], item[1], item[2]) + + if (instance) { + // instance._execute_array.call(instance.people, item[3]) + } - override_ph_init_func() - ;(posthog_master['init'] as any)() + // // if target is not defined, we called init after the lib already + // // loaded, so there won't be an array of things to execute + // if (typeof target !== 'undefined' && _isArray(target)) { + // // Crunch through the people queue first - we queue this data up & + // // flush on identify, so it's better to do all these operations first + // instance._execute_array.call(instance.people, (target as any).people) + // instance._execute_array(target) + // } + } + }) + + delete snippetPostHog['_i'] + } - // Fire loaded events after updating the window's posthog object - _each(instances, function (instance) { - instance._loaded() - }) + ;(window as any)['posthog'] = posthogMaster add_dom_loaded_handler() } export function init_as_module(): PostHog { - init_type = InitType.INIT_MODULE - ;(posthog_master as any) = new PostHog() + const posthogMaster = (instances[PRIMARY_INSTANCE_NAME] = new PostHog()) - override_ph_init_func() add_dom_loaded_handler() - return posthog_master as any + return posthogMaster } From 81480a73933447fd042fbdca89df0f6a4d4454ee Mon Sep 17 00:00:00 2001 From: Ben White Date: Fri, 4 Aug 2023 10:30:53 +0200 Subject: [PATCH 06/16] Removed the initComplete logic --- src/posthog-core.ts | 42 +++++++++--------------------------------- 1 file changed, 9 insertions(+), 33 deletions(-) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 8b114d14f..0cb86c655 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -289,12 +289,7 @@ export class PostHog { // IE11 compatible. We could use polyfills, which would make the // code a bit cleaner, but will add some overhead. // - _init( - token: string, - config: Partial = {}, - name?: string, - initComplete?: (instance: PostHog) => void - ): PostHog | void { + _init(token: string, config: Partial = {}, name?: string): PostHog | void { if (!token) { console.warn( 'PostHog was initialized without a token. This likely indicates a misconfiguration. Please check the first argument passed to posthog.init()' @@ -311,24 +306,6 @@ export class PostHog { this.config = {} as PostHogConfig // will be set right below this._triggered_notifs = [] - // To avoid using Promises and their helper functions, we instead keep - // track of which callbacks have been called, and then call initComplete - // when all of them have been called. To add additional async code, add - // to `callbacksHandled` and pass updateInitComplete as a callback to - // the async code. - const callbacksHandled = { segmentRegister: false, syncCode: false } - const updateInitComplete = (callbackName: keyof typeof callbacksHandled) => () => { - // Update the register of callbacks that have been called, and if - // they have all been called, then we are ready to call - // initComplete. - if (!callbacksHandled[callbackName]) { - callbacksHandled[callbackName] = true - if (callbacksHandled.segmentRegister && callbacksHandled.syncCode) { - initComplete?.(this) - } - } - } - this.set_config( _extend({}, defaultConfig(), config, { name: name, @@ -408,10 +385,6 @@ export class PostHog { }) this.persistence.set_user_state('identified') } - - config.segment.register(this.segmentIntegration()).then(updateInitComplete('segmentRegister')) - } else { - updateInitComplete('segmentRegister')() } if (config.bootstrap?.distinctID !== undefined) { @@ -466,17 +439,20 @@ export class PostHog { window.addEventListener('onpagehide' in self ? 'pagehide' : 'unload', this._handle_unload.bind(this)) this.toolbar.maybeLoadToolbar() - // Make sure that we also call the initComplete callback at the end of - // the synchronous code as well. - updateInitComplete('syncCode')() - this._loaded() + // We wan't to avoid promises for IE11 compatibility, so we use callbacks here + if (config.segment) { + config.segment.register(this.segmentIntegration()).then(() => { + this._loaded() + }) + } else { + this._loaded() + } return this } // Private methods - _loaded(): void { // Pause `reloadFeatureFlags` calls in config.loaded callback. // These feature flags are loaded in the decide call made right From 01265994a8ce9b152012f506bd48a304e228831f Mon Sep 17 00:00:00 2001 From: Ben White Date: Fri, 4 Aug 2023 11:02:19 +0200 Subject: [PATCH 07/16] Documented weird Added some proper logging to explain how the snippet calls work --- playground/session-recordings/index.html | 7 +++ src/posthog-core.ts | 54 ++++++++++++++++-------- 2 files changed, 44 insertions(+), 17 deletions(-) diff --git a/playground/session-recordings/index.html b/playground/session-recordings/index.html index cd52fc996..6a11a9de5 100644 --- a/playground/session-recordings/index.html +++ b/playground/session-recordings/index.html @@ -110,6 +110,13 @@ 'other' ) + posthog.capture('event') + posthog.capture('event2') + posthog.people.set({ test: true }) + posthog.other.capture('other_event') + posthog.other.people.set({ test: true }) + console.log(posthog) + setTimeout(() => { posthog.debug() document.getElementById('current-session-id').innerHTML = posthog.sessionRecording.sessionId diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 480836683..3e39cdca5 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -68,11 +68,6 @@ this.__x === private - only use within the class Globals should be all caps */ -// TODO: the type of this is very loose. Sometimes it's also PostHogLib itself -let posthog_master: Record & { - init: (token: string, config: Partial, name: string) => void -} - const instances: Record = {} // some globals for comparisons @@ -2023,27 +2018,52 @@ export function init_from_snippet(): void { const snippetPostHog = (window as any)['posthog'] if (snippetPostHog) { + /** + * The snippet uses some clever tricks to allow deferred loading of array.js (this code) + * + * window.posthog is an array which the queue of calls made before the lib is loaded + * It has methods attached to it to simulate the posthog object so for instance + * + * window.posthog.init("TOKEN", {api_host: "foo" }) + * window.posthog.capture("my-event", {foo: "bar" }) + * + * ... will mean that window.posthog will look like this: + * window.posthog == [ + * ["my-event", {foo: "bar"}] + * ] + * + * window.posthog[_i] == [ + * ["TOKEN", {api_host: "foo" }, "posthog"] + * ] + * + * If a name is given to the init function then the same as above is true but as a sub-property on the object: + * + * window.posthog.init("TOKEN", {}, "ph2") + * window.posthog.ph2.people.set({foo: "bar"}) + * + * window.posthog.ph2 == [] + * window.posthog.people == [ + * ["set", {foo: "bar"}] + * ] + * + */ + // Call all pre-loaded init calls properly + _each(snippetPostHog['_i'], function (item: [token: string, config: Partial, name: string]) { if (item && _isArray(item)) { const instance = posthogMaster.init(item[0], item[1], item[2]) + let instanceSnippet = snippetPostHog[item[2]] || snippetPostHog + if (instance) { - // instance._execute_array.call(instance.people, item[3]) + // Crunch through the people queue first - we queue this data up & + // flush on identify, so it's better to do all these operations first + instance._execute_array.call(instance.people, instanceSnippet.people) + instance._execute_array(instanceSnippet) } - - // // if target is not defined, we called init after the lib already - // // loaded, so there won't be an array of things to execute - // if (typeof target !== 'undefined' && _isArray(target)) { - // // Crunch through the people queue first - we queue this data up & - // // flush on identify, so it's better to do all these operations first - // instance._execute_array.call(instance.people, (target as any).people) - // instance._execute_array(target) - // } } }) - - delete snippetPostHog['_i'] } ;(window as any)['posthog'] = posthogMaster From 05a755ba3d47d84b57e184a45eb604a1464de29b Mon Sep 17 00:00:00 2001 From: Ben White Date: Fri, 4 Aug 2023 16:30:13 +0200 Subject: [PATCH 08/16] Some fixes --- src/__tests__/posthog-core.identify.js | 2 +- src/posthog-core.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/__tests__/posthog-core.identify.js b/src/__tests__/posthog-core.identify.js index ee6b51c1c..e02014532 100644 --- a/src/__tests__/posthog-core.identify.js +++ b/src/__tests__/posthog-core.identify.js @@ -9,7 +9,7 @@ jest.mock('../decide') given('lib', () => { const posthog = new PostHog() - posthog._init('testtoken', given.config, 'testhog') + posthog.init('testtoken', given.config) return Object.assign(posthog, given.overrides) }) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 3e39cdca5..c3c369fb6 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -862,7 +862,7 @@ export class PostHog { properties['$window_id'] = windowId } - if (this.webPerformance?.isEnabled) { + if (this.webPerformance?.isEnabled()) { if (event_name === '$pageview') { this.pageViewIdManager.onPageview() } From ced68fb62753fa614966720185b9ab2a948db208 Mon Sep 17 00:00:00 2001 From: Paul D'Ambra Date: Wed, 13 Sep 2023 09:00:10 +0100 Subject: [PATCH 09/16] fix lint issues --- src/extensions/sessionrecording.ts | 2 +- src/posthog-core.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/src/extensions/sessionrecording.ts b/src/extensions/sessionrecording.ts index e01970d54..9e7b6d4e3 100644 --- a/src/extensions/sessionrecording.ts +++ b/src/extensions/sessionrecording.ts @@ -348,7 +348,7 @@ export class SessionRecording { this.mutationRateLimiter = this.mutationRateLimiter ?? - new MutationRateLimiter(this.rrwebRecord!, { + new MutationRateLimiter(this.rrwebRecord, { onBlockedNode: (id, node) => { const message = `Too many mutations on node '${id}'. Rate limiting. This could be due to SVG animations or something similar` logger.log(message, { diff --git a/src/posthog-core.ts b/src/posthog-core.ts index c3c369fb6..353c9aea0 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -2054,7 +2054,7 @@ export function init_from_snippet(): void { if (item && _isArray(item)) { const instance = posthogMaster.init(item[0], item[1], item[2]) - let instanceSnippet = snippetPostHog[item[2]] || snippetPostHog + const instanceSnippet = snippetPostHog[item[2]] || snippetPostHog if (instance) { // Crunch through the people queue first - we queue this data up & From cbf8b5dc34458a1f28b92143e98fd8da8ed2ee6e Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 2 Jan 2024 16:55:35 +0100 Subject: [PATCH 10/16] Fix --- src/posthog-core.ts | 20 +++++++++++--------- 1 file changed, 11 insertions(+), 9 deletions(-) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 388cde8fb..6ce7fd80a 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -346,23 +346,25 @@ export class PostHog { this.sessionRecording = new SessionRecording(this) this.sessionRecording.startRecordingIfEnabled() - this.webPerformance = new WebPerformanceObserver(this) - this.webPerformance.startObservingIfEnabled() + this.sessionRecording = new SessionRecording(this) + this.sessionRecording.startRecordingIfEnabled() - this.exceptionAutocapture = new ExceptionObserver(this) + if (this.config.__preview_measure_pageview_stats) { + this.pageViewManager.startMeasuringScrollPosition() + } - this.__autocapture = this.get_config('autocapture') + this.__autocapture = this.config.autocapture autocapture._setIsAutocaptureEnabled(this) if (autocapture._isAutocaptureEnabled) { - this.__autocapture = this.get_config('autocapture') + this.__autocapture = this.config.autocapture const num_buckets = 100 const num_enabled_buckets = 100 - if (!autocapture.enabledForProject(this.get_config('token'), num_buckets, num_enabled_buckets)) { + if (!autocapture.enabledForProject(this.config.token, num_buckets, num_enabled_buckets)) { this.__autocapture = false - logger.log('Not in active bucket: disabling Automatic Event Collection.') + logger.info('Not in active bucket: disabling Automatic Event Collection.') } else if (!autocapture.isBrowserSupported()) { this.__autocapture = false - logger.log('Disabling Automatic Event Collection because this browser is not supported') + logger.info('Disabling Automatic Event Collection because this browser is not supported') } else { autocapture.init(this) } @@ -370,7 +372,7 @@ export class PostHog { // if any instance on the page has debug = true, we set the // global debug to be true - Config.DEBUG = Config.DEBUG || this.get_config('debug') + Config.DEBUG = Config.DEBUG || this.config.debug this._gdpr_init() From 7df06cb88b65667b989e4a5c76f277326256230f Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 2 Jan 2024 16:56:12 +0100 Subject: [PATCH 11/16] Fix --- src/posthog-core.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 6ce7fd80a..9456ab8f2 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -9,7 +9,7 @@ import { isCrossDomainCookie, isDistinctIdStringLike, } from './utils' -import { assignableWindow, window } from './utils/globals' +import { window } from './utils/globals' import { autocapture } from './autocapture' import { PostHogFeatureFlags } from './posthog-featureflags' import { PostHogPersistence } from './posthog-persistence' @@ -295,14 +295,14 @@ export class PostHog { // _init(token: string, config: Partial = {}, name?: string): PostHog | void { if (!token) { - console.warn( + logger.critical( 'PostHog was initialized without a token. This likely indicates a misconfiguration. Please check the first argument passed to posthog.init()' ) return } if (this.__loaded) { - console.warn('You have already initialized PostHog! Re-initialising is a no-op') + logger.warn('You have already initialized PostHog! Re-initialising is a no-op') return } From b4aaf67bd15ea8cce352b47d56200f12953aed49 Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 2 Jan 2024 16:56:57 +0100 Subject: [PATCH 12/16] Fix --- src/posthog-featureflags.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/posthog-featureflags.ts b/src/posthog-featureflags.ts index 93c9aa967..924134252 100644 --- a/src/posthog-featureflags.ts +++ b/src/posthog-featureflags.ts @@ -301,15 +301,15 @@ export class PostHogFeatureFlags { this._override_warning = false if (flags === false) { - this.instance.persistence?.unregister(PERSISTENCE_OVERRIDE_FEATURE_FLAGS) + this.instance.persistence.unregister(PERSISTENCE_OVERRIDE_FEATURE_FLAGS) } else if (_isArray(flags)) { const flagsObj: Record = {} for (let i = 0; i < flags.length; i++) { flagsObj[flags[i]] = true } - this.instance.persistence?.register({ [PERSISTENCE_OVERRIDE_FEATURE_FLAGS]: flagsObj }) + this.instance.persistence.register({ [PERSISTENCE_OVERRIDE_FEATURE_FLAGS]: flagsObj }) } else { - this.instance.persistence?.register({ [PERSISTENCE_OVERRIDE_FEATURE_FLAGS]: flags }) + this.instance.persistence.register({ [PERSISTENCE_OVERRIDE_FEATURE_FLAGS]: flags }) } } /* From bb57ae021203c693309a2bce30309085a7877c1c Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 2 Jan 2024 18:20:15 +0100 Subject: [PATCH 13/16] Test fixes --- src/__tests__/posthog-core.identify.js | 6 ++-- src/__tests__/posthog-core.js | 45 +++----------------------- 2 files changed, 8 insertions(+), 43 deletions(-) diff --git a/src/__tests__/posthog-core.identify.js b/src/__tests__/posthog-core.identify.js index 26eea4ad7..9efe54762 100644 --- a/src/__tests__/posthog-core.identify.js +++ b/src/__tests__/posthog-core.identify.js @@ -1,5 +1,6 @@ -import { PostHog } from '../posthog-core' +import _posthog from '../loader-module' import { PostHogPersistence } from '../posthog-persistence' +import { uuidv7 } from '../uuidv7' jest.mock('../gdpr-utils', () => ({ ...jest.requireActual('../gdpr-utils'), @@ -8,8 +9,7 @@ jest.mock('../gdpr-utils', () => ({ jest.mock('../decide') given('lib', () => { - const posthog = new PostHog() - posthog.init('testtoken', given.config) + const posthog = _posthog.init('testtoken', given.config, uuidv7()) return Object.assign(posthog, given.overrides) }) diff --git a/src/__tests__/posthog-core.js b/src/__tests__/posthog-core.js index 7db3a4f6c..49e4fbd68 100644 --- a/src/__tests__/posthog-core.js +++ b/src/__tests__/posthog-core.js @@ -1,4 +1,4 @@ -import { init_as_module, PostHog } from '../posthog-core' +import _posthog from '../loader-module' import { PostHogPersistence } from '../posthog-persistence' import { Decide } from '../decide' import { autocapture } from '../autocapture' @@ -6,6 +6,7 @@ import { autocapture } from '../autocapture' import { truth } from './helpers/truth' import { _info } from '../utils/event-utils' import { document, window } from '../utils/globals' +import { uuidv7 } from '../uuidv7' jest.mock('../gdpr-utils', () => ({ ...jest.requireActual('../gdpr-utils'), @@ -14,8 +15,7 @@ jest.mock('../gdpr-utils', () => ({ jest.mock('../decide') given('lib', () => { - const posthog = new PostHog() - posthog._init('testtoken', given.config, 'testhog') + const posthog = _posthog.init('testtoken', given.config, uuidv7()) posthog.debug() return Object.assign(posthog, given.overrides) }) @@ -498,8 +498,6 @@ describe('posthog core', () => { }) describe('bootstrapping feature flags', () => { - given('subject', () => () => given.lib._init('posthog', given.config, 'testhog')) - given('overrides', () => ({ _send_request: jest.fn(), capture: jest.fn(), @@ -516,7 +514,6 @@ describe('posthog core', () => { }, })) - given.subject() expect(given.lib.get_distinct_id()).toBe('abcd') expect(given.lib.get_property('$device_id')).toBe('abcd') expect(given.lib.persistence.get_user_state()).toBe('anonymous') @@ -542,7 +539,6 @@ describe('posthog core', () => { get_device_id: () => 'og-device-id', })) - given.subject() expect(given.lib.get_distinct_id()).toBe('abcd') expect(given.lib.get_property('$device_id')).toBe('og-device-id') expect(given.lib.persistence.get_user_state()).toBe('identified') @@ -558,7 +554,6 @@ describe('posthog core', () => { }, })) - given.subject() expect(given.lib.get_distinct_id()).not.toBe('abcd') expect(given.lib.get_distinct_id()).not.toEqual(undefined) expect(given.lib.getFeatureFlag('multivariant')).toBe('variant-1') @@ -589,7 +584,6 @@ describe('posthog core', () => { }, })) - given.subject() expect(given.lib.getFeatureFlagPayload('multivariant')).toBe('some-payload') expect(given.lib.getFeatureFlagPayload('enabled')).toEqual({ another: 'value' }) expect(given.lib.getFeatureFlagPayload('jsonString')).toEqual({ a: 'payload' }) @@ -604,7 +598,6 @@ describe('posthog core', () => { bootstrap: {}, })) - given.subject() expect(given.lib.get_distinct_id()).not.toBe('abcd') expect(given.lib.get_distinct_id()).not.toEqual(undefined) expect(given.lib.getFeatureFlag('multivariant')).toBe(undefined) @@ -626,7 +619,6 @@ describe('posthog core', () => { }, })) - given.subject() given.lib.featureFlags.onFeatureFlags(() => (called = true)) expect(called).toEqual(true) }) @@ -640,7 +632,6 @@ describe('posthog core', () => { }, })) - given.subject() given.lib.featureFlags.onFeatureFlags(() => (called = true)) expect(called).toEqual(false) }) @@ -654,7 +645,6 @@ describe('posthog core', () => { }, })) - given.subject() given.lib.featureFlags.onFeatureFlags(() => (called = true)) expect(called).toEqual(false) }) @@ -662,7 +652,6 @@ describe('posthog core', () => { describe('init()', () => { jest.spyOn(window, 'window', 'get') - given('subject', () => () => given.lib._init('posthog', given.config, 'testhog')) given('overrides', () => ({ get_distinct_id: () => given.distinct_id, @@ -682,7 +671,6 @@ describe('posthog core', () => { given('advanced_disable_decide', () => true) it('can set an xhr error handler', () => { - init_as_module() const fakeOnXHRError = 'configured error' given('subject', () => given.lib.init( @@ -697,7 +685,6 @@ describe('posthog core', () => { }) it('does not load decide endpoint on advanced_disable_decide', () => { - given.subject() expect(given.decide).toBe(undefined) expect(given.overrides._send_request.mock.calls.length).toBe(0) // No outgoing requests }) @@ -706,44 +693,31 @@ describe('posthog core', () => { given('config', () => ({ api_host: 'https://example.com/custom/', })) - given.subject() expect(given.lib.config.api_host).toBe('https://example.com/custom') }) it('does not set __loaded_recorder_version flag if recording script has not been included', () => { - given('overrides', () => ({ - __loaded_recorder_version: undefined, - })) delete window.rrweb window.rrweb = { record: undefined } delete window.rrwebRecord window.rrwebRecord = undefined - given.subject() expect(given.lib.__loaded_recorder_version).toEqual(undefined) }) it('set __loaded_recorder_version flag to v1 if recording script has been included', () => { - given('overrides', () => ({ - __loaded_recorder_version: undefined, - })) delete window.rrweb window.rrweb = { record: 'anything', version: '1.1.3' } delete window.rrwebRecord window.rrwebRecord = 'is possible' - given.subject() expect(given.lib.__loaded_recorder_version).toMatch(/^1\./) // start with 1.?.? }) - it('set __loaded_recorder_version flag to v1 if recording script has been included', () => { - given('overrides', () => ({ - __loaded_recorder_version: undefined, - })) + it('set __loaded_recorder_version flag to v2 if recording script has been included', () => { delete window.rrweb window.rrweb = { record: 'anything', version: '2.0.0-alpha.6' } delete window.rrwebRecord window.rrwebRecord = 'is possible' - given.subject() expect(given.lib.__loaded_recorder_version).toMatch(/^2\./) // start with 2.?.? }) @@ -754,6 +728,7 @@ describe('posthog core', () => { startRecordingIfEnabled: jest.fn(), }, toolbar: { + maybeLoadToolbar: jest.fn(), afterDecideResponse: jest.fn(), }, persistence: { @@ -762,15 +737,11 @@ describe('posthog core', () => { }, })) - given.subject() - jest.spyOn(given.lib.toolbar, 'afterDecideResponse').mockImplementation() jest.spyOn(given.lib.sessionRecording, 'afterDecideResponse').mockImplementation() jest.spyOn(given.lib.persistence, 'register').mockImplementation() // Autocapture - expect(given.lib.__autocapture).toEqual(undefined) - expect(autocapture.init).not.toHaveBeenCalled() expect(autocapture.afterDecideResponse).not.toHaveBeenCalled() // Feature flags @@ -790,8 +761,6 @@ describe('posthog core', () => { it('sets a random UUID as distinct_id/$device_id if distinct_id is unset', () => { given('distinct_id', () => undefined) - given.subject() - expect(given.lib.register_once).toHaveBeenCalledWith( { $device_id: truth((val) => val.match(/^[0-9a-f-]+$/)), @@ -804,8 +773,6 @@ describe('posthog core', () => { it('does not set distinct_id/$device_id if distinct_id is unset', () => { given('distinct_id', () => 'existing-id') - given.subject() - expect(given.lib.register_once).not.toHaveBeenCalled() }) @@ -815,8 +782,6 @@ describe('posthog core', () => { get_device_id: (uuid) => 'custom-' + uuid.slice(0, 8), })) - given.subject() - expect(given.lib.register_once).toHaveBeenCalledWith( { $device_id: truth((val) => val.match(/^custom-[0-9a-f]+/)), From c18861b140ce1ee896f78fe75417aa716a98a11e Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 2 Jan 2024 18:39:08 +0100 Subject: [PATCH 14/16] Some fixes --- src/__tests__/posthog-core.js | 24 +++++++++--------------- 1 file changed, 9 insertions(+), 15 deletions(-) diff --git a/src/__tests__/posthog-core.js b/src/__tests__/posthog-core.js index 49e4fbd68..6209096d0 100644 --- a/src/__tests__/posthog-core.js +++ b/src/__tests__/posthog-core.js @@ -761,19 +761,16 @@ describe('posthog core', () => { it('sets a random UUID as distinct_id/$device_id if distinct_id is unset', () => { given('distinct_id', () => undefined) - expect(given.lib.register_once).toHaveBeenCalledWith( - { - $device_id: truth((val) => val.match(/^[0-9a-f-]+$/)), - distinct_id: truth((val) => val.match(/^[0-9a-f-]+$/)), - }, - '' - ) + expect(given.lib.persistence.props).toMatchObject({ + $device_id: truth((val) => val.match(/^[0-9a-f-]+$/)), + distinct_id: truth((val) => val.match(/^[0-9a-f-]+$/)), + }) }) it('does not set distinct_id/$device_id if distinct_id is unset', () => { given('distinct_id', () => 'existing-id') - expect(given.lib.register_once).not.toHaveBeenCalled() + expect(given.lib.persistence.props.distinct_id).not.toEqual('existing-id') }) it('uses config.get_device_id for uuid generation if passed', () => { @@ -782,13 +779,10 @@ describe('posthog core', () => { get_device_id: (uuid) => 'custom-' + uuid.slice(0, 8), })) - expect(given.lib.register_once).toHaveBeenCalledWith( - { - $device_id: truth((val) => val.match(/^custom-[0-9a-f]+/)), - distinct_id: truth((val) => val.match(/^custom-[0-9a-f]+/)), - }, - '' - ) + expect(given.lib.persistence.props).toMatchObject({ + $device_id: truth((val) => val.match(/^custom-[0-9a-f]+/)), + distinct_id: truth((val) => val.match(/^custom-[0-9a-f]+/)), + }) }) }) }) From 85e10a87ad8a90e9fae5ab4f93c4f24b6c5dc1eb Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 2 Jan 2024 19:01:38 +0100 Subject: [PATCH 15/16] Test Fixes --- src/__tests__/posthog-core.js | 32 ++++++++++++++++++++------------ src/posthog-core.ts | 5 +++-- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/src/__tests__/posthog-core.js b/src/__tests__/posthog-core.js index 6209096d0..6a7a8f590 100644 --- a/src/__tests__/posthog-core.js +++ b/src/__tests__/posthog-core.js @@ -3,7 +3,6 @@ import { PostHogPersistence } from '../posthog-persistence' import { Decide } from '../decide' import { autocapture } from '../autocapture' -import { truth } from './helpers/truth' import { _info } from '../utils/event-utils' import { document, window } from '../utils/globals' import { uuidv7 } from '../uuidv7' @@ -14,13 +13,17 @@ jest.mock('../gdpr-utils', () => ({ })) jest.mock('../decide') -given('lib', () => { - const posthog = _posthog.init('testtoken', given.config, uuidv7()) - posthog.debug() - return Object.assign(posthog, given.overrides) -}) - describe('posthog core', () => { + given('lib', () => { + const posthog = _posthog.init('testtoken', given.config, uuidv7()) + posthog.debug() + return Object.assign(posthog, given.overrides) + }) + + afterEach(() => { + // Make sure there's no cached persistence + given.lib.persistence?.clear?.() + }) describe('capture()', () => { given('eventName', () => '$event') @@ -38,7 +41,7 @@ describe('posthog core', () => { given('overrides', () => ({ __loaded: true, - config: given.config, + // config: given.config, persistence: { remove_event_timer: jest.fn(), properties: jest.fn(), @@ -760,11 +763,16 @@ describe('posthog core', () => { describe('device id behavior', () => { it('sets a random UUID as distinct_id/$device_id if distinct_id is unset', () => { given('distinct_id', () => undefined) + given('config', () => ({ + get_device_id: (uuid) => uuid, + })) expect(given.lib.persistence.props).toMatchObject({ - $device_id: truth((val) => val.match(/^[0-9a-f-]+$/)), - distinct_id: truth((val) => val.match(/^[0-9a-f-]+$/)), + $device_id: expect.stringMatching(/^[0-9a-f-]+$/), + distinct_id: expect.stringMatching(/^[0-9a-f-]+$/), }) + + expect(given.lib.persistence.props.$device_id).toEqual(given.lib.persistence.props.distinct_id) }) it('does not set distinct_id/$device_id if distinct_id is unset', () => { @@ -780,8 +788,8 @@ describe('posthog core', () => { })) expect(given.lib.persistence.props).toMatchObject({ - $device_id: truth((val) => val.match(/^custom-[0-9a-f]+/)), - distinct_id: truth((val) => val.match(/^custom-[0-9a-f]+/)), + $device_id: expect.stringMatching(/^custom-[0-9a-f-]+$/), + distinct_id: expect.stringMatching(/^custom-[0-9a-f-]+$/), }) }) }) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 9456ab8f2..416018b8c 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -427,6 +427,7 @@ export class PostHog { // or the device id if something was already stored // in the persitence const uuid = this.config.get_device_id(uuidv7()) + this.register_once( { distinct_id: uuid, @@ -1717,7 +1718,7 @@ export class PostHog { * @param {String} property_name The name of the super property you want to retrieve */ get_property(property_name: string): Property | undefined { - return this.persistence?.['props'][property_name] + return this.persistence?.props[property_name] } /** @@ -1740,7 +1741,7 @@ export class PostHog { * @param {String} property_name The name of the session super property you want to retrieve */ getSessionProperty(property_name: string): Property | undefined { - return this.sessionPersistence?.['props'][property_name] + return this.sessionPersistence?.props[property_name] } toString(): string { From 923514aa5619bef0e10af55dea7fe6cebab6c398 Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 2 Jan 2024 19:14:05 +0100 Subject: [PATCH 16/16] Almost there --- src/__tests__/posthog-core.js | 1 - 1 file changed, 1 deletion(-) diff --git a/src/__tests__/posthog-core.js b/src/__tests__/posthog-core.js index 6a7a8f590..db8694f61 100644 --- a/src/__tests__/posthog-core.js +++ b/src/__tests__/posthog-core.js @@ -41,7 +41,6 @@ describe('posthog core', () => { given('overrides', () => ({ __loaded: true, - // config: given.config, persistence: { remove_event_timer: jest.fn(), properties: jest.fn(),