From 277ed18244ab8aba9c7a52bf608cff62f2e9a1a5 Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 28 Nov 2024 18:44:44 +0100 Subject: [PATCH 01/20] Fixes --- playground/nextjs/pages/_app.tsx | 7 +++---- playground/nextjs/pages/_document.tsx | 12 +++++++++++- playground/nextjs/pages/survey.tsx | 2 +- playground/nextjs/src/posthog.ts | 10 ++++++++-- src/site-apps.ts | 12 ++++++++++++ src/utils/globals.ts | 2 ++ 6 files changed, 37 insertions(+), 8 deletions(-) diff --git a/playground/nextjs/pages/_app.tsx b/playground/nextjs/pages/_app.tsx index 46784b661..3d74c2fe6 100644 --- a/playground/nextjs/pages/_app.tsx +++ b/playground/nextjs/pages/_app.tsx @@ -4,10 +4,9 @@ import { useEffect } from 'react' import type { AppProps } from 'next/app' import { useRouter } from 'next/router' -import posthog from 'posthog-js' import { PostHogProvider } from 'posthog-js/react' import { CookieBanner } from '@/src/CookieBanner' -import '@/src/posthog' +import { posthog } from '@/src/posthog' import Head from 'next/head' import { PageHeader } from '@/src/Header' import { useUser } from '@/src/auth' @@ -46,10 +45,10 @@ export default function App({ Component, pageProps }: AppProps) { http-equiv="Content-Security-Policy" content={` default-src 'self'; - connect-src 'self' ${localhostDomain} https://*.posthog.com; + connect-src 'self' ${localhostDomain} https://*.posthog.com https://lottie.host; script-src 'self' 'unsafe-eval' 'unsafe-inline' ${localhostDomain} https://*.posthog.com; style-src 'self' 'unsafe-inline' ${localhostDomain} https://*.posthog.com; - img-src 'self' ${localhostDomain} https://*.posthog.com; + img-src 'self' ${localhostDomain} https://*.posthog.com https://lottie.host; `} /> diff --git a/playground/nextjs/pages/_document.tsx b/playground/nextjs/pages/_document.tsx index d586b3595..bdc7be58b 100644 --- a/playground/nextjs/pages/_document.tsx +++ b/playground/nextjs/pages/_document.tsx @@ -1,10 +1,20 @@ +import { POSTHOG_USE_SNIPPET } from '@/src/posthog' import { Html, Head, Main, NextScript } from 'next/document' +import Script from 'next/script' import React from 'react' export default function Document() { return ( - + + {POSTHOG_USE_SNIPPET ? ( + + ) : null} +
diff --git a/playground/nextjs/pages/survey.tsx b/playground/nextjs/pages/survey.tsx index 3587985e1..e7049b08d 100644 --- a/playground/nextjs/pages/survey.tsx +++ b/playground/nextjs/pages/survey.tsx @@ -1,6 +1,6 @@ import { usePostHog } from 'posthog-js/react' import { useEffect, useState } from 'react' -import { Survey } from 'posthog-js' +import type { Survey } from 'posthog-js' export default function SurveyForm() { const posthog = usePostHog() diff --git a/playground/nextjs/src/posthog.ts b/playground/nextjs/src/posthog.ts index 6a4062a34..11a656e24 100644 --- a/playground/nextjs/src/posthog.ts +++ b/playground/nextjs/src/posthog.ts @@ -4,12 +4,19 @@ // import 'posthog-js/dist/exception-autocapture' // import 'posthog-js/dist/tracing-headers' -import posthog, { PostHogConfig } from 'posthog-js' +import posthogJS, { PostHogConfig } from 'posthog-js' import { User } from './auth' export const PERSON_PROCESSING_MODE: 'always' | 'identified_only' | 'never' = (process.env.NEXT_PUBLIC_POSTHOG_PERSON_PROCESSING_MODE as any) || 'identified_only' +export const POSTHOG_USE_SNIPPET: boolean = (process.env.NEXT_PUBLIC_POSTHOG_USE_SNIPPET as any) || false + +export const posthog = POSTHOG_USE_SNIPPET + ? typeof window !== 'undefined' + ? (window as any).posthog + : null + : posthogJS /** * Below is an example of a consent-driven config for PostHog * Lots of things start in a disabled state and posthog will not use cookies without consent @@ -57,7 +64,6 @@ if (typeof window !== 'undefined') { persistence_name: `${process.env.NEXT_PUBLIC_POSTHOG_KEY}_nextjs`, ...configForConsent(), }) - // Help with debugging(window as any).posthog = posthog } diff --git a/src/site-apps.ts b/src/site-apps.ts index 73414ace1..4af7565f5 100644 --- a/src/site-apps.ts +++ b/src/site-apps.ts @@ -75,6 +75,18 @@ export class SiteApps { } afterDecideResponse(response?: DecideResponse): void { + if (assignableWindow._POSTHOG_SITE_APPS) { + // Loaded via new config so we have the apps preloaded + if (this.instance.config.opt_in_site_apps) { + assignableWindow._POSTHOG_SITE_APPS.forEach((app) => { + app.load(this.instance) + }) + } + this.loaded = true + + return + } + if (isArray(response?.siteApps) && response.siteApps.length > 0) { if (this.enabled && this.instance.config.opt_in_site_apps) { const checkIfAllLoaded = () => { diff --git a/src/utils/globals.ts b/src/utils/globals.ts index 377fe22b5..be12ade58 100644 --- a/src/utils/globals.ts +++ b/src/utils/globals.ts @@ -20,6 +20,8 @@ export type AssignableWindow = Window & typeof globalThis & Record & { __PosthogExtensions__?: PostHogExtensions + _POSTHOG_CONFIG?: Record // TODO: Better typing + _POSTHOG_SITE_APPS?: { token: string; load: (posthog: PostHog) => void }[] } /** From 57d6b77d8b6d236469795fa0ad07bde65086e850 Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 3 Dec 2024 13:22:38 +0100 Subject: [PATCH 02/20] Started adding alternative loader --- src/autocapture.ts | 4 +- src/decide.ts | 73 ++++++++++++++++++- src/entrypoints/external-scripts-loader.ts | 4 + src/extensions/dead-clicks-autocapture.ts | 4 +- src/extensions/exception-autocapture/index.ts | 4 +- src/extensions/replay/sessionrecording.ts | 8 +- src/extensions/web-vitals/index.ts | 4 +- src/heatmaps.ts | 4 +- src/posthog-core.ts | 25 +++++-- src/posthog-surveys.ts | 4 +- src/site-apps.ts | 4 +- src/types.ts | 18 ++++- src/utils/globals.ts | 5 +- src/web-experiments.ts | 4 +- 14 files changed, 128 insertions(+), 37 deletions(-) diff --git a/src/autocapture.ts b/src/autocapture.ts index a4cb4078f..59feb9172 100644 --- a/src/autocapture.ts +++ b/src/autocapture.ts @@ -15,7 +15,7 @@ import { splitClassString, } from './autocapture-utils' import RageClick from './extensions/rageclick' -import { AutocaptureConfig, COPY_AUTOCAPTURE_EVENT, DecideResponse, EventName, Properties } from './types' +import { AutocaptureConfig, COPY_AUTOCAPTURE_EVENT, EventName, Properties, RemoteConfig } from './types' import { PostHog } from './posthog-core' import { AUTOCAPTURE_DISABLED_SERVER_SIDE } from './constants' @@ -286,7 +286,7 @@ export class Autocapture { } } - public afterDecideResponse(response: DecideResponse) { + public onRemoteConfig(response: RemoteConfig) { if (response.elementsChainAsString) { this._elementsChainAsString = response.elementsChainAsString } diff --git a/src/decide.ts b/src/decide.ts index 88c7ab92d..b82ae5e60 100644 --- a/src/decide.ts +++ b/src/decide.ts @@ -1,9 +1,19 @@ import { PostHog } from './posthog-core' -import { Compression, DecideResponse } from './types' +import { Compression, DecideResponse, RemoteConfig } from './types' import { STORED_GROUP_PROPERTIES_KEY, STORED_PERSON_PROPERTIES_KEY } from './constants' import { logger } from './utils/logger' -import { document } from './utils/globals' +import { assignableWindow, document } from './utils/globals' + +// TODO: Add check for global config existing. +// Modify the whole "afterDecideResponse" function to be a method of the PostHog class + +// 1. Option is to load the config endpoint (if __preview is enabled) and then if flags are needed call decide, and then call "afterDecideResponse" with the combined response :thinking: +// 2. Other option is to separate out the values so that we have a "config response" and a "flags response" separately... + +// TODO: Fix WebExperiments to wait for flags instead of only decide +// TODO: Fix Surveys to do the same +// TODO: Background refresh of the config - every 5 minutes to match CDN cache - at least when active export class Decide { constructor(private readonly instance: PostHog) { @@ -11,7 +21,48 @@ export class Decide { this.instance.decideEndpointWasHit = this.instance._hasBootstrappedFeatureFlags() } + private _loadRemoteConfigJs(cb: (config?: RemoteConfig) => void): void { + if (assignableWindow.__PosthogExtensions__?.loadExternalDependency) { + assignableWindow.__PosthogExtensions__?.loadExternalDependency?.(this.instance, 'remote-config', () => { + return cb(assignableWindow._POSTHOG_CONFIG) + }) + } + } + + private _loadRemoteConfigJSON(cb: (config?: RemoteConfig) => void): void { + this.instance._send_request({ + method: 'GET', + url: this.instance.requestRouter.endpointFor('assets', `/array/${this.instance.config.token}/config`), + callback: (response) => { + cb(response.json as RemoteConfig | undefined) + }, + }) + } + call(): void { + if (this.instance.config.__preview_remote_config) { + // Attempt 1 - use the pre-loaded config if it came as part of the token-specific array.js + if (assignableWindow._POSTHOG_CONFIG) { + this.onRemoteConfig(assignableWindow._POSTHOG_CONFIG) + return + } + + // Attempt 2 - if we have the external deps loader then lets load the script version of the config that includes site apps + this._loadRemoteConfigJs((config) => { + if (!config) { + // Attempt 3 Load the config json instead of the script - we won't get site apps etc. but we will get the config + this._loadRemoteConfigJSON((config) => { + this.onRemoteConfig(config) + }) + return + } + + this.onRemoteConfig(config) + }) + + return + } + /* Calls /decide endpoint to fetch options for autocapture, session recording, feature flags & compression. */ @@ -65,4 +116,22 @@ export class Decide { this.instance._afterDecideResponse(response) } + + private onRemoteConfig(config?: RemoteConfig): void { + if (!config) { + logger.error('Failed to fetch remote config from PostHog.') + return + } + if (!(document && document.body)) { + logger.info('document not ready yet, trying again in 500 milliseconds...') + setTimeout(() => { + this.onRemoteConfig(config) + }, 500) + return + } + + this.instance._onRemoteConfig(config) + + // Additionally trigger loading of flags if necessary + } } diff --git a/src/entrypoints/external-scripts-loader.ts b/src/entrypoints/external-scripts-loader.ts index 084b47d82..32489b25d 100644 --- a/src/entrypoints/external-scripts-loader.ts +++ b/src/entrypoints/external-scripts-loader.ts @@ -43,6 +43,10 @@ assignableWindow.__PosthogExtensions__.loadExternalDependency = ( ): void => { let scriptUrlToLoad = `/static/${kind}.js` + `?v=${posthog.version}` + if (kind === 'remote-config') { + scriptUrlToLoad = `/array/${posthog.config.token}/config.js` + } + if (kind === 'toolbar') { // toolbar.js is served from the PostHog CDN, this has a TTL of 24 hours. // the toolbar asset includes a rotating "token" that is valid for 5 minutes. diff --git a/src/extensions/dead-clicks-autocapture.ts b/src/extensions/dead-clicks-autocapture.ts index 43aab7b46..1c4d1108f 100644 --- a/src/extensions/dead-clicks-autocapture.ts +++ b/src/extensions/dead-clicks-autocapture.ts @@ -3,7 +3,7 @@ import { DEAD_CLICKS_ENABLED_SERVER_SIDE } from '../constants' import { isBoolean, isObject } from '../utils/type-utils' import { assignableWindow, document, LazyLoadedDeadClicksAutocaptureInterface } from '../utils/globals' import { logger } from '../utils/logger' -import { DeadClicksAutoCaptureConfig, DecideResponse } from '../types' +import { DeadClicksAutoCaptureConfig, RemoteConfig } from '../types' const LOGGER_PREFIX = '[Dead Clicks]' @@ -31,7 +31,7 @@ export class DeadClicksAutocapture { this.startIfEnabled() } - public afterDecideResponse(response: DecideResponse) { + public onRemoteConfig(response: RemoteConfig) { if (this.instance.persistence) { this.instance.persistence.register({ [DEAD_CLICKS_ENABLED_SERVER_SIDE]: response?.captureDeadClicks, diff --git a/src/extensions/exception-autocapture/index.ts b/src/extensions/exception-autocapture/index.ts index 6f278dde9..71e683cb0 100644 --- a/src/extensions/exception-autocapture/index.ts +++ b/src/extensions/exception-autocapture/index.ts @@ -1,6 +1,6 @@ import { assignableWindow, window } from '../../utils/globals' import { PostHog } from '../../posthog-core' -import { DecideResponse, Properties } from '../../types' +import { Properties, RemoteConfig } from '../../types' import { logger } from '../../utils/logger' import { EXCEPTION_CAPTURE_ENABLED_SERVER_SIDE } from '../../constants' @@ -86,7 +86,7 @@ export class ExceptionObserver { this.unwrapUnhandledRejection?.() } - afterDecideResponse(response: DecideResponse) { + onRemoteConfig(response: RemoteConfig) { const autocaptureExceptionsResponse = response.autocaptureExceptions // store this in-memory in case persistence is disabled diff --git a/src/extensions/replay/sessionrecording.ts b/src/extensions/replay/sessionrecording.ts index 195579786..f36cbfea3 100644 --- a/src/extensions/replay/sessionrecording.ts +++ b/src/extensions/replay/sessionrecording.ts @@ -21,11 +21,11 @@ import { import { PostHog } from '../../posthog-core' import { CaptureResult, - DecideResponse, FlagVariant, NetworkRecordOptions, NetworkRequest, Properties, + RemoteConfig, SessionRecordingUrlTrigger, } from '../../types' import { @@ -617,8 +617,8 @@ export class SessionRecording { }) } - afterDecideResponse(response: DecideResponse) { - this._persistDecideResponse(response) + onRemoteConfig(response: RemoteConfig) { + this._persistRemoteConfig(response) this._linkedFlag = response.sessionRecording?.linkedFlag || null @@ -671,7 +671,7 @@ export class SessionRecording { } } - private _persistDecideResponse(response: DecideResponse): void { + private _persistRemoteConfig(response: RemoteConfig): void { if (this.instance.persistence) { const persistence = this.instance.persistence diff --git a/src/extensions/web-vitals/index.ts b/src/extensions/web-vitals/index.ts index 7b9bf1937..ccea6a4df 100644 --- a/src/extensions/web-vitals/index.ts +++ b/src/extensions/web-vitals/index.ts @@ -1,5 +1,5 @@ import { PostHog } from '../../posthog-core' -import { DecideResponse, SupportedWebVitalsMetrics } from '../../types' +import { RemoteConfig, SupportedWebVitalsMetrics } from '../../types' import { logger } from '../../utils/logger' import { isBoolean, isNullish, isNumber, isObject, isUndefined } from '../../utils/type-utils' import { WEB_VITALS_ALLOWED_METRICS, WEB_VITALS_ENABLED_SERVER_SIDE } from '../../constants' @@ -70,7 +70,7 @@ export class WebVitalsAutocapture { } } - public afterDecideResponse(response: DecideResponse) { + public onRemoteConfig(response: RemoteConfig) { const webVitalsOptIn = isObject(response.capturePerformance) && !!response.capturePerformance.web_vitals const allowedMetrics = isObject(response.capturePerformance) diff --git a/src/heatmaps.ts b/src/heatmaps.ts index 5ed1a41ef..e94400220 100644 --- a/src/heatmaps.ts +++ b/src/heatmaps.ts @@ -1,6 +1,6 @@ import { includes, registerEvent } from './utils' import RageClick from './extensions/rageclick' -import { DeadClickCandidate, DecideResponse, Properties } from './types' +import { DeadClickCandidate, Properties, RemoteConfig } from './types' import { PostHog } from './posthog-core' import { document, window } from './utils/globals' @@ -99,7 +99,7 @@ export class Heatmaps { } } - public afterDecideResponse(response: DecideResponse) { + public onRemoteConfig(response: RemoteConfig) { const optIn = !!response['heatmaps'] if (this.instance.persistence) { diff --git a/src/posthog-core.ts b/src/posthog-core.ts index c7594073e..e27f4fd80 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -41,6 +41,7 @@ import { Properties, Property, QueuedRequestOptions, + RemoteConfig, RequestCallback, SessionIdChangedCallback, SnippetArrayItem, @@ -567,15 +568,23 @@ export class PostHog { : 'always', }) - this.siteApps?.afterDecideResponse(response) - this.sessionRecording?.afterDecideResponse(response) - this.autocapture?.afterDecideResponse(response) - this.heatmaps?.afterDecideResponse(response) + this.siteApps?.onRemoteConfig(response) + this.sessionRecording?.onRemoteConfig(response) + this.autocapture?.onRemoteConfig(response) + this.heatmaps?.onRemoteConfig(response) this.experiments?.afterDecideResponse(response) - this.surveys?.afterDecideResponse(response) - this.webVitalsAutocapture?.afterDecideResponse(response) - this.exceptionObserver?.afterDecideResponse(response) - this.deadClicksAutocapture?.afterDecideResponse(response) + this.surveys?.onRemoteConfig(response) + this.webVitalsAutocapture?.onRemoteConfig(response) + this.exceptionObserver?.onRemoteConfig(response) + this.deadClicksAutocapture?.onRemoteConfig(response) + } + + _onRemoteConfig(config: RemoteConfig) { + // TODO: check config. If "hasFlags" is anything other than false - load the flags from decide (later will be /flags) + + if (config.hasFeatureFlags !== false) { + // Check explicitly for false - anything else means we there could be so lets load them + } } _loaded(): void { diff --git a/src/posthog-surveys.ts b/src/posthog-surveys.ts index 4b25ef9bb..9476bb936 100644 --- a/src/posthog-surveys.ts +++ b/src/posthog-surveys.ts @@ -10,7 +10,7 @@ import { import { isUrlMatchingRegex } from './utils/request-utils' import { SurveyEventReceiver } from './utils/survey-event-receiver' import { assignableWindow, document, window } from './utils/globals' -import { DecideResponse } from './types' +import { RemoteConfig } from './types' import { logger } from './utils/logger' import { isNullish } from './utils/type-utils' import { getSurveySeenStorageKeys } from './extensions/surveys/surveys-utils' @@ -69,7 +69,7 @@ export class PostHogSurveys { this._surveyEventReceiver = null } - afterDecideResponse(response: DecideResponse) { + onRemoteConfig(response: RemoteConfig) { this._decideServerResponse = !!response['surveys'] this.loadIfEnabled() } diff --git a/src/site-apps.ts b/src/site-apps.ts index 5424ddd45..9206f8475 100644 --- a/src/site-apps.ts +++ b/src/site-apps.ts @@ -1,5 +1,5 @@ import { PostHog } from './posthog-core' -import { CaptureResult, DecideResponse } from './types' +import { CaptureResult, RemoteConfig } from './types' import { assignableWindow } from './utils/globals' import { logger } from './utils/logger' import { isArray } from './utils/type-utils' @@ -74,7 +74,7 @@ export class SiteApps { return globals } - afterDecideResponse(response?: DecideResponse): void { + onRemoteConfig(response?: RemoteConfig): void { if (assignableWindow._POSTHOG_SITE_APPS) { // Loaded via new config so we have the apps preloaded if (this.instance.config.opt_in_site_apps) { diff --git a/src/types.ts b/src/types.ts index c52b3d6e1..d351ebc2c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -338,6 +338,12 @@ export interface PostHogConfig { * whether to wrap fetch and add tracing headers to the request * */ __add_tracing_headers?: boolean + + /** + * PREVIEW - MAY CHANGE WITHOUT WARNING - DO NOT USE IN PRODUCTION + * whether to wrap fetch and add tracing headers to the request + * */ + __preview_remote_config?: boolean } export interface OptInOutCapturingOptions { @@ -480,11 +486,8 @@ export type SessionRecordingCanvasOptions = { canvasQuality?: string | null } -export interface DecideResponse { +export interface RemoteConfig { supportedCompression: Compression[] - featureFlags: Record - featureFlagPayloads: Record - errorsWhileComputingFlags: boolean autocapture_opt_out?: boolean /** * originally capturePerformance was replay only and so boolean true @@ -528,6 +531,13 @@ export interface DecideResponse { heatmaps?: boolean defaultIdentifiedOnly?: boolean captureDeadClicks?: boolean + hasFeatureFlags?: boolean // Indicates if the team has any flags enabled (if not we don't need to load them) +} + +export interface DecideResponse extends RemoteConfig { + featureFlags: Record + featureFlagPayloads: Record + errorsWhileComputingFlags: boolean } export type FeatureFlagsCallback = ( diff --git a/src/utils/globals.ts b/src/utils/globals.ts index be12ade58..23ca7a1ba 100644 --- a/src/utils/globals.ts +++ b/src/utils/globals.ts @@ -1,7 +1,7 @@ import { ErrorProperties } from '../extensions/exception-autocapture/error-conversion' import type { PostHog } from '../posthog-core' import { SessionIdManager } from '../sessionid' -import { DeadClicksAutoCaptureConfig, ErrorEventArgs, ErrorMetadata, Properties } from '../types' +import { DeadClicksAutoCaptureConfig, ErrorEventArgs, ErrorMetadata, Properties, RemoteConfig } from '../types' /* * Global helpers to protect access to browser globals in a way that is safer for different targets @@ -20,7 +20,7 @@ export type AssignableWindow = Window & typeof globalThis & Record & { __PosthogExtensions__?: PostHogExtensions - _POSTHOG_CONFIG?: Record // TODO: Better typing + _POSTHOG_CONFIG?: RemoteConfig _POSTHOG_SITE_APPS?: { token: string; load: (posthog: PostHog) => void }[] } @@ -37,6 +37,7 @@ export type PostHogExtensionKind = | 'tracing-headers' | 'surveys' | 'dead-clicks-autocapture' + | 'remote-config' export interface LazyLoadedDeadClicksAutocaptureInterface { start: (observerTarget: Node) => void diff --git a/src/web-experiments.ts b/src/web-experiments.ts index 20b4bfd53..ca48ba115 100644 --- a/src/web-experiments.ts +++ b/src/web-experiments.ts @@ -40,9 +40,7 @@ export class WebExperiments { this.applyFeatureFlagChanges(flags) } - if (this.instance.onFeatureFlags) { - this.instance.onFeatureFlags(appFeatureFLags) - } + this.instance.onFeatureFlags(appFeatureFLags) this._flagToExperiments = new Map() } From 5a345c979d0a1b1793806e692e2a75468770830a Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 3 Dec 2024 13:42:05 +0100 Subject: [PATCH 03/20] Renamed all things to the remote config version --- src/__tests__/autocapture.test.ts | 22 +++--- .../exception-observer.test.ts | 6 +- .../replay/sessionrecording.test.ts | 68 +++++++++---------- src/__tests__/extensions/web-vitals.test.ts | 4 +- src/__tests__/heatmaps.test.ts | 2 +- src/__tests__/posthog-core.ts | 2 +- src/__tests__/site-apps.ts | 14 ++-- src/__tests__/web-experiments.test.ts | 6 +- src/decide.ts | 1 + src/posthog-core.ts | 2 +- src/web-experiments.ts | 2 +- 11 files changed, 66 insertions(+), 63 deletions(-) diff --git a/src/__tests__/autocapture.test.ts b/src/__tests__/autocapture.test.ts index 1cf4ebb2d..a2e8039f7 100644 --- a/src/__tests__/autocapture.test.ts +++ b/src/__tests__/autocapture.test.ts @@ -380,7 +380,7 @@ describe('Autocapture system', () => { beforeEach(() => { posthog.config.rageclick = true // Trigger proper enabling - autocapture.afterDecideResponse({} as DecideResponse) + autocapture.onRemoteConfig({} as DecideResponse) }) it('should capture rageclick', () => { @@ -502,7 +502,7 @@ describe('Autocapture system', () => { it('should not capture events when config returns false, when an element matching any of the event selectors is clicked', () => { posthog.config.autocapture = false - autocapture.afterDecideResponse({} as DecideResponse) + autocapture.onRemoteConfig({} as DecideResponse) const eventElement1 = document.createElement('div') const eventElement2 = document.createElement('div') @@ -524,7 +524,7 @@ describe('Autocapture system', () => { }) it('should not capture events when config returns true but server setting is disabled', () => { - autocapture.afterDecideResponse({ + autocapture.onRemoteConfig({ autocapture_opt_out: true, } as DecideResponse) @@ -932,7 +932,7 @@ describe('Autocapture system', () => { type: 'click', } as unknown as MouseEvent - autocapture.afterDecideResponse({ + autocapture.onRemoteConfig({ elementsChainAsString: true, } as DecideResponse) @@ -1003,7 +1003,7 @@ describe('Autocapture system', () => { beforeEach(() => { document.title = 'test page' posthog.config.mask_all_element_attributes = false - autocapture.afterDecideResponse({} as DecideResponse) + autocapture.onRemoteConfig({} as DecideResponse) }) it('should capture click events', () => { @@ -1056,7 +1056,7 @@ describe('Autocapture system', () => { 'when client side config is %p and remote opt out is %p - autocapture enabled should be %p', (clientSideOptIn, serverSideOptOut, expected) => { posthog.config.autocapture = clientSideOptIn - autocapture.afterDecideResponse({ + autocapture.onRemoteConfig({ autocapture_opt_out: serverSideOptOut, } as DecideResponse) expect(autocapture.isEnabled).toBe(expected) @@ -1065,12 +1065,12 @@ describe('Autocapture system', () => { it('should call _addDomEventHandlders if autocapture is true in client config', () => { posthog.config.autocapture = true - autocapture.afterDecideResponse({} as DecideResponse) + autocapture.onRemoteConfig({} as DecideResponse) expect(autocapture['_addDomEventHandlers']).toHaveBeenCalled() }) it('should not call _addDomEventHandlders if autocapture is opted out in server config', () => { - autocapture.afterDecideResponse({ autocapture_opt_out: true } as DecideResponse) + autocapture.onRemoteConfig({ autocapture_opt_out: true } as DecideResponse) expect(autocapture['_addDomEventHandlers']).not.toHaveBeenCalled() }) @@ -1078,16 +1078,16 @@ describe('Autocapture system', () => { expect(autocapture['_addDomEventHandlers']).not.toHaveBeenCalled() posthog.config.autocapture = false - autocapture.afterDecideResponse({} as DecideResponse) + autocapture.onRemoteConfig({} as DecideResponse) expect(autocapture['_addDomEventHandlers']).not.toHaveBeenCalled() }) it('should NOT call _addDomEventHandlders when the token has already been initialized', () => { - autocapture.afterDecideResponse({} as DecideResponse) + autocapture.onRemoteConfig({} as DecideResponse) expect(autocapture['_addDomEventHandlers']).toHaveBeenCalledTimes(1) - autocapture.afterDecideResponse({} as DecideResponse) + autocapture.onRemoteConfig({} as DecideResponse) expect(autocapture['_addDomEventHandlers']).toHaveBeenCalledTimes(1) }) }) diff --git a/src/__tests__/extensions/exception-autocapture/exception-observer.test.ts b/src/__tests__/extensions/exception-autocapture/exception-observer.test.ts index ad934e2b1..654a8da25 100644 --- a/src/__tests__/extensions/exception-autocapture/exception-observer.test.ts +++ b/src/__tests__/extensions/exception-autocapture/exception-observer.test.ts @@ -67,7 +67,7 @@ describe('Exception Observer', () => { describe('when enabled', () => { beforeEach(() => { - exceptionObserver.afterDecideResponse({ autocaptureExceptions: true } as DecideResponse) + exceptionObserver.onRemoteConfig({ autocaptureExceptions: true } as DecideResponse) }) it('should instrument handlers when started', () => { @@ -173,7 +173,7 @@ describe('Exception Observer', () => { window!.onerror = originalOnError window!.onunhandledrejection = originalOnUnhandledRejection - exceptionObserver.afterDecideResponse({ autocaptureExceptions: true } as DecideResponse) + exceptionObserver.onRemoteConfig({ autocaptureExceptions: true } as DecideResponse) }) it('should wrap original onerror handler if one was present when wrapped', () => { @@ -232,7 +232,7 @@ describe('Exception Observer', () => { describe('when disabled', () => { beforeEach(() => { - exceptionObserver.afterDecideResponse({ autocaptureExceptions: false } as DecideResponse) + exceptionObserver.onRemoteConfig({ autocaptureExceptions: false } as DecideResponse) }) it('cannot be started', () => { diff --git a/src/__tests__/extensions/replay/sessionrecording.test.ts b/src/__tests__/extensions/replay/sessionrecording.test.ts index d786d56e3..a0c676588 100644 --- a/src/__tests__/extensions/replay/sessionrecording.test.ts +++ b/src/__tests__/extensions/replay/sessionrecording.test.ts @@ -486,7 +486,7 @@ describe('SessionRecording', () => { }) it('loads script based on script config', () => { - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', scriptConfig: { script: 'experimental-recorder' } }, }) @@ -553,7 +553,7 @@ describe('SessionRecording', () => { windowId: 'windowId', }) - sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: undefined })) + sessionRecording.onRemoteConfig(makeDecideResponse({ sessionRecording: undefined })) expect(sessionRecording['status']).toBe('disabled') expect(sessionRecording['buffer'].data.length).toEqual(0) expect(posthog.capture).not.toHaveBeenCalled() @@ -564,7 +564,7 @@ describe('SessionRecording', () => { expect(loadScriptMock).toHaveBeenCalled() expect(sessionRecording['status']).toBe('buffering') - sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) + sessionRecording.onRemoteConfig(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) expect(sessionRecording['status']).toBe('active') }) @@ -573,14 +573,14 @@ describe('SessionRecording', () => { expect(loadScriptMock).toHaveBeenCalled() expect(sessionRecording['isSampled']).toBe(null) - sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) + sessionRecording.onRemoteConfig(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) expect(sessionRecording['isSampled']).toBe(null) }) it('stores true in persistence if recording is enabled from the server', () => { posthog.persistence?.register({ [SESSION_RECORDING_ENABLED_SERVER_SIDE]: undefined }) - sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) + sessionRecording.onRemoteConfig(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) expect(posthog.get_property(SESSION_RECORDING_ENABLED_SERVER_SIDE)).toBe(true) }) @@ -588,7 +588,7 @@ describe('SessionRecording', () => { it('stores true in persistence if canvas is enabled from the server', () => { posthog.persistence?.register({ [SESSION_RECORDING_CANVAS_RECORDING]: undefined }) - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', recordCanvas: true, canvasFps: 6, canvasQuality: '0.2' }, }) @@ -604,7 +604,7 @@ describe('SessionRecording', () => { it('stores false in persistence if recording is not enabled from the server', () => { posthog.persistence?.register({ [SESSION_RECORDING_ENABLED_SERVER_SIDE]: undefined }) - sessionRecording.afterDecideResponse(makeDecideResponse({})) + sessionRecording.onRemoteConfig(makeDecideResponse({})) expect(posthog.get_property(SESSION_RECORDING_ENABLED_SERVER_SIDE)).toBe(false) }) @@ -612,7 +612,7 @@ describe('SessionRecording', () => { it('stores sample rate', () => { posthog.persistence?.register({ SESSION_RECORDING_SAMPLE_RATE: undefined }) - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', sampleRate: '0.70' }, }) @@ -624,7 +624,7 @@ describe('SessionRecording', () => { it('starts session recording, saves setting and endpoint when enabled', () => { posthog.persistence?.register({ [SESSION_RECORDING_ENABLED_SERVER_SIDE]: undefined }) - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/ses/' }, }) @@ -642,7 +642,7 @@ describe('SessionRecording', () => { it('does not emit to capture if the sample rate is 0', () => { sessionRecording.startIfEnabledOrStop() - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', sampleRate: '0.00' }, }) @@ -657,7 +657,7 @@ describe('SessionRecording', () => { it('does emit to capture if the sample rate is null', () => { sessionRecording.startIfEnabledOrStop() - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', sampleRate: null }, }) @@ -669,7 +669,7 @@ describe('SessionRecording', () => { it('stores excluded session when excluded', () => { sessionRecording.startIfEnabledOrStop() - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', sampleRate: '0.00' }, }) @@ -684,7 +684,7 @@ describe('SessionRecording', () => { _emit(createIncrementalSnapshot({ data: { source: 1 } })) expect(posthog.capture).not.toHaveBeenCalled() - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', sampleRate: '1.00' }, }) @@ -704,7 +704,7 @@ describe('SessionRecording', () => { it('sets emit as expected when sample rate is 0.5', () => { sessionRecording.startIfEnabledOrStop() - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', sampleRate: '0.50' }, }) @@ -758,7 +758,7 @@ describe('SessionRecording', () => { it('skips when any config variable is missing', () => { sessionRecording.startIfEnabledOrStop() - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', recordCanvas: null, canvasFps: null, canvasQuality: null }, }) @@ -837,7 +837,7 @@ describe('SessionRecording', () => { windowId: 'windowId', }) - sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) + sessionRecording.onRemoteConfig(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) // next call to emit won't flush the buffer // the events aren't big enough @@ -869,7 +869,7 @@ describe('SessionRecording', () => { }) it('buffers emitted events', () => { - sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) + sessionRecording.onRemoteConfig(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) sessionRecording.startIfEnabledOrStop() expect(loadScriptMock).toHaveBeenCalled() @@ -904,7 +904,7 @@ describe('SessionRecording', () => { }) it('flushes buffer if the size of the buffer hits the limit', () => { - sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) + sessionRecording.onRemoteConfig(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) sessionRecording.startIfEnabledOrStop() expect(loadScriptMock).toHaveBeenCalled() const bigData = 'a'.repeat(RECORDING_MAX_EVENT_SIZE * 0.8) @@ -949,7 +949,7 @@ describe('SessionRecording', () => { }) it('flushes buffer if the session_id changes', () => { - sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) + sessionRecording.onRemoteConfig(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) sessionRecording.startIfEnabledOrStop() expect(sessionRecording['buffer'].sessionId).toEqual(sessionId) @@ -1061,7 +1061,7 @@ describe('SessionRecording', () => { it('can emit when there are circular references', () => { posthog.config.session_recording.compress_events = false - sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) + sessionRecording.onRemoteConfig(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) sessionRecording.startIfEnabledOrStop() const someObject = { emit: 1 } @@ -1362,7 +1362,7 @@ describe('SessionRecording', () => { beforeEach(() => { sessionRecording.startIfEnabledOrStop() - sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) + sessionRecording.onRemoteConfig(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) expect(sessionRecording['status']).toEqual('active') startingTimestamp = sessionRecording['_lastActivityTimestamp'] @@ -1718,7 +1718,7 @@ describe('SessionRecording', () => { expect(sessionRecording['_linkedFlag']).toEqual(null) expect(sessionRecording['_linkedFlagSeen']).toEqual(false) - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', linkedFlag: 'the-flag-key' } }) ) @@ -1741,7 +1741,7 @@ describe('SessionRecording', () => { expect(sessionRecording['_linkedFlag']).toEqual(null) expect(sessionRecording['_linkedFlagSeen']).toEqual(false) - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', linkedFlag: { flag: 'the-flag-key', variant: 'test-a' } }, }) @@ -1766,7 +1766,7 @@ describe('SessionRecording', () => { expect(sessionRecording['_linkedFlag']).toEqual(null) expect(sessionRecording['_linkedFlagSeen']).toEqual(false) - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', linkedFlag: 'the-flag-key' } }) ) @@ -1793,7 +1793,7 @@ describe('SessionRecording', () => { expect(sessionRecording['_linkedFlagSeen']).toEqual(false) expect(sessionRecording['status']).toEqual('buffering') - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', @@ -1870,7 +1870,7 @@ describe('SessionRecording', () => { }) it('can set minimum duration from decide response', () => { - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { minimumDurationMilliseconds: 1500 }, }) @@ -1879,7 +1879,7 @@ describe('SessionRecording', () => { }) it('does not flush if below the minimum duration', () => { - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { minimumDurationMilliseconds: 1500 }, }) @@ -1899,7 +1899,7 @@ describe('SessionRecording', () => { }) it('does flush if session duration is negative', () => { - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { minimumDurationMilliseconds: 1500 }, }) @@ -1924,7 +1924,7 @@ describe('SessionRecording', () => { }) it('does not stay buffering after the minimum duration', () => { - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { minimumDurationMilliseconds: 1500 }, }) @@ -1970,7 +1970,7 @@ describe('SessionRecording', () => { }) sessionRecording = new SessionRecording(posthog) - sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) + sessionRecording.onRemoteConfig(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) sessionRecording.startIfEnabledOrStop() expect(loadScriptMock).toHaveBeenCalled() @@ -2089,7 +2089,7 @@ describe('SessionRecording', () => { beforeEach(() => { posthog.config.session_recording.compress_events = true - sessionRecording.afterDecideResponse(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) + sessionRecording.onRemoteConfig(makeDecideResponse({ sessionRecording: { endpoint: '/s/' } })) sessionRecording.startIfEnabledOrStop() }) @@ -2271,7 +2271,7 @@ describe('SessionRecording', () => { describe('URL blocking', () => { beforeEach(() => { sessionRecording.startIfEnabledOrStop() - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', @@ -2343,7 +2343,7 @@ describe('SessionRecording', () => { }) it('flushes buffer and starts when sees event', async () => { - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', @@ -2371,7 +2371,7 @@ describe('SessionRecording', () => { }) it('starts if sees an event but still waiting for a URL', async () => { - sessionRecording.afterDecideResponse( + sessionRecording.onRemoteConfig( makeDecideResponse({ sessionRecording: { endpoint: '/s/', diff --git a/src/__tests__/extensions/web-vitals.test.ts b/src/__tests__/extensions/web-vitals.test.ts index 5cd284a13..b28aeb125 100644 --- a/src/__tests__/extensions/web-vitals.test.ts +++ b/src/__tests__/extensions/web-vitals.test.ts @@ -113,7 +113,7 @@ describe('web vitals', () => { assignableWindow.__PosthogExtensions__.loadExternalDependency = loadScriptMock // need to force this to get the web vitals script loaded - posthog.webVitalsAutocapture!.afterDecideResponse({ + posthog.webVitalsAutocapture!.onRemoteConfig({ capturePerformance: { web_vitals: true }, } as unknown as DecideResponse) @@ -244,7 +244,7 @@ describe('web vitals', () => { 'when client side config is %p and remote opt in is %p - web vitals enabled should be %p', (clientSideOptIn, serverSideOptIn, expected) => { posthog.config.capture_performance = { web_vitals: clientSideOptIn } - posthog.webVitalsAutocapture!.afterDecideResponse({ + posthog.webVitalsAutocapture!.onRemoteConfig({ capturePerformance: { web_vitals: serverSideOptIn }, } as DecideResponse) expect(posthog.webVitalsAutocapture!.isEnabled).toBe(expected) diff --git a/src/__tests__/heatmaps.test.ts b/src/__tests__/heatmaps.test.ts index 0f1588d2c..5168c8a37 100644 --- a/src/__tests__/heatmaps.test.ts +++ b/src/__tests__/heatmaps.test.ts @@ -217,7 +217,7 @@ describe('heatmaps', () => { (deprecatedclientSideOptIn, clientSideOptIn, serverSideOptIn, expected) => { posthog.config.enable_heatmaps = deprecatedclientSideOptIn posthog.config.capture_heatmaps = clientSideOptIn - posthog.heatmaps!.afterDecideResponse({ + posthog.heatmaps!.onRemoteConfig({ heatmaps: serverSideOptIn, } as DecideResponse) expect(posthog.heatmaps!.isEnabled).toBe(expected) diff --git a/src/__tests__/posthog-core.ts b/src/__tests__/posthog-core.ts index b3f2c1c9c..9161fee2c 100644 --- a/src/__tests__/posthog-core.ts +++ b/src/__tests__/posthog-core.ts @@ -842,7 +842,7 @@ describe('posthog core', () => { expect(posthog.persistence.register).not.toHaveBeenCalled() // FFs are saved this way // Session recording - expect(posthog.sessionRecording.afterDecideResponse).not.toHaveBeenCalled() + expect(posthog.sessionRecording.onRemoteConfig).not.toHaveBeenCalled() }) describe('device id behavior', () => { diff --git a/src/__tests__/site-apps.ts b/src/__tests__/site-apps.ts index 7964a7e17..aea6407a5 100644 --- a/src/__tests__/site-apps.ts +++ b/src/__tests__/site-apps.ts @@ -224,7 +224,7 @@ describe('SiteApps', () => { describe('afterDecideResponse', () => { it('sets loaded to true and enabled to false when response is undefined', () => { - siteAppsInstance.afterDecideResponse(undefined) + siteAppsInstance.onRemoteConfig(undefined) expect(siteAppsInstance.loaded).toBe(true) expect(siteAppsInstance.enabled).toBe(false) @@ -240,7 +240,7 @@ describe('SiteApps', () => { ], } as DecideResponse - siteAppsInstance.afterDecideResponse(response) + siteAppsInstance.onRemoteConfig(response) expect(siteAppsInstance.appsLoading.size).toBe(2) expect(siteAppsInstance.loaded).toBe(false) @@ -261,7 +261,7 @@ describe('SiteApps', () => { siteApps: [{ id: '1', url: '/site_app/1' }], } as DecideResponse - siteAppsInstance.afterDecideResponse(response) + siteAppsInstance.onRemoteConfig(response) expect(siteAppsInstance.loaded).toBe(true) expect(siteAppsInstance.enabled).toBe(false) @@ -276,7 +276,7 @@ describe('SiteApps', () => { siteApps: [{ id: '1', url: '/site_app/1' }], } as DecideResponse - siteAppsInstance.afterDecideResponse(response) + siteAppsInstance.onRemoteConfig(response) // Wait for the simulated async loading to complete setTimeout(() => { @@ -293,7 +293,7 @@ describe('SiteApps', () => { siteApps: [{ id: '1', url: '/site_app/1' }], } as DecideResponse - siteAppsInstance.afterDecideResponse(response) + siteAppsInstance.onRemoteConfig(response) expect(assignableWindow['__$$ph_site_app_1']).toBe(posthog) expect(typeof assignableWindow['__$$ph_site_app_1_missed_invocations']).toBe('function') @@ -312,7 +312,7 @@ describe('SiteApps', () => { siteApps: [{ id: '1', url: '/site_app/1' }], } as DecideResponse - siteAppsInstance.afterDecideResponse(response) + siteAppsInstance.onRemoteConfig(response) expect(logger.error).toHaveBeenCalledWith( 'PostHog site apps are disabled. Enable the "opt_in_site_apps" config to proceed.' @@ -327,7 +327,7 @@ describe('SiteApps', () => { siteApps: [], } as DecideResponse - siteAppsInstance.afterDecideResponse(response) + siteAppsInstance.onRemoteConfig(response) expect(siteAppsInstance.loaded).toBe(true) expect(siteAppsInstance.enabled).toBe(false) diff --git a/src/__tests__/web-experiments.test.ts b/src/__tests__/web-experiments.test.ts index 8e9ace341..2e5df4f11 100644 --- a/src/__tests__/web-experiments.test.ts +++ b/src/__tests__/web-experiments.test.ts @@ -128,6 +128,7 @@ describe('Web Experimentation', () => { .fn() .mockImplementation(({ callback }) => callback({ statusCode: 200, json: experimentsResponse })), consent: { isOptedOut: () => true } as unknown as ConsentManager, + onFeatureFlags: jest.fn(), }) posthog.requestRouter = new RequestRouter(posthog) @@ -170,7 +171,7 @@ describe('Web Experimentation', () => { function assertElementChanged(variant: string, expectedProperty: string, value: string) { const elParent = createTestDocument() webExperiment = new WebExperiments(posthog) - webExperiment.afterDecideResponse({ + webExperiment.onRemoteConfig({ featureFlags: { 'signup-button-test': variant, }, @@ -200,7 +201,7 @@ describe('Web Experimentation', () => { webExperiment._is_bot = () => true const elParent = createTestDocument() - webExperiment.afterDecideResponse({ + webExperiment.onRemoteConfig({ featureFlags: { 'signup-button-test': 'Sign me up', }, @@ -262,6 +263,7 @@ describe('Web Experimentation', () => { .fn() .mockImplementation(({ callback }) => callback({ statusCode: 200, json: expResponse })), consent: { isOptedOut: () => true } as unknown as ConsentManager, + onFeatureFlags: jest.fn(), }) posthog.requestRouter = new RequestRouter(disabledPostHog) diff --git a/src/decide.ts b/src/decide.ts index b82ae5e60..9a84b4d6a 100644 --- a/src/decide.ts +++ b/src/decide.ts @@ -118,6 +118,7 @@ export class Decide { } private onRemoteConfig(config?: RemoteConfig): void { + // NOTE: Once this is rolled out we will remove the "decide" related code above. Until then the code duplication is fine. if (!config) { logger.error('Failed to fetch remote config from PostHog.') return diff --git a/src/posthog-core.ts b/src/posthog-core.ts index e27f4fd80..e9ce4ed5e 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -572,7 +572,7 @@ export class PostHog { this.sessionRecording?.onRemoteConfig(response) this.autocapture?.onRemoteConfig(response) this.heatmaps?.onRemoteConfig(response) - this.experiments?.afterDecideResponse(response) + this.experiments?.onRemoteConfig(response) this.surveys?.onRemoteConfig(response) this.webVitalsAutocapture?.onRemoteConfig(response) this.exceptionObserver?.onRemoteConfig(response) diff --git a/src/web-experiments.ts b/src/web-experiments.ts index ca48ba115..7a15762e9 100644 --- a/src/web-experiments.ts +++ b/src/web-experiments.ts @@ -65,7 +65,7 @@ export class WebExperiments { }) } - afterDecideResponse(response: DecideResponse) { + onRemoteConfig(response: DecideResponse) { if (this._is_bot()) { WebExperiments.logInfo('Refusing to render web experiment since the viewer is a likely bot') return From c32f364a1acfa9c1cfb16ef8fd4de90eb0136915 Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 3 Dec 2024 14:21:34 +0100 Subject: [PATCH 04/20] Fixes --- rollup.config.js | 4 ++- src/__tests__/decide.ts | 8 +++--- src/__tests__/personProcessing.test.ts | 8 +++--- src/__tests__/posthog-core.loaded.ts | 1 + src/__tests__/posthog-core.ts | 22 +++++++------- src/decide.ts | 2 +- src/posthog-core.ts | 40 ++++++++++++-------------- src/web-experiments.ts | 26 ++++++++--------- 8 files changed, 54 insertions(+), 57 deletions(-) diff --git a/rollup.config.js b/rollup.config.js index f947d51c8..e53cab8d0 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -57,7 +57,9 @@ const plugins = (es5) => [ }), ] -const entrypoints = fs.readdirSync('./src/entrypoints') +// const entrypoints = fs.readdirSync('./src/entrypoints') + +const entrypoints = ['array.ts'] const entrypointTargets = entrypoints.map((file) => { const fileParts = file.split('.') diff --git a/src/__tests__/decide.ts b/src/__tests__/decide.ts index 026f6d0b5..49163b2c2 100644 --- a/src/__tests__/decide.ts +++ b/src/__tests__/decide.ts @@ -52,7 +52,7 @@ describe('Decide', () => { get_property: (key: string) => posthog.persistence!.props[key], capture: jest.fn(), _addCaptureHook: jest.fn(), - _afterDecideResponse: jest.fn(), + _onRemoteConfig: jest.fn(), get_distinct_id: jest.fn().mockImplementation(() => 'distinctid'), _send_request: jest.fn().mockImplementation(({ callback }) => callback?.({ config: {} })), featureFlags: { @@ -200,7 +200,7 @@ describe('Decide', () => { subject({} as DecideResponse) expect(posthog.featureFlags.receivedFeatureFlags).toHaveBeenCalledWith({}, false) - expect(posthog._afterDecideResponse).toHaveBeenCalledWith({}) + expect(posthog._onRemoteConfig).toHaveBeenCalledWith({}) }) it('Make sure receivedFeatureFlags is called with errors if the decide response fails', () => { @@ -225,7 +225,7 @@ describe('Decide', () => { } as unknown as DecideResponse subject(decideResponse) - expect(posthog._afterDecideResponse).toHaveBeenCalledWith(decideResponse) + expect(posthog._onRemoteConfig).toHaveBeenCalledWith(decideResponse) expect(posthog.featureFlags.receivedFeatureFlags).not.toHaveBeenCalled() }) @@ -242,7 +242,7 @@ describe('Decide', () => { } as unknown as DecideResponse subject(decideResponse) - expect(posthog._afterDecideResponse).toHaveBeenCalledWith(decideResponse) + expect(posthog._onRemoteConfig).toHaveBeenCalledWith(decideResponse) expect(posthog.featureFlags.receivedFeatureFlags).not.toHaveBeenCalled() }) }) diff --git a/src/__tests__/personProcessing.test.ts b/src/__tests__/personProcessing.test.ts index a5abee831..12abe13b3 100644 --- a/src/__tests__/personProcessing.test.ts +++ b/src/__tests__/personProcessing.test.ts @@ -2,7 +2,7 @@ import { createPosthogInstance } from './helpers/posthog-instance' import { uuidv7 } from '../uuidv7' import { logger } from '../utils/logger' import { INITIAL_CAMPAIGN_PARAMS, INITIAL_REFERRER_INFO } from '../constants' -import { DecideResponse } from '../types' +import { RemoteConfig } from '../types' jest.mock('../utils/logger') @@ -725,7 +725,7 @@ describe('person processing', () => { posthog.capture('startup page view') // act - posthog._afterDecideResponse({ defaultIdentifiedOnly: false } as DecideResponse) + posthog._onRemoteConfig({ defaultIdentifiedOnly: false } as RemoteConfig) posthog.capture('custom event') // assert @@ -740,7 +740,7 @@ describe('person processing', () => { posthog.capture('startup page view') // act - posthog._afterDecideResponse({ defaultIdentifiedOnly: false } as DecideResponse) + posthog._onRemoteConfig({ defaultIdentifiedOnly: false } as RemoteConfig) posthog.capture('custom event') // assert @@ -759,7 +759,7 @@ describe('person processing', () => { ) // act - posthog1._afterDecideResponse({ defaultIdentifiedOnly: false } as DecideResponse) + posthog1._onRemoteConfig({ defaultIdentifiedOnly: false } as RemoteConfig) posthog1.capture('custom event 1') const { posthog: posthog2, beforeSendMock: beforeSendMock2 } = await setup( undefined, diff --git a/src/__tests__/posthog-core.loaded.ts b/src/__tests__/posthog-core.loaded.ts index 1dc667f4a..925087ce2 100644 --- a/src/__tests__/posthog-core.loaded.ts +++ b/src/__tests__/posthog-core.loaded.ts @@ -27,6 +27,7 @@ describe('loaded() with flags', () => { resetRequestQueue: jest.fn(), _startReloadTimer: jest.fn(), receivedFeatureFlags: jest.fn(), + onFeatureFlags: jest.fn(), }, _send_request: jest.fn(({ callback }) => callback?.({ status: 200, json: {} })), }) diff --git a/src/__tests__/posthog-core.ts b/src/__tests__/posthog-core.ts index 9161fee2c..78fc2043f 100644 --- a/src/__tests__/posthog-core.ts +++ b/src/__tests__/posthog-core.ts @@ -7,7 +7,7 @@ import * as globals from '../utils/globals' import { ENABLE_PERSON_PROCESSING, USER_STATE } from '../constants' import { createPosthogInstance, defaultPostHog } from './helpers/posthog-instance' import { logger } from '../utils/logger' -import { DecideResponse, PostHogConfig } from '../types' +import { PostHogConfig, RemoteConfig } from '../types' import { PostHog } from '../posthog-core' import { PostHogPersistence } from '../posthog-persistence' import { SessionIdManager } from '../sessionid' @@ -295,7 +295,7 @@ describe('posthog core', () => { it('sends payloads to alternative endpoint if given', () => { const posthog = posthogWith({ ...defaultConfig, request_batching: false }, defaultOverrides) - posthog._afterDecideResponse({ analytics: { endpoint: '/i/v0/e/' } } as DecideResponse) + posthog._onRemoteConfig({ analytics: { endpoint: '/i/v0/e/' } } as RemoteConfig) posthog.capture('event-name', { foo: 'bar', length: 0 }) @@ -320,7 +320,7 @@ describe('posthog core', () => { it('sends payloads to overriden _url, even if alternative endpoint is set', () => { const posthog = posthogWith({ ...defaultConfig, request_batching: false }, defaultOverrides) - posthog._afterDecideResponse({ analytics: { endpoint: '/i/v0/e/' } } as DecideResponse) + posthog._onRemoteConfig({ analytics: { endpoint: '/i/v0/e/' } } as RemoteConfig) posthog.capture('event-name', { foo: 'bar', length: 0 }, { _url: 'https://app.posthog.com/s/' }) @@ -336,29 +336,29 @@ describe('posthog core', () => { it('enables compression from decide response', () => { const posthog = posthogWith({}) - posthog._afterDecideResponse({ supportedCompression: ['gzip-js', 'base64'] } as DecideResponse) + posthog._onRemoteConfig({ supportedCompression: ['gzip-js', 'base64'] } as RemoteConfig) expect(posthog.compression).toEqual('gzip-js') }) it('uses defaultIdentifiedOnly from decide response', () => { const posthog = posthogWith({}) - posthog._afterDecideResponse({ defaultIdentifiedOnly: true } as DecideResponse) + posthog._onRemoteConfig({ defaultIdentifiedOnly: true } as RemoteConfig) expect(posthog.config.person_profiles).toEqual('identified_only') - posthog._afterDecideResponse({ defaultIdentifiedOnly: false } as DecideResponse) + posthog._onRemoteConfig({ defaultIdentifiedOnly: false } as RemoteConfig) expect(posthog.config.person_profiles).toEqual('always') }) it('defaultIdentifiedOnly does not override person_profiles if already set', () => { const posthog = posthogWith({ person_profiles: 'always' }) - posthog._afterDecideResponse({ defaultIdentifiedOnly: true } as DecideResponse) + posthog._onRemoteConfig({ defaultIdentifiedOnly: true } as RemoteConfig) expect(posthog.config.person_profiles).toEqual('always') }) it('enables compression from decide response when only one received', () => { const posthog = posthogWith({}) - posthog._afterDecideResponse({ supportedCompression: ['base64'] } as DecideResponse) + posthog._onRemoteConfig({ supportedCompression: ['base64'] } as RemoteConfig) expect(posthog.compression).toEqual('base64') }) @@ -366,7 +366,7 @@ describe('posthog core', () => { it('does not enable compression from decide response if compression is disabled', () => { const posthog = posthogWith({ disable_compression: true, persistence: 'memory' }) - posthog._afterDecideResponse({ supportedCompression: ['gzip-js', 'base64'] } as DecideResponse) + posthog._onRemoteConfig({ supportedCompression: ['gzip-js', 'base64'] } as RemoteConfig) expect(posthog.compression).toEqual(undefined) }) @@ -374,7 +374,7 @@ describe('posthog core', () => { it('defaults to /e if no endpoint is given', () => { const posthog = posthogWith({}) - posthog._afterDecideResponse({} as DecideResponse) + posthog._onRemoteConfig({} as RemoteConfig) expect(posthog.analyticsDefaultEndpoint).toEqual('/e/') }) @@ -382,7 +382,7 @@ describe('posthog core', () => { it('uses the specified analytics endpoint if given', () => { const posthog = posthogWith({}) - posthog._afterDecideResponse({ analytics: { endpoint: '/i/v0/e/' } } as DecideResponse) + posthog._onRemoteConfig({ analytics: { endpoint: '/i/v0/e/' } } as RemoteConfig) expect(posthog.analyticsDefaultEndpoint).toEqual('/i/v0/e/') }) diff --git a/src/decide.ts b/src/decide.ts index 9a84b4d6a..3f3ccec6d 100644 --- a/src/decide.ts +++ b/src/decide.ts @@ -114,7 +114,7 @@ export class Decide { return } - this.instance._afterDecideResponse(response) + this.instance._onRemoteConfig(response) } private onRemoteConfig(config?: RemoteConfig): void { diff --git a/src/posthog-core.ts b/src/posthog-core.ts index e9ce4ed5e..58c2a3b39 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -32,7 +32,6 @@ import { CaptureOptions, CaptureResult, Compression, - DecideResponse, EarlyAccessFeatureCallback, EventName, IsFeatureEnabledOptions, @@ -545,42 +544,39 @@ export class PostHog { return this } - // Private methods - _afterDecideResponse(response: DecideResponse) { + _onRemoteConfig(config: RemoteConfig) { + // TODO: check config. If "hasFlags" is anything other than false - load the flags from decide (later will be /flags) + this.compression = undefined - if (response.supportedCompression && !this.config.disable_compression) { - this.compression = includes(response['supportedCompression'], Compression.GZipJS) + if (config.supportedCompression && !this.config.disable_compression) { + this.compression = includes(config['supportedCompression'], Compression.GZipJS) ? Compression.GZipJS - : includes(response['supportedCompression'], Compression.Base64) + : includes(config['supportedCompression'], Compression.Base64) ? Compression.Base64 : undefined } - if (response.analytics?.endpoint) { - this.analyticsDefaultEndpoint = response.analytics.endpoint + if (config.analytics?.endpoint) { + this.analyticsDefaultEndpoint = config.analytics.endpoint } this.set_config({ person_profiles: this._initialPersonProfilesConfig ? this._initialPersonProfilesConfig - : response['defaultIdentifiedOnly'] + : config['defaultIdentifiedOnly'] ? 'identified_only' : 'always', }) - this.siteApps?.onRemoteConfig(response) - this.sessionRecording?.onRemoteConfig(response) - this.autocapture?.onRemoteConfig(response) - this.heatmaps?.onRemoteConfig(response) - this.experiments?.onRemoteConfig(response) - this.surveys?.onRemoteConfig(response) - this.webVitalsAutocapture?.onRemoteConfig(response) - this.exceptionObserver?.onRemoteConfig(response) - this.deadClicksAutocapture?.onRemoteConfig(response) - } - - _onRemoteConfig(config: RemoteConfig) { - // TODO: check config. If "hasFlags" is anything other than false - load the flags from decide (later will be /flags) + this.siteApps?.onRemoteConfig(config) + this.sessionRecording?.onRemoteConfig(config) + this.autocapture?.onRemoteConfig(config) + this.heatmaps?.onRemoteConfig(config) + this.experiments?.onRemoteConfig(config) + this.surveys?.onRemoteConfig(config) + this.webVitalsAutocapture?.onRemoteConfig(config) + this.exceptionObserver?.onRemoteConfig(config) + this.deadClicksAutocapture?.onRemoteConfig(config) if (config.hasFeatureFlags !== false) { // Check explicitly for false - anything else means we there could be so lets load them diff --git a/src/web-experiments.ts b/src/web-experiments.ts index 7a15762e9..3568e0e3f 100644 --- a/src/web-experiments.ts +++ b/src/web-experiments.ts @@ -1,5 +1,5 @@ import { PostHog } from './posthog-core' -import { DecideResponse } from './types' +import { RemoteConfig } from './types' import { navigator, window } from './utils/globals' import { WebExperiment, @@ -9,7 +9,7 @@ import { WebExperimentVariant, } from './web-experiments-types' import { WEB_EXPERIMENTS } from './constants' -import { isNullish } from './utils/type-utils' +import { isNullish, isString } from './utils/type-utils' import { getQueryParam, isUrlMatchingRegex } from './utils/request-utils' import { logger } from './utils/logger' import { Info } from './utils/event-utils' @@ -31,7 +31,6 @@ export const webExperimentUrlValidationMap: Record< export class WebExperiments { instance: PostHog - private _featureFlags?: Record private _flagToExperiments?: Map constructor(instance: PostHog) { @@ -65,15 +64,18 @@ export class WebExperiments { }) } - onRemoteConfig(response: DecideResponse) { + onRemoteConfig(config: RemoteConfig) { + // TODO: Does this need to listen to config or rather just flags?? if (this._is_bot()) { WebExperiments.logInfo('Refusing to render web experiment since the viewer is a likely bot') return } - this._featureFlags = response.featureFlags - this.loadIfEnabled() - this.previewWebExperiment() + this.instance.onFeatureFlags(() => { + // NOTE: Should this only fire once? + this.loadIfEnabled() + this.previewWebExperiment() + }) } previewWebExperiment() { @@ -108,11 +110,7 @@ export class WebExperiments { this._flagToExperiments = new Map() webExperiments.forEach((webExperiment) => { - if ( - webExperiment.feature_flag_key && - this._featureFlags && - this._featureFlags[webExperiment.feature_flag_key] - ) { + if (webExperiment.feature_flag_key) { if (this._flagToExperiments) { WebExperiments.logInfo( `setting flag key `, @@ -123,8 +121,8 @@ export class WebExperiments { this._flagToExperiments?.set(webExperiment.feature_flag_key, webExperiment) } - const selectedVariant = this._featureFlags[webExperiment.feature_flag_key] as unknown as string - if (selectedVariant && webExperiment.variants[selectedVariant]) { + const selectedVariant = this.instance.featureFlags.getFeatureFlag(webExperiment.feature_flag_key) + if (isString(selectedVariant) && webExperiment.variants[selectedVariant]) { this.applyTransforms( webExperiment.name, selectedVariant, From 6ff863f2a571b6f6b6ffd0fc71f8eddb3d6cf628 Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 3 Dec 2024 14:47:04 +0100 Subject: [PATCH 05/20] Fix up loading logic --- src/__tests__/decide.ts | 62 ++++++++++++++++++++++++++++++++++++++++- src/decide.ts | 5 ++++ 2 files changed, 66 insertions(+), 1 deletion(-) diff --git a/src/__tests__/decide.ts b/src/__tests__/decide.ts index 49163b2c2..fb4514049 100644 --- a/src/__tests__/decide.ts +++ b/src/__tests__/decide.ts @@ -2,8 +2,9 @@ import { Decide } from '../decide' import { PostHogPersistence } from '../posthog-persistence' import { RequestRouter } from '../utils/request-router' import { PostHog } from '../posthog-core' -import { DecideResponse, PostHogConfig, Properties } from '../types' +import { DecideResponse, PostHogConfig, Properties, RemoteConfig } from '../types' import '../entrypoints/external-scripts-loader' +import { assignableWindow } from '../utils/globals' const expectDecodedSendRequest = ( send_request: PostHog['_send_request'], @@ -246,4 +247,63 @@ describe('Decide', () => { expect(posthog.featureFlags.receivedFeatureFlags).not.toHaveBeenCalled() }) }) + + describe('remote config', () => { + const config = { surveys: true } as RemoteConfig + + beforeEach(() => { + posthog.config.__preview_remote_config = true + assignableWindow._POSTHOG_CONFIG = undefined + assignableWindow.POSTHOG_DEBUG = true + + assignableWindow.__PosthogExtensions__.loadExternalDependency = jest.fn( + (_ph: PostHog, _name: string, cb: (err?: any) => void) => { + assignableWindow._POSTHOG_CONFIG = config as RemoteConfig + cb() + } + ) + + posthog._send_request = jest.fn().mockImplementation(({ callback }) => callback?.({ json: config })) + }) + + it('properly pulls from the window and uses it if set', () => { + assignableWindow._POSTHOG_CONFIG = config as RemoteConfig + decide().call() + + expect(assignableWindow.__PosthogExtensions__.loadExternalDependency).not.toHaveBeenCalled() + expect(posthog._send_request).not.toHaveBeenCalled() + + expect(posthog._onRemoteConfig).toHaveBeenCalledWith(config) + }) + + it('loads the script if window config not set', () => { + decide().call() + + expect(assignableWindow.__PosthogExtensions__.loadExternalDependency).toHaveBeenCalledWith( + posthog, + 'remote-config', + expect.any(Function) + ) + expect(posthog._send_request).not.toHaveBeenCalled() + expect(posthog._onRemoteConfig).toHaveBeenCalledWith(config) + }) + + it('loads the json if window config not set and js failed', () => { + assignableWindow.__PosthogExtensions__.loadExternalDependency = jest.fn( + (_ph: PostHog, _name: string, cb: (err?: any) => void) => { + cb() + } + ) + + decide().call() + + expect(assignableWindow.__PosthogExtensions__.loadExternalDependency).toHaveBeenCalled() + expect(posthog._send_request).toHaveBeenCalledWith({ + method: 'GET', + url: 'https://test.com/array/testtoken/config', + callback: expect.any(Function), + }) + expect(posthog._onRemoteConfig).toHaveBeenCalledWith(config) + }) + }) }) diff --git a/src/decide.ts b/src/decide.ts index 3f3ccec6d..a6e663b72 100644 --- a/src/decide.ts +++ b/src/decide.ts @@ -26,6 +26,9 @@ export class Decide { assignableWindow.__PosthogExtensions__?.loadExternalDependency?.(this.instance, 'remote-config', () => { return cb(assignableWindow._POSTHOG_CONFIG) }) + } else { + logger.error('PostHog Extensions not found. Cannot load remote config.') + cb() } } @@ -43,6 +46,7 @@ export class Decide { if (this.instance.config.__preview_remote_config) { // Attempt 1 - use the pre-loaded config if it came as part of the token-specific array.js if (assignableWindow._POSTHOG_CONFIG) { + logger.info('Using preloaded remote config', assignableWindow._POSTHOG_CONFIG) this.onRemoteConfig(assignableWindow._POSTHOG_CONFIG) return } @@ -50,6 +54,7 @@ export class Decide { // Attempt 2 - if we have the external deps loader then lets load the script version of the config that includes site apps this._loadRemoteConfigJs((config) => { if (!config) { + logger.info('No config found after loading remote JS config. Falling back to JSON.') // Attempt 3 Load the config json instead of the script - we won't get site apps etc. but we will get the config this._loadRemoteConfigJSON((config) => { this.onRemoteConfig(config) From e5c506ab8dac21a83ee68d36b99d25221c734e02 Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 3 Dec 2024 15:05:00 +0100 Subject: [PATCH 06/20] Fixes --- src/__tests__/decide.ts | 17 +++++++++++++++++ src/decide.ts | 22 +++++++++++++++++++++- src/posthog-core.ts | 11 +---------- 3 files changed, 39 insertions(+), 11 deletions(-) diff --git a/src/__tests__/decide.ts b/src/__tests__/decide.ts index fb4514049..b770283ac 100644 --- a/src/__tests__/decide.ts +++ b/src/__tests__/decide.ts @@ -57,6 +57,8 @@ describe('Decide', () => { get_distinct_id: jest.fn().mockImplementation(() => 'distinctid'), _send_request: jest.fn().mockImplementation(({ callback }) => callback?.({ config: {} })), featureFlags: { + resetRequestQueue: jest.fn(), + reloadFeatureFlags: jest.fn(), receivedFeatureFlags: jest.fn(), setReloadingPaused: jest.fn(), _startReloadTimer: jest.fn(), @@ -305,5 +307,20 @@ describe('Decide', () => { }) expect(posthog._onRemoteConfig).toHaveBeenCalledWith(config) }) + + it.each([ + [true, true], + [false, false], + [undefined, true], + ])('conditionally reloads feature flags - hasFlags: %s, shouldReload: %s', (hasFeatureFlags, shouldReload) => { + assignableWindow._POSTHOG_CONFIG = { hasFeatureFlags } as RemoteConfig + decide().call() + + if (shouldReload) { + expect(posthog.featureFlags.reloadFeatureFlags).toHaveBeenCalled() + } else { + expect(posthog.featureFlags.reloadFeatureFlags).not.toHaveBeenCalled() + } + }) }) }) diff --git a/src/decide.ts b/src/decide.ts index a6e663b72..787d5e368 100644 --- a/src/decide.ts +++ b/src/decide.ts @@ -43,6 +43,17 @@ export class Decide { } call(): void { + // Call decide to get what features are enabled and other settings. + // As a reminder, if the /decide endpoint is disabled, feature flags, toolbar, session recording, autocapture, + // and compression will not be available. + const disableRemoteCalls = !!this.instance.config.advanced_disable_decide + + if (!disableRemoteCalls) { + // TRICKY: Reset any decide reloads queued during config.loaded because they'll be + // covered by the decide call right above. + this.instance.featureFlags.resetRequestQueue() + } + if (this.instance.config.__preview_remote_config) { // Attempt 1 - use the pre-loaded config if it came as part of the token-specific array.js if (assignableWindow._POSTHOG_CONFIG) { @@ -51,6 +62,11 @@ export class Decide { return } + if (disableRemoteCalls) { + logger.warn('Remote config is disabled. Falling back to local config.') + return + } + // Attempt 2 - if we have the external deps loader then lets load the script version of the config that includes site apps this._loadRemoteConfigJs((config) => { if (!config) { @@ -138,6 +154,10 @@ export class Decide { this.instance._onRemoteConfig(config) - // Additionally trigger loading of flags if necessary + if (config.hasFeatureFlags !== false) { + // If the config has feature flags, we need to call decide to get the feature flags + // This completely separates it from the config logic which is good in terms of separation of concerns + this.instance.featureFlags.reloadFeatureFlags() + } } } diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 58c2a3b39..5f0ad2ad8 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -610,16 +610,7 @@ export class PostHog { }, 1) } - // Call decide to get what features are enabled and other settings. - // As a reminder, if the /decide endpoint is disabled, feature flags, toolbar, session recording, autocapture, - // and compression will not be available. - if (!disableDecide) { - new Decide(this).call() - - // TRICKY: Reset any decide reloads queued during config.loaded because they'll be - // covered by the decide call right above. - this.featureFlags.resetRequestQueue() - } + new Decide(this).call() } _start_queue_if_opted_in(): void { From 87f2d8020ee4200f63515ee2f42284c125e35536 Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 3 Dec 2024 15:25:13 +0100 Subject: [PATCH 07/20] Fix up remote config stuff --- src/posthog-core.ts | 1 - src/web-experiments.ts | 39 ++++++++++++++++----------------------- 2 files changed, 16 insertions(+), 24 deletions(-) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 5f0ad2ad8..e26ec2abb 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -572,7 +572,6 @@ export class PostHog { this.sessionRecording?.onRemoteConfig(config) this.autocapture?.onRemoteConfig(config) this.heatmaps?.onRemoteConfig(config) - this.experiments?.onRemoteConfig(config) this.surveys?.onRemoteConfig(config) this.webVitalsAutocapture?.onRemoteConfig(config) this.exceptionObserver?.onRemoteConfig(config) diff --git a/src/web-experiments.ts b/src/web-experiments.ts index 3568e0e3f..5e976aa3e 100644 --- a/src/web-experiments.ts +++ b/src/web-experiments.ts @@ -1,5 +1,4 @@ import { PostHog } from './posthog-core' -import { RemoteConfig } from './types' import { navigator, window } from './utils/globals' import { WebExperiment, @@ -30,20 +29,28 @@ export const webExperimentUrlValidationMap: Record< } export class WebExperiments { - instance: PostHog private _flagToExperiments?: Map - constructor(instance: PostHog) { - this.instance = instance - const appFeatureFLags = (flags: string[]) => { + constructor(private instance: PostHog) { + this.instance.onFeatureFlags((flags: string[]) => { this.applyFeatureFlagChanges(flags) - } - - this.instance.onFeatureFlags(appFeatureFLags) - this._flagToExperiments = new Map() + }) } applyFeatureFlagChanges(flags: string[]) { + if (this._is_bot()) { + WebExperiments.logInfo('Refusing to render web experiment since the viewer is a likely bot') + return + } + + if (!this._flagToExperiments) { + // Indicates first load so we trigger the loaders + this._flagToExperiments = new Map() + // NOTE: Should this only fire once? + this.loadIfEnabled() + this.previewWebExperiment() + } + if (isNullish(this._flagToExperiments) || this.instance.config.disable_web_experiments) { return } @@ -64,20 +71,6 @@ export class WebExperiments { }) } - onRemoteConfig(config: RemoteConfig) { - // TODO: Does this need to listen to config or rather just flags?? - if (this._is_bot()) { - WebExperiments.logInfo('Refusing to render web experiment since the viewer is a likely bot') - return - } - - this.instance.onFeatureFlags(() => { - // NOTE: Should this only fire once? - this.loadIfEnabled() - this.previewWebExperiment() - }) - } - previewWebExperiment() { const location = WebExperiments.getWindowLocation() if (location?.search) { From 676fdbdc4de144a5e3cd413031c8dd549817482b Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 3 Dec 2024 15:50:04 +0100 Subject: [PATCH 08/20] Fixes --- playground/nextjs/src/posthog.ts | 1 + src/decide.ts | 2 ++ src/web-experiments.ts | 5 +++-- 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/playground/nextjs/src/posthog.ts b/playground/nextjs/src/posthog.ts index 11a656e24..27ca2c9d6 100644 --- a/playground/nextjs/src/posthog.ts +++ b/playground/nextjs/src/posthog.ts @@ -62,6 +62,7 @@ if (typeof window !== 'undefined') { persistence: cookieConsentGiven() ? 'localStorage+cookie' : 'memory', person_profiles: PERSON_PROCESSING_MODE === 'never' ? 'identified_only' : PERSON_PROCESSING_MODE, persistence_name: `${process.env.NEXT_PUBLIC_POSTHOG_KEY}_nextjs`, + __preview_remote_config: true, ...configForConsent(), }) // Help with debugging(window as any).posthog = posthog diff --git a/src/decide.ts b/src/decide.ts index 787d5e368..3b6a07d53 100644 --- a/src/decide.ts +++ b/src/decide.ts @@ -155,6 +155,8 @@ export class Decide { this.instance._onRemoteConfig(config) if (config.hasFeatureFlags !== false) { + // TRICKY: This is set in the parent for some reason... + this.instance.featureFlags.setReloadingPaused(false) // If the config has feature flags, we need to call decide to get the feature flags // This completely separates it from the config logic which is good in terms of separation of concerns this.instance.featureFlags.reloadFeatureFlags() diff --git a/src/web-experiments.ts b/src/web-experiments.ts index 5e976aa3e..6e2476d14 100644 --- a/src/web-experiments.ts +++ b/src/web-experiments.ts @@ -33,11 +33,11 @@ export class WebExperiments { constructor(private instance: PostHog) { this.instance.onFeatureFlags((flags: string[]) => { - this.applyFeatureFlagChanges(flags) + this.onFeatureFlags(flags) }) } - applyFeatureFlagChanges(flags: string[]) { + onFeatureFlags(flags: string[]) { if (this._is_bot()) { WebExperiments.logInfo('Refusing to render web experiment since the viewer is a likely bot') return @@ -49,6 +49,7 @@ export class WebExperiments { // NOTE: Should this only fire once? this.loadIfEnabled() this.previewWebExperiment() + return } if (isNullish(this._flagToExperiments) || this.instance.config.disable_web_experiments) { From 6a143227276527e886ed5e45e9641513464a63ee Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 3 Dec 2024 16:06:18 +0100 Subject: [PATCH 09/20] Fixes --- src/__tests__/posthog-core.ts | 21 ++++++--------------- src/decide.ts | 4 ++++ 2 files changed, 10 insertions(+), 15 deletions(-) diff --git a/src/__tests__/posthog-core.ts b/src/__tests__/posthog-core.ts index 78fc2043f..25026ef0a 100644 --- a/src/__tests__/posthog-core.ts +++ b/src/__tests__/posthog-core.ts @@ -1,5 +1,3 @@ -import { Decide } from '../decide' - import { Info } from '../utils/event-utils' import { document, window } from '../utils/globals' import { uuidv7 } from '../uuidv7' @@ -15,8 +13,6 @@ import { RequestQueue } from '../request-queue' import { SessionRecording } from '../extensions/replay/sessionrecording' import { PostHogFeatureFlags } from '../posthog-featureflags' -jest.mock('../decide') - describe('posthog core', () => { const baseUTCDateTime = new Date(Date.UTC(2020, 0, 1, 0, 0, 0)) const eventName = '$event' @@ -1141,21 +1137,15 @@ describe('posthog core', () => { }) describe('/decide', () => { - beforeEach(() => { - const call = jest.fn() - ;(Decide as any).mockImplementation(() => ({ call })) - }) - - afterEach(() => { - ;(Decide as any).mockReset() - }) - it('is called by default', async () => { const instance = await createPosthogInstance(uuidv7()) instance.featureFlags.setReloadingPaused = jest.fn() + instance._send_request = jest.fn() instance._loaded() - expect(new Decide(instance).call).toHaveBeenCalled() + expect(instance._send_request.mock.calls[0][0]).toMatchObject({ + url: 'http://localhost/decide/?v=3', + }) expect(instance.featureFlags.setReloadingPaused).toHaveBeenCalledWith(true) }) @@ -1164,9 +1154,10 @@ describe('posthog core', () => { advanced_disable_decide: true, }) instance.featureFlags.setReloadingPaused = jest.fn() + instance._send_request = jest.fn() instance._loaded() - expect(new Decide(instance).call).not.toHaveBeenCalled() + expect(instance._send_request).not.toHaveBeenCalled() expect(instance.featureFlags.setReloadingPaused).not.toHaveBeenCalled() }) }) diff --git a/src/decide.ts b/src/decide.ts index 3b6a07d53..029434ed8 100644 --- a/src/decide.ts +++ b/src/decide.ts @@ -84,6 +84,10 @@ export class Decide { return } + if (disableRemoteCalls) { + return + } + /* Calls /decide endpoint to fetch options for autocapture, session recording, feature flags & compression. */ From e8ed9f290bc45c86aea6053b9e0cb583e5fdc50d Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 3 Dec 2024 17:41:59 +0100 Subject: [PATCH 10/20] Fixes --- src/web-experiments.ts | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/src/web-experiments.ts b/src/web-experiments.ts index 6e2476d14..244b10bfe 100644 --- a/src/web-experiments.ts +++ b/src/web-experiments.ts @@ -43,19 +43,18 @@ export class WebExperiments { return } - if (!this._flagToExperiments) { + if (this.instance.config.disable_web_experiments) { + return + } + + if (isNullish(this._flagToExperiments)) { // Indicates first load so we trigger the loaders this._flagToExperiments = new Map() - // NOTE: Should this only fire once? this.loadIfEnabled() this.previewWebExperiment() return } - if (isNullish(this._flagToExperiments) || this.instance.config.disable_web_experiments) { - return - } - WebExperiments.logInfo('applying feature flags', flags) flags.forEach((flag) => { if (this._flagToExperiments && this._flagToExperiments?.has(flag)) { From a49302de9d7b3c39c97ae343e7a41c7a8716e9c4 Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 3 Dec 2024 17:48:24 +0100 Subject: [PATCH 11/20] Fix tests --- src/__tests__/web-experiments.test.ts | 31 +++++++++++++++++---------- src/web-experiments.ts | 2 +- 2 files changed, 21 insertions(+), 12 deletions(-) diff --git a/src/__tests__/web-experiments.test.ts b/src/__tests__/web-experiments.test.ts index 2e5df4f11..3f504c30d 100644 --- a/src/__tests__/web-experiments.test.ts +++ b/src/__tests__/web-experiments.test.ts @@ -1,6 +1,6 @@ import { WebExperiments } from '../web-experiments' import { PostHog } from '../posthog-core' -import { DecideResponse, PostHogConfig } from '../types' +import { PostHogConfig } from '../types' import { PostHogPersistence } from '../posthog-persistence' import { WebExperiment } from '../web-experiments-types' import { RequestRouter } from '../utils/request-router' @@ -111,7 +111,10 @@ describe('Web Experimentation', () => { }, } as unknown as WebExperiment + const simulateFeatureFlags: jest.Mock = jest.fn() + beforeEach(() => { + let cachedFlags = {} persistence = { props: {}, register: jest.fn() } as unknown as PostHogPersistence posthog = makePostHog({ config: { @@ -129,6 +132,14 @@ describe('Web Experimentation', () => { .mockImplementation(({ callback }) => callback({ statusCode: 200, json: experimentsResponse })), consent: { isOptedOut: () => true } as unknown as ConsentManager, onFeatureFlags: jest.fn(), + getFeatureFlag: (key: string) => { + return cachedFlags[key] + }, + }) + + simulateFeatureFlags.mockImplementation((flags) => { + cachedFlags = flags + webExperiment.onFeatureFlags(Object.keys(flags)) }) posthog.requestRouter = new RequestRouter(posthog) @@ -171,11 +182,10 @@ describe('Web Experimentation', () => { function assertElementChanged(variant: string, expectedProperty: string, value: string) { const elParent = createTestDocument() webExperiment = new WebExperiments(posthog) - webExperiment.onRemoteConfig({ - featureFlags: { - 'signup-button-test': variant, - }, - } as unknown as DecideResponse) + + simulateFeatureFlags({ + 'signup-button-test': variant, + }) switch (expectedProperty) { case 'css': @@ -201,11 +211,10 @@ describe('Web Experimentation', () => { webExperiment._is_bot = () => true const elParent = createTestDocument() - webExperiment.onRemoteConfig({ - featureFlags: { - 'signup-button-test': 'Sign me up', - }, - } as unknown as DecideResponse) + simulateFeatureFlags({ + 'signup-button-test': 'Sign me up', + }) + expect(elParent.innerText).toEqual('original') }) }) diff --git a/src/web-experiments.ts b/src/web-experiments.ts index 244b10bfe..74bbc904b 100644 --- a/src/web-experiments.ts +++ b/src/web-experiments.ts @@ -114,7 +114,7 @@ export class WebExperiments { this._flagToExperiments?.set(webExperiment.feature_flag_key, webExperiment) } - const selectedVariant = this.instance.featureFlags.getFeatureFlag(webExperiment.feature_flag_key) + const selectedVariant = this.instance.getFeatureFlag(webExperiment.feature_flag_key) if (isString(selectedVariant) && webExperiment.variants[selectedVariant]) { this.applyTransforms( webExperiment.name, From a76528d4d7a4af57f9bd7a299658b8260ac47985 Mon Sep 17 00:00:00 2001 From: Ben White Date: Tue, 3 Dec 2024 17:49:18 +0100 Subject: [PATCH 12/20] Fix --- rollup.config.js | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/rollup.config.js b/rollup.config.js index e53cab8d0..f947d51c8 100644 --- a/rollup.config.js +++ b/rollup.config.js @@ -57,9 +57,7 @@ const plugins = (es5) => [ }), ] -// const entrypoints = fs.readdirSync('./src/entrypoints') - -const entrypoints = ['array.ts'] +const entrypoints = fs.readdirSync('./src/entrypoints') const entrypointTargets = entrypoints.map((file) => { const fileParts = file.split('.') From 5548ae4fe832f0122b4dc96cbab3b173935076e9 Mon Sep 17 00:00:00 2001 From: Ben White Date: Wed, 4 Dec 2024 13:27:45 +0100 Subject: [PATCH 13/20] Fix comments --- src/decide.ts | 10 ---------- 1 file changed, 10 deletions(-) diff --git a/src/decide.ts b/src/decide.ts index 029434ed8..e91c04e79 100644 --- a/src/decide.ts +++ b/src/decide.ts @@ -5,16 +5,6 @@ import { STORED_GROUP_PROPERTIES_KEY, STORED_PERSON_PROPERTIES_KEY } from './con import { logger } from './utils/logger' import { assignableWindow, document } from './utils/globals' -// TODO: Add check for global config existing. -// Modify the whole "afterDecideResponse" function to be a method of the PostHog class - -// 1. Option is to load the config endpoint (if __preview is enabled) and then if flags are needed call decide, and then call "afterDecideResponse" with the combined response :thinking: -// 2. Other option is to separate out the values so that we have a "config response" and a "flags response" separately... - -// TODO: Fix WebExperiments to wait for flags instead of only decide -// TODO: Fix Surveys to do the same -// TODO: Background refresh of the config - every 5 minutes to match CDN cache - at least when active - export class Decide { constructor(private readonly instance: PostHog) { // don't need to wait for `decide` to return if flags were provided on initialisation From 2cff552fbabab97d5fac303bf2c1963877abb86b Mon Sep 17 00:00:00 2001 From: Ben White Date: Wed, 4 Dec 2024 13:29:36 +0100 Subject: [PATCH 14/20] Fixes --- src/posthog-core.ts | 6 ------ src/site-apps.ts | 12 ------------ 2 files changed, 18 deletions(-) diff --git a/src/posthog-core.ts b/src/posthog-core.ts index e26ec2abb..920709306 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -545,8 +545,6 @@ export class PostHog { } _onRemoteConfig(config: RemoteConfig) { - // TODO: check config. If "hasFlags" is anything other than false - load the flags from decide (later will be /flags) - this.compression = undefined if (config.supportedCompression && !this.config.disable_compression) { this.compression = includes(config['supportedCompression'], Compression.GZipJS) @@ -576,10 +574,6 @@ export class PostHog { this.webVitalsAutocapture?.onRemoteConfig(config) this.exceptionObserver?.onRemoteConfig(config) this.deadClicksAutocapture?.onRemoteConfig(config) - - if (config.hasFeatureFlags !== false) { - // Check explicitly for false - anything else means we there could be so lets load them - } } _loaded(): void { diff --git a/src/site-apps.ts b/src/site-apps.ts index 9206f8475..63586b0d3 100644 --- a/src/site-apps.ts +++ b/src/site-apps.ts @@ -75,18 +75,6 @@ export class SiteApps { } onRemoteConfig(response?: RemoteConfig): void { - if (assignableWindow._POSTHOG_SITE_APPS) { - // Loaded via new config so we have the apps preloaded - if (this.instance.config.opt_in_site_apps) { - assignableWindow._POSTHOG_SITE_APPS.forEach((app) => { - app.load(this.instance) - }) - } - this.loaded = true - - return - } - if (isArray(response?.siteApps) && response.siteApps.length > 0) { if (this.enabled && this.instance.config.opt_in_site_apps) { const checkIfAllLoaded = () => { From a632f4aa2f502080538af510c1dbf7ce1920ec0e Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 5 Dec 2024 13:28:59 +0100 Subject: [PATCH 15/20] Fixes all over --- playground/nextjs/src/posthog.ts | 4 +- src/site-apps.ts | 184 +++++++++++++++++++++---------- src/types.ts | 29 +++++ src/utils/globals.ts | 11 +- src/utils/logger.ts | 92 +++++++++------- 5 files changed, 218 insertions(+), 102 deletions(-) diff --git a/playground/nextjs/src/posthog.ts b/playground/nextjs/src/posthog.ts index 27ca2c9d6..89de3feb1 100644 --- a/playground/nextjs/src/posthog.ts +++ b/playground/nextjs/src/posthog.ts @@ -62,10 +62,12 @@ if (typeof window !== 'undefined') { persistence: cookieConsentGiven() ? 'localStorage+cookie' : 'memory', person_profiles: PERSON_PROCESSING_MODE === 'never' ? 'identified_only' : PERSON_PROCESSING_MODE, persistence_name: `${process.env.NEXT_PUBLIC_POSTHOG_KEY}_nextjs`, + opt_in_site_apps: true, __preview_remote_config: true, ...configForConsent(), }) - // Help with debugging(window as any).posthog = posthog + // Help with debugging + ;(window as any).posthog = posthog } export const posthogHelpers = { diff --git a/src/site-apps.ts b/src/site-apps.ts index 63586b0d3..815d6a400 100644 --- a/src/site-apps.ts +++ b/src/site-apps.ts @@ -1,51 +1,53 @@ import { PostHog } from './posthog-core' -import { CaptureResult, RemoteConfig } from './types' +import { CaptureResult, Properties, RemoteConfig, SiteApp, SiteAppGlobals, SiteAppLoader } from './types' import { assignableWindow } from './utils/globals' -import { logger } from './utils/logger' +import { logger as _logger } from './utils/logger' import { isArray } from './utils/type-utils' +const logger = _logger.createLogger('[Site Apps]') + export class SiteApps { - instance: PostHog enabled: boolean - missedInvocations: Record[] - loaded: boolean - appsLoading: Set - - constructor(instance: PostHog) { - this.instance = instance - // can't use if site apps are disabled, or if we're not asking /decide for site apps - this.enabled = !!this.instance.config.opt_in_site_apps && !this.instance.config.advanced_disable_decide + apps: Record + + private stopBuffering?: () => void + private bufferedInvocations: SiteAppGlobals[] + + constructor(private instance: PostHog) { + this.enabled = !!this.instance.config.opt_in_site_apps // events captured between loading posthog-js and the site app; up to 1000 events - this.missedInvocations = [] - // capture events until loaded - this.loaded = false - this.appsLoading = new Set() + this.bufferedInvocations = [] + this.apps = {} } eventCollector(_eventName: string, eventPayload?: CaptureResult | undefined) { - if (!this.enabled) { + if (!eventPayload) { return } - if (!this.loaded && eventPayload) { - const globals = this.globalsForEvent(eventPayload) - this.missedInvocations.push(globals) - if (this.missedInvocations.length > 1000) { - this.missedInvocations = this.missedInvocations.slice(10) - } + const globals = this.globalsForEvent(eventPayload) + this.bufferedInvocations.push(globals) + if (this.bufferedInvocations.length > 1000) { + this.bufferedInvocations = this.bufferedInvocations.slice(10) } } init() { - this.instance?._addCaptureHook(this.eventCollector.bind(this)) + if (this.enabled) { + const stop = this.instance._addCaptureHook(this.eventCollector.bind(this)) + this.stopBuffering = () => { + stop() + this.stopBuffering = undefined + } + } } - globalsForEvent(event: CaptureResult): Record { + globalsForEvent(event: CaptureResult): SiteAppGlobals { if (!event) { throw new Error('Event payload is required') } - const groups: Record> = {} + const groups: SiteAppGlobals['groups'] = {} const groupIds = this.instance.get_property('$groups') || [] - const groupProperties = this.instance.get_property('$stored_group_properties') || {} + const groupProperties: Record = this.instance.get_property('$stored_group_properties') || {} for (const [type, properties] of Object.entries(groupProperties)) { groups[type] = { id: groupIds[type], type, properties } } @@ -74,44 +76,104 @@ export class SiteApps { return globals } - onRemoteConfig(response?: RemoteConfig): void { - if (isArray(response?.siteApps) && response.siteApps.length > 0) { - if (this.enabled && this.instance.config.opt_in_site_apps) { - const checkIfAllLoaded = () => { - // Stop collecting events once all site apps are loaded - if (this.appsLoading.size === 0) { - this.loaded = true - this.missedInvocations = [] - } - } - for (const { id, url } of response['siteApps']) { - // TODO: if we have opted out and "type" is "site_destination", ignore it... but do include "site_app" types - this.appsLoading.add(id) - assignableWindow[`__$$ph_site_app_${id}`] = this.instance - assignableWindow[`__$$ph_site_app_${id}_missed_invocations`] = () => this.missedInvocations - assignableWindow[`__$$ph_site_app_${id}_callback`] = () => { - this.appsLoading.delete(id) - checkIfAllLoaded() - } - assignableWindow.__PosthogExtensions__?.loadSiteApp?.(this.instance, url, (err) => { - if (err) { - this.appsLoading.delete(id) - checkIfAllLoaded() - return logger.error(`Error while initializing PostHog app with config id ${id}`, err) - } - }) + setupSiteApp(loader: SiteAppLoader) { + const app: SiteApp = { + id: loader.id, + loaded: false, + errored: false, + } + this.apps[loader.id] = app + + const onLoaded = (success: boolean) => { + this.apps[loader.id].errored = success + this.apps[loader.id].loaded = true + + logger.info(`Site app with id ${loader.id} ${success ? 'loaded' : 'errored'}`) + + if (success && this.bufferedInvocations.length) { + logger.info(`Processing ${this.bufferedInvocations.length} events for site app with id ${loader.id}`) + this.bufferedInvocations.forEach((globals) => app.processEvent?.(globals)) + } + + for (const app of Object.values(this.apps)) { + if (!app.loaded) { + // If any other apps are not loaded, we don't want to stop buffering + return } - } else if (response['siteApps'].length > 0) { - logger.error('PostHog site apps are disabled. Enable the "opt_in_site_apps" config to proceed.') - this.loaded = true - } else { - this.loaded = true } - } else { - this.loaded = true - this.enabled = false + + this.stopBuffering?.() + } + + try { + const { processEvent } = loader.init({ + posthog: this.instance, + callback: (success) => { + onLoaded(success) + }, + }) + + if (processEvent) { + app.processEvent = processEvent + } + } catch (e) { + logger.error(`Error while initializing PostHog app with config id ${loader.id}`, e) + onLoaded(false) } } - // TODO: opting out of stuff should disable this + private onCapturedEvent(event: CaptureResult) { + if (Object.keys(this.apps).length === 0) { + return + } + + const globals = this.globalsForEvent(event) + + for (const app of Object.values(this.apps)) { + try { + app.processEvent?.(globals) + } catch (e) { + logger.error(`Error while processing event ${event.event} for site app ${app.id}`, e) + } + } + } + + onRemoteConfig(response: RemoteConfig): void { + if (isArray(assignableWindow._POSTHOG_JS_APPS)) { + if (!this.enabled) { + logger.error(`PostHog site apps are disabled. Enable the "opt_in_site_apps" config to proceed.`) + return + } + + for (const app of assignableWindow._POSTHOG_JS_APPS) { + this.setupSiteApp(app) + } + + if (!assignableWindow._POSTHOG_JS_APPS.length) { + this.stopBuffering?.() + } else { + // NOTE: We could improve this to only fire if we actually have listeners for the event + this.instance.on('eventCaptured', (event) => this.onCapturedEvent(event)) + } + + return + } + + // NOTE: Below his is now only the fallback for legacy site app support. Once we have fully removed to the remote config loader we can get rid of this + + this.stopBuffering?.() + if (!this.enabled) { + logger.error(`PostHog site apps are disabled. Enable the "opt_in_site_apps" config to proceed.`) + return + } + + for (const { id, url } of response['siteApps']) { + assignableWindow[`__$$ph_site_app_${id}`] = this.instance + assignableWindow.__PosthogExtensions__?.loadSiteApp?.(this.instance, url, (err) => { + if (err) { + return logger.error(`Error while initializing PostHog app with config id ${id}`, err) + } + }) + } + } } diff --git a/src/types.ts b/src/types.ts index d351ebc2c..320b85019 100644 --- a/src/types.ts +++ b/src/types.ts @@ -540,6 +540,35 @@ export interface DecideResponse extends RemoteConfig { errorsWhileComputingFlags: boolean } +export type SiteAppGlobals = { + event: { + uuid: string + event: EventName + properties: Properties + timestamp?: Date + elements_chain?: string + distinct_id?: string + } + person: { + properties: Properties + } + groups: Record +} + +export type SiteAppLoader = { + id: string + init: (config: { posthog: PostHog; callback: (success: boolean) => void }) => { + processEvent?: (globals: SiteAppGlobals) => void + } +} + +export type SiteApp = { + id: string + loaded: boolean + errored: boolean + processEvent?: (globals: SiteAppGlobals) => void +} + export type FeatureFlagsCallback = ( flags: string[], variants: Record, diff --git a/src/utils/globals.ts b/src/utils/globals.ts index 23ca7a1ba..a2e2479fa 100644 --- a/src/utils/globals.ts +++ b/src/utils/globals.ts @@ -1,7 +1,14 @@ import { ErrorProperties } from '../extensions/exception-autocapture/error-conversion' import type { PostHog } from '../posthog-core' import { SessionIdManager } from '../sessionid' -import { DeadClicksAutoCaptureConfig, ErrorEventArgs, ErrorMetadata, Properties, RemoteConfig } from '../types' +import { + DeadClicksAutoCaptureConfig, + ErrorEventArgs, + ErrorMetadata, + Properties, + RemoteConfig, + SiteAppLoader, +} from '../types' /* * Global helpers to protect access to browser globals in a way that is safer for different targets @@ -21,7 +28,7 @@ export type AssignableWindow = Window & Record & { __PosthogExtensions__?: PostHogExtensions _POSTHOG_CONFIG?: RemoteConfig - _POSTHOG_SITE_APPS?: { token: string; load: (posthog: PostHog) => void }[] + _POSTHOG_JS_APPS?: SiteAppLoader[] } /** diff --git a/src/utils/logger.ts b/src/utils/logger.ts index a2bbfc405..669c91359 100644 --- a/src/utils/logger.ts +++ b/src/utils/logger.ts @@ -2,44 +2,60 @@ import Config from '../config' import { isUndefined } from './type-utils' import { assignableWindow, window } from './globals' -const LOGGER_PREFIX = '[PostHog.js]' -export const logger = { - _log: (level: 'log' | 'warn' | 'error', ...args: any[]) => { - if ( - window && - (Config.DEBUG || assignableWindow.POSTHOG_DEBUG) && - !isUndefined(window.console) && - window.console - ) { - const consoleLog = - '__rrweb_original__' in window.console[level] - ? (window.console[level] as any)['__rrweb_original__'] - : window.console[level] +export type Logger = { + _log: (level: 'log' | 'warn' | 'error', ...args: any[]) => void + info: (...args: any[]) => void + warn: (...args: any[]) => void + error: (...args: any[]) => void + critical: (...args: any[]) => void + uninitializedWarning: (methodName: string) => void + createLogger: (prefix: string) => Logger +} + +const createLogger = (prefix: string): Logger => { + const logger: Logger = { + _log: (level: 'log' | 'warn' | 'error', ...args: any[]) => { + if ( + window && + (Config.DEBUG || assignableWindow.POSTHOG_DEBUG) && + !isUndefined(window.console) && + window.console + ) { + const consoleLog = + '__rrweb_original__' in window.console[level] + ? (window.console[level] as any)['__rrweb_original__'] + : window.console[level] + + // eslint-disable-next-line no-console + consoleLog(prefix, ...args) + } + }, + + info: (...args: any[]) => { + logger._log('log', ...args) + }, + + warn: (...args: any[]) => { + logger._log('warn', ...args) + }, + error: (...args: any[]) => { + logger._log('error', ...args) + }, + + critical: (...args: any[]) => { + // Critical errors are always logged to the console // eslint-disable-next-line no-console - consoleLog(LOGGER_PREFIX, ...args) - } - }, - - info: (...args: any[]) => { - logger._log('log', ...args) - }, - - warn: (...args: any[]) => { - logger._log('warn', ...args) - }, - - error: (...args: any[]) => { - logger._log('error', ...args) - }, - - critical: (...args: any[]) => { - // Critical errors are always logged to the console - // eslint-disable-next-line no-console - console.error(LOGGER_PREFIX, ...args) - }, - - uninitializedWarning: (methodName: string) => { - logger.error(`You must initialize PostHog before calling ${methodName}`) - }, + console.error(prefix, ...args) + }, + + uninitializedWarning: (methodName: string) => { + logger.error(`You must initialize PostHog before calling ${methodName}`) + }, + + createLogger: (additionalPrefix: string) => createLogger(`${prefix} ${additionalPrefix}`), + } + return logger } + +export const logger = createLogger('[PostHog.js]') From 256bafb2c7a8188dc41d483992e808fe0fcc64cf Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 5 Dec 2024 14:15:45 +0100 Subject: [PATCH 16/20] Fix --- src/__tests__/site-apps.ts | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/__tests__/site-apps.ts b/src/__tests__/site-apps.ts index aea6407a5..1d85888f9 100644 --- a/src/__tests__/site-apps.ts +++ b/src/__tests__/site-apps.ts @@ -11,6 +11,10 @@ import { isFunction } from '../utils/type-utils' jest.mock('../utils/logger', () => ({ logger: { error: jest.fn(), + createLogger: jest.fn(() => ({ + error: jest.fn(), + info: jest.fn(), + })), }, })) From 7aa3da664869309b6007ef1046fd26becce777b0 Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 5 Dec 2024 14:29:26 +0100 Subject: [PATCH 17/20] Fixes --- src/__tests__/site-apps.ts | 124 ++++++++++++++++++------------------- src/site-apps.ts | 16 +++-- 2 files changed, 71 insertions(+), 69 deletions(-) diff --git a/src/__tests__/site-apps.ts b/src/__tests__/site-apps.ts index 1d85888f9..bf1cbb867 100644 --- a/src/__tests__/site-apps.ts +++ b/src/__tests__/site-apps.ts @@ -21,6 +21,7 @@ jest.mock('../utils/logger', () => ({ describe('SiteApps', () => { let posthog: PostHog let siteAppsInstance: SiteApps + let emitCaptureEvent: ((eventName: string, eventPayload: CaptureResult) => void) | undefined const defaultConfig: Partial = { token: 'testtoken', @@ -48,14 +49,19 @@ describe('SiteApps', () => { }), } + const removeCaptureHook = jest.fn() + posthog = { - config: { ...defaultConfig }, + config: { ...defaultConfig, opt_in_site_apps: true }, persistence: new PostHogPersistence(defaultConfig as PostHogConfig), register: (props: Properties) => posthog.persistence!.register(props), unregister: (key: string) => posthog.persistence!.unregister(key), get_property: (key: string) => posthog.persistence!.props[key], capture: jest.fn(), - _addCaptureHook: jest.fn(), + _addCaptureHook: jest.fn((cb) => { + emitCaptureEvent = cb + return removeCaptureHook + }), _afterDecideResponse: jest.fn(), get_distinct_id: jest.fn().mockImplementation(() => 'distinctid'), _send_request: jest.fn().mockImplementation(({ callback }) => callback?.({ config: {} })), @@ -77,94 +83,86 @@ describe('SiteApps', () => { }) describe('constructor', () => { - it('sets enabled to true when opt_in_site_apps is true and advanced_disable_decide is false', () => { + it('sets enabled to true when opt_in_site_apps is true', () => { posthog.config = { ...defaultConfig, opt_in_site_apps: true, - advanced_disable_decide: false, } as PostHogConfig - siteAppsInstance = new SiteApps(posthog) - - expect(siteAppsInstance.enabled).toBe(true) + expect(siteAppsInstance.isEnabled).toBe(true) }) it('sets enabled to false when opt_in_site_apps is false', () => { posthog.config = { ...defaultConfig, opt_in_site_apps: false, - advanced_disable_decide: false, } as PostHogConfig siteAppsInstance = new SiteApps(posthog) - expect(siteAppsInstance.enabled).toBe(false) - }) - - it('sets enabled to false when advanced_disable_decide is true', () => { - posthog.config = { - ...defaultConfig, - opt_in_site_apps: true, - advanced_disable_decide: true, - } as PostHogConfig - - siteAppsInstance = new SiteApps(posthog) - - expect(siteAppsInstance.enabled).toBe(false) + expect(siteAppsInstance.isEnabled).toBe(false) }) it('initializes missedInvocations, loaded, appsLoading correctly', () => { - expect(siteAppsInstance.missedInvocations).toEqual([]) - expect(siteAppsInstance.loaded).toBe(false) - expect(siteAppsInstance.appsLoading).toEqual(new Set()) + expect(siteAppsInstance['bufferedInvocations']).toEqual([]) + expect(siteAppsInstance.apps).toEqual({}) }) }) describe('init', () => { it('adds eventCollector as a capture hook', () => { + expect(siteAppsInstance['stopBuffering']).toBeUndefined() siteAppsInstance.init() expect(posthog._addCaptureHook).toHaveBeenCalledWith(expect.any(Function)) + expect(siteAppsInstance['stopBuffering']).toEqual(expect.any(Function)) }) - }) - describe('eventCollector', () => { - it('does nothing if enabled is false', () => { - siteAppsInstance.enabled = false - siteAppsInstance.eventCollector('event_name', {} as CaptureResult) + it('does not add eventCollector as a capture hook if disabled', () => { + posthog.config.opt_in_site_apps = false + siteAppsInstance.init() - expect(siteAppsInstance.missedInvocations.length).toBe(0) + expect(posthog._addCaptureHook).not.toHaveBeenCalled() + expect(siteAppsInstance['stopBuffering']).toBeUndefined() }) + }) - it('collects event if enabled and loaded is false', () => { - siteAppsInstance.enabled = true - siteAppsInstance.loaded = false - - const eventPayload = { event: 'test_event', properties: { prop1: 'value1' } } as CaptureResult - - jest.spyOn(siteAppsInstance, 'globalsForEvent').mockReturnValue({ some: 'globals' }) - - siteAppsInstance.eventCollector('test_event', eventPayload) + describe('eventCollector', () => { + beforeEach(() => { + siteAppsInstance.init() + }) - expect(siteAppsInstance.globalsForEvent).toHaveBeenCalledWith(eventPayload) - expect(siteAppsInstance.missedInvocations).toEqual([{ some: 'globals' }]) + it('collects events if enabled after init', () => { + emitCaptureEvent?.('test_event', { event: 'test_event', properties: { prop1: 'value1' } } as any) + + expect(siteAppsInstance['bufferedInvocations']).toMatchInlineSnapshot(` + Array [ + Object { + "event": Object { + "distinct_id": undefined, + "elements_chain": "", + "event": "test_event", + "properties": Object { + "prop1": "value1", + }, + }, + "groups": Object {}, + "person": Object { + "properties": undefined, + }, + }, + ] + `) }) it('trims missedInvocations to last 990 when exceeding 1000', () => { - siteAppsInstance.enabled = true - siteAppsInstance.loaded = false - - siteAppsInstance.missedInvocations = new Array(1000).fill({}) - - const eventPayload = { event: 'test_event', properties: { prop1: 'value1' } } as CaptureResult - - jest.spyOn(siteAppsInstance, 'globalsForEvent').mockReturnValue({ some: 'globals' }) + siteAppsInstance['bufferedInvocations'] = new Array(1000).fill({}) - siteAppsInstance.eventCollector('test_event', eventPayload) + emitCaptureEvent?.('test_event', { event: 'test_event', properties: { prop1: 'value1' } } as any) - expect(siteAppsInstance.missedInvocations.length).toBe(991) - expect(siteAppsInstance.missedInvocations[0]).toEqual({}) - expect(siteAppsInstance.missedInvocations[990]).toEqual({ some: 'globals' }) + expect(siteAppsInstance['bufferedInvocations'].length).toBe(991) + expect(siteAppsInstance['bufferedInvocations'][0]).toEqual({}) + expect(siteAppsInstance['bufferedInvocations'][990]).toMatchObject({ event: { event: 'test_event' } }) }) }) @@ -226,17 +224,17 @@ describe('SiteApps', () => { }) }) - describe('afterDecideResponse', () => { + describe('onRemoteConfig', () => { it('sets loaded to true and enabled to false when response is undefined', () => { siteAppsInstance.onRemoteConfig(undefined) expect(siteAppsInstance.loaded).toBe(true) - expect(siteAppsInstance.enabled).toBe(false) + expect(siteAppsInstance.isEnabled).toBe(false) }) it('loads site apps when enabled and opt_in_site_apps is true', (done) => { posthog.config.opt_in_site_apps = true - siteAppsInstance.enabled = true + siteAppsInstance.isEnabled = true const response = { siteApps: [ { id: '1', url: '/site_app/1' }, @@ -259,7 +257,7 @@ describe('SiteApps', () => { }) it('does not load site apps when enabled is false', () => { - siteAppsInstance.enabled = false + siteAppsInstance.isEnabled = false posthog.config.opt_in_site_apps = false const response = { siteApps: [{ id: '1', url: '/site_app/1' }], @@ -268,13 +266,13 @@ describe('SiteApps', () => { siteAppsInstance.onRemoteConfig(response) expect(siteAppsInstance.loaded).toBe(true) - expect(siteAppsInstance.enabled).toBe(false) + expect(siteAppsInstance.isEnabled).toBe(false) expect(assignableWindow.__PosthogExtensions__?.loadSiteApp).not.toHaveBeenCalled() }) it('clears missedInvocations when all apps are loaded', (done) => { posthog.config.opt_in_site_apps = true - siteAppsInstance.enabled = true + siteAppsInstance.isEnabled = true siteAppsInstance.missedInvocations = [{ some: 'data' }] const response = { siteApps: [{ id: '1', url: '/site_app/1' }], @@ -292,7 +290,7 @@ describe('SiteApps', () => { it('sets assignableWindow properties for each site app', () => { posthog.config.opt_in_site_apps = true - siteAppsInstance.enabled = true + siteAppsInstance.isEnabled = true const response = { siteApps: [{ id: '1', url: '/site_app/1' }], } as DecideResponse @@ -311,7 +309,7 @@ describe('SiteApps', () => { it('logs error if site apps are disabled but response contains site apps', () => { posthog.config.opt_in_site_apps = false - siteAppsInstance.enabled = false + siteAppsInstance.isEnabled = false const response = { siteApps: [{ id: '1', url: '/site_app/1' }], } as DecideResponse @@ -325,7 +323,7 @@ describe('SiteApps', () => { }) it('sets loaded to true if response.siteApps is empty', () => { - siteAppsInstance.enabled = true + siteAppsInstance.isEnabled = true posthog.config.opt_in_site_apps = true const response = { siteApps: [], @@ -334,7 +332,7 @@ describe('SiteApps', () => { siteAppsInstance.onRemoteConfig(response) expect(siteAppsInstance.loaded).toBe(true) - expect(siteAppsInstance.enabled).toBe(false) + expect(siteAppsInstance.isEnabled).toBe(false) }) }) }) diff --git a/src/site-apps.ts b/src/site-apps.ts index 815d6a400..4fdf30dc5 100644 --- a/src/site-apps.ts +++ b/src/site-apps.ts @@ -7,32 +7,36 @@ import { isArray } from './utils/type-utils' const logger = _logger.createLogger('[Site Apps]') export class SiteApps { - enabled: boolean apps: Record private stopBuffering?: () => void private bufferedInvocations: SiteAppGlobals[] constructor(private instance: PostHog) { - this.enabled = !!this.instance.config.opt_in_site_apps // events captured between loading posthog-js and the site app; up to 1000 events this.bufferedInvocations = [] this.apps = {} } - eventCollector(_eventName: string, eventPayload?: CaptureResult | undefined) { + public get isEnabled(): boolean { + return !!this.instance.config.opt_in_site_apps + } + + private eventCollector(_eventName: string, eventPayload?: CaptureResult | undefined) { + console.log('eventCollector', _eventName, eventPayload) if (!eventPayload) { return } const globals = this.globalsForEvent(eventPayload) this.bufferedInvocations.push(globals) + console.log('globals', globals, this.bufferedInvocations) if (this.bufferedInvocations.length > 1000) { this.bufferedInvocations = this.bufferedInvocations.slice(10) } } init() { - if (this.enabled) { + if (this.isEnabled) { const stop = this.instance._addCaptureHook(this.eventCollector.bind(this)) this.stopBuffering = () => { stop() @@ -140,7 +144,7 @@ export class SiteApps { onRemoteConfig(response: RemoteConfig): void { if (isArray(assignableWindow._POSTHOG_JS_APPS)) { - if (!this.enabled) { + if (!this.isEnabled) { logger.error(`PostHog site apps are disabled. Enable the "opt_in_site_apps" config to proceed.`) return } @@ -162,7 +166,7 @@ export class SiteApps { // NOTE: Below his is now only the fallback for legacy site app support. Once we have fully removed to the remote config loader we can get rid of this this.stopBuffering?.() - if (!this.enabled) { + if (!this.isEnabled) { logger.error(`PostHog site apps are disabled. Enable the "opt_in_site_apps" config to proceed.`) return } From d5af6f65f2f2f2e0a48a977399e7906e85f42c4b Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 5 Dec 2024 15:41:15 +0100 Subject: [PATCH 18/20] Fixes --- src/__tests__/site-apps.ts | 254 +++++++++++++++++++++++-------------- src/site-apps.ts | 10 +- 2 files changed, 169 insertions(+), 95 deletions(-) diff --git a/src/__tests__/site-apps.ts b/src/__tests__/site-apps.ts index bf1cbb867..d5b4698b6 100644 --- a/src/__tests__/site-apps.ts +++ b/src/__tests__/site-apps.ts @@ -2,26 +2,16 @@ import { SiteApps } from '../site-apps' import { PostHogPersistence } from '../posthog-persistence' import { RequestRouter } from '../utils/request-router' import { PostHog } from '../posthog-core' -import { DecideResponse, PostHogConfig, Properties, CaptureResult } from '../types' +import { PostHogConfig, Properties, CaptureResult, RemoteConfig } from '../types' import { assignableWindow } from '../utils/globals' -import { logger } from '../utils/logger' import '../entrypoints/external-scripts-loader' import { isFunction } from '../utils/type-utils' -jest.mock('../utils/logger', () => ({ - logger: { - error: jest.fn(), - createLogger: jest.fn(() => ({ - error: jest.fn(), - info: jest.fn(), - })), - }, -})) - describe('SiteApps', () => { let posthog: PostHog let siteAppsInstance: SiteApps let emitCaptureEvent: ((eventName: string, eventPayload: CaptureResult) => void) | undefined + let removeCaptureHook = jest.fn() const defaultConfig: Partial = { token: 'testtoken', @@ -49,7 +39,10 @@ describe('SiteApps', () => { }), } - const removeCaptureHook = jest.fn() + delete assignableWindow._POSTHOG_JS_APPS + delete assignableWindow.POSTHOG_DEBUG + + removeCaptureHook = jest.fn() posthog = { config: { ...defaultConfig, opt_in_site_apps: true }, @@ -73,6 +66,7 @@ describe('SiteApps', () => { requestRouter: new RequestRouter({ config: defaultConfig } as unknown as PostHog), _hasBootstrappedFeatureFlags: jest.fn(), getGroups: () => ({ organization: '5' }), + on: jest.fn(), } as unknown as PostHog siteAppsInstance = new SiteApps(posthog) @@ -224,115 +218,191 @@ describe('SiteApps', () => { }) }) - describe('onRemoteConfig', () => { - it('sets loaded to true and enabled to false when response is undefined', () => { - siteAppsInstance.onRemoteConfig(undefined) - - expect(siteAppsInstance.loaded).toBe(true) - expect(siteAppsInstance.isEnabled).toBe(false) + describe('legacy site apps loading', () => { + beforeEach(() => { + posthog.config.opt_in_site_apps = true + siteAppsInstance.init() }) - it('loads site apps when enabled and opt_in_site_apps is true', (done) => { + it('loads stops buffering if no site apps', () => { posthog.config.opt_in_site_apps = true - siteAppsInstance.isEnabled = true - const response = { - siteApps: [ - { id: '1', url: '/site_app/1' }, - { id: '2', url: '/site_app/2' }, - ], - } as DecideResponse - - siteAppsInstance.onRemoteConfig(response) + siteAppsInstance.onRemoteConfig({} as RemoteConfig) - expect(siteAppsInstance.appsLoading.size).toBe(2) - expect(siteAppsInstance.loaded).toBe(false) - - // Wait for the simulated async loading to complete - setTimeout(() => { - expect(assignableWindow.__PosthogExtensions__?.loadSiteApp).toHaveBeenCalledTimes(2) - expect(siteAppsInstance.appsLoading.size).toBe(0) - expect(siteAppsInstance.loaded).toBe(true) - done() - }, 10) + expect(removeCaptureHook).toHaveBeenCalled() + expect(siteAppsInstance['stopBuffering']).toBeUndefined() + expect(assignableWindow.__PosthogExtensions__?.loadSiteApp).not.toHaveBeenCalled() }) - it('does not load site apps when enabled is false', () => { - siteAppsInstance.isEnabled = false + it('does not loads site apps if disabled', () => { posthog.config.opt_in_site_apps = false - const response = { - siteApps: [{ id: '1', url: '/site_app/1' }], - } as DecideResponse - - siteAppsInstance.onRemoteConfig(response) + siteAppsInstance.onRemoteConfig({ + siteApps: [ + { id: '1', url: '/site_app/1' }, + { id: '2', url: '/site_app/2' }, + ], + } as RemoteConfig) - expect(siteAppsInstance.loaded).toBe(true) - expect(siteAppsInstance.isEnabled).toBe(false) + expect(removeCaptureHook).toHaveBeenCalled() + expect(siteAppsInstance['stopBuffering']).toBeUndefined() expect(assignableWindow.__PosthogExtensions__?.loadSiteApp).not.toHaveBeenCalled() }) - it('clears missedInvocations when all apps are loaded', (done) => { - posthog.config.opt_in_site_apps = true - siteAppsInstance.isEnabled = true - siteAppsInstance.missedInvocations = [{ some: 'data' }] - const response = { + it('does not load site apps if new global loader exists', () => { + assignableWindow._POSTHOG_JS_APPS = [] + siteAppsInstance.onRemoteConfig({ siteApps: [{ id: '1', url: '/site_app/1' }], - } as DecideResponse + } as RemoteConfig) - siteAppsInstance.onRemoteConfig(response) - - // Wait for the simulated async loading to complete - setTimeout(() => { - expect(siteAppsInstance.loaded).toBe(true) - expect(siteAppsInstance.missedInvocations).toEqual([]) - done() - }, 10) + expect(assignableWindow.__PosthogExtensions__?.loadSiteApp).not.toHaveBeenCalled() }) - it('sets assignableWindow properties for each site app', () => { - posthog.config.opt_in_site_apps = true - siteAppsInstance.isEnabled = true - const response = { - siteApps: [{ id: '1', url: '/site_app/1' }], - } as DecideResponse - - siteAppsInstance.onRemoteConfig(response) + it('loads site apps if new global loader is not available', () => { + siteAppsInstance.onRemoteConfig({ + siteApps: [ + { id: '1', url: '/site_app/1' }, + { id: '2', url: '/site_app/2' }, + ], + } as RemoteConfig) - expect(assignableWindow['__$$ph_site_app_1']).toBe(posthog) - expect(typeof assignableWindow['__$$ph_site_app_1_missed_invocations']).toBe('function') - expect(typeof assignableWindow['__$$ph_site_app_1_callback']).toBe('function') + expect(removeCaptureHook).toHaveBeenCalled() + expect(siteAppsInstance['stopBuffering']).toBeUndefined() + expect(assignableWindow.__PosthogExtensions__?.loadSiteApp).toHaveBeenCalledTimes(2) expect(assignableWindow.__PosthogExtensions__?.loadSiteApp).toHaveBeenCalledWith( posthog, '/site_app/1', expect.any(Function) ) + expect(assignableWindow.__PosthogExtensions__?.loadSiteApp).toHaveBeenCalledWith( + posthog, + '/site_app/2', + expect.any(Function) + ) + }) + }) + + describe('onRemoteConfig', () => { + let appConfigs: { + posthog: PostHog + callback: (success: boolean) => void + }[] = [] + + beforeEach(() => { + appConfigs = [] + assignableWindow._POSTHOG_JS_APPS = [ + { + id: '1', + init: jest.fn((config) => { + appConfigs.push(config) + return { + processEvent: jest.fn(), + } + }), + }, + { + id: '2', + init: jest.fn((config) => { + appConfigs.push(config) + return { + processEvent: jest.fn(), + } + }), + }, + ] + + siteAppsInstance.init() }) - it('logs error if site apps are disabled but response contains site apps', () => { - posthog.config.opt_in_site_apps = false - siteAppsInstance.isEnabled = false - const response = { - siteApps: [{ id: '1', url: '/site_app/1' }], - } as DecideResponse + it('sets up the eventCaptured listener if site apps', () => { + siteAppsInstance.onRemoteConfig({} as RemoteConfig) + expect(posthog.on).toHaveBeenCalledWith('eventCaptured', expect.any(Function)) + }) + + it('does not sets up the eventCaptured listener if no site apps', () => { + assignableWindow._POSTHOG_JS_APPS = [] + siteAppsInstance.onRemoteConfig({} as RemoteConfig) + expect(posthog.on).not.toHaveBeenCalled() + }) + + it('loads site apps via the window object if defined', () => { + siteAppsInstance.onRemoteConfig({} as RemoteConfig) + expect(appConfigs[0]).toBeDefined() + expect(siteAppsInstance.apps['1']).toEqual({ + errored: false, + loaded: false, + id: '1', + processEvent: expect.any(Function), + }) + + appConfigs[0].callback(true) + + expect(siteAppsInstance.apps['1']).toEqual({ + errored: false, + loaded: true, + id: '1', + processEvent: expect.any(Function), + }) + }) + + it('marks site app as errored if callback fails', () => { + siteAppsInstance.onRemoteConfig({} as RemoteConfig) + expect(appConfigs[0]).toBeDefined() + expect(siteAppsInstance.apps['1']).toMatchObject({ + errored: false, + loaded: false, + }) + + appConfigs[0].callback(false) + + expect(siteAppsInstance.apps['1']).toMatchObject({ + errored: true, + loaded: true, + }) + }) - siteAppsInstance.onRemoteConfig(response) + it('calls the processEvent method if it exists and events are buffered', () => { + emitCaptureEvent?.('test_event1', { event: 'test_event1' } as any) + siteAppsInstance.onRemoteConfig({} as RemoteConfig) + emitCaptureEvent?.('test_event2', { event: 'test_event2' } as any) + expect(siteAppsInstance['bufferedInvocations'].length).toBe(2) + appConfigs[0].callback(true) - expect(logger.error).toHaveBeenCalledWith( - 'PostHog site apps are disabled. Enable the "opt_in_site_apps" config to proceed.' + expect(siteAppsInstance.apps['1'].processEvent).toHaveBeenCalledTimes(2) + expect(siteAppsInstance.apps['1'].processEvent).toHaveBeenCalledWith( + siteAppsInstance.globalsForEvent({ event: 'test_event1' } as any) + ) + expect(siteAppsInstance.apps['1'].processEvent).toHaveBeenCalledWith( + siteAppsInstance.globalsForEvent({ event: 'test_event2' } as any) ) - expect(siteAppsInstance.loaded).toBe(true) }) - it('sets loaded to true if response.siteApps is empty', () => { - siteAppsInstance.isEnabled = true - posthog.config.opt_in_site_apps = true - const response = { - siteApps: [], - } as DecideResponse + it('clears the buffer after all apps are loaded', () => { + emitCaptureEvent?.('test_event1', { event: 'test_event1' } as any) + emitCaptureEvent?.('test_event2', { event: 'test_event2' } as any) + expect(siteAppsInstance['bufferedInvocations'].length).toBe(2) + + siteAppsInstance.onRemoteConfig({} as RemoteConfig) + appConfigs[0].callback(true) + expect(siteAppsInstance['bufferedInvocations'].length).toBe(2) + appConfigs[1].callback(false) + expect(siteAppsInstance['bufferedInvocations'].length).toBe(0) + }) - siteAppsInstance.onRemoteConfig(response) + it('logs error if site apps are disabled but response contains site apps', () => { + posthog.config.opt_in_site_apps = false + assignableWindow.POSTHOG_DEBUG = true - expect(siteAppsInstance.loaded).toBe(true) - expect(siteAppsInstance.isEnabled).toBe(false) + siteAppsInstance.onRemoteConfig({} as RemoteConfig) + + expect(window.console.error).toBeCalledTimes(1) + expect(window.console.error.mock.calls[0]).toMatchInlineSnapshot(` + Array [ + "[PostHog.js] [Site Apps]", + "PostHog site apps are disabled. Enable the \\"opt_in_site_apps\\" config to proceed.", + ] + `) + expect(siteAppsInstance.apps).toEqual({}) + + expect(removeCaptureHook).toHaveBeenCalled() }) }) }) diff --git a/src/site-apps.ts b/src/site-apps.ts index 4fdf30dc5..70d897e3a 100644 --- a/src/site-apps.ts +++ b/src/site-apps.ts @@ -23,13 +23,11 @@ export class SiteApps { } private eventCollector(_eventName: string, eventPayload?: CaptureResult | undefined) { - console.log('eventCollector', _eventName, eventPayload) if (!eventPayload) { return } const globals = this.globalsForEvent(eventPayload) this.bufferedInvocations.push(globals) - console.log('globals', globals, this.bufferedInvocations) if (this.bufferedInvocations.length > 1000) { this.bufferedInvocations = this.bufferedInvocations.slice(10) } @@ -40,6 +38,7 @@ export class SiteApps { const stop = this.instance._addCaptureHook(this.eventCollector.bind(this)) this.stopBuffering = () => { stop() + this.bufferedInvocations = [] this.stopBuffering = undefined } } @@ -89,7 +88,7 @@ export class SiteApps { this.apps[loader.id] = app const onLoaded = (success: boolean) => { - this.apps[loader.id].errored = success + this.apps[loader.id].errored = !success this.apps[loader.id].loaded = true logger.info(`Site app with id ${loader.id} ${success ? 'loaded' : 'errored'}`) @@ -166,6 +165,11 @@ export class SiteApps { // NOTE: Below his is now only the fallback for legacy site app support. Once we have fully removed to the remote config loader we can get rid of this this.stopBuffering?.() + + if (!response['siteApps']?.length) { + return + } + if (!this.isEnabled) { logger.error(`PostHog site apps are disabled. Enable the "opt_in_site_apps" config to proceed.`) return From fbef34a7237e64d26d6e80868294457d771c4a22 Mon Sep 17 00:00:00 2001 From: Ben White Date: Thu, 5 Dec 2024 15:41:39 +0100 Subject: [PATCH 19/20] Fixes --- src/__tests__/site-apps.ts | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/__tests__/site-apps.ts b/src/__tests__/site-apps.ts index d5b4698b6..da4de17cb 100644 --- a/src/__tests__/site-apps.ts +++ b/src/__tests__/site-apps.ts @@ -401,8 +401,6 @@ describe('SiteApps', () => { ] `) expect(siteAppsInstance.apps).toEqual({}) - - expect(removeCaptureHook).toHaveBeenCalled() }) }) }) From a8c05fc975c7a8c02b004dae91e3792facd34777 Mon Sep 17 00:00:00 2001 From: Ben White Date: Fri, 6 Dec 2024 10:28:39 +0100 Subject: [PATCH 20/20] Update src/types.ts --- src/types.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types.ts b/src/types.ts index 320b85019..303b501b1 100644 --- a/src/types.ts +++ b/src/types.ts @@ -341,7 +341,7 @@ export interface PostHogConfig { /** * PREVIEW - MAY CHANGE WITHOUT WARNING - DO NOT USE IN PRODUCTION - * whether to wrap fetch and add tracing headers to the request + * enables the new RemoteConfig approach to loading config instead of decide * */ __preview_remote_config?: boolean }