diff --git a/cypress/e2e/capture.cy.ts b/cypress/e2e/capture.cy.ts index c3cd27214..56e6fc618 100644 --- a/cypress/e2e/capture.cy.ts +++ b/cypress/e2e/capture.cy.ts @@ -180,7 +180,7 @@ describe('Event capture', () => { }, advanced_disable_feature_flags: true, }, - waitForDecide: false, + waitForFeatureFlags: false, }) cy.intercept( @@ -240,13 +240,17 @@ describe('Event capture', () => { it('makes a single decide request', () => { start({}) - cy.get('@decide.all').then((calls) => { + cy.get('@remote-config.all').then((calls) => { + expect(calls.length).to.equal(1) + }) + + cy.get('@feature-flags.all').then((calls) => { expect(calls.length).to.equal(1) }) cy.phCaptures().should('include', '$pageview') // @ts-expect-error - TS is wrong that get returns HTMLElement here - cy.get('@decide').should(({ request }) => { + cy.get('@feature-flags').should(({ request }) => { const payload = getBase64EncodedPayload(request) expect(payload.token).to.equal('test_token') expect(payload.groups).to.deep.equal({}) @@ -255,7 +259,7 @@ describe('Event capture', () => { describe('when disabled', () => { it('captures pageviews, custom events when autocapture disabled', () => { - start({ options: { autocapture: false }, waitForDecide: false }) + start({ options: { autocapture: false }, waitForRemoteConfig: false }) cy.wait(50) cy.get('[data-cy-custom-event-button]').click() @@ -328,7 +332,7 @@ describe('Event capture', () => { describe('advanced_disable_decide config', () => { it('does not autocapture anything when /decide is disabled', () => { - start({ options: { autocapture: false, advanced_disable_decide: true }, waitForDecide: false }) + start({ options: { autocapture: false, advanced_disable_decide: true }, waitForFeatureFlags: false }) cy.get('body').click(100, 100) cy.get('body').click(98, 102) @@ -342,7 +346,11 @@ describe('Event capture', () => { }) it('does not capture session recordings', () => { - start({ options: { advanced_disable_decide: true }, waitForDecide: false }) + start({ + options: { advanced_disable_decide: true }, + waitForRemoteConfig: false, + waitForFeatureFlags: false, + }) cy.get('[data-cy-custom-event-button]').click() cy.wait('@capture') diff --git a/cypress/e2e/error-tracking.cy.ts b/cypress/e2e/error-tracking.cy.ts index 419d436ee..700bb59eb 100644 --- a/cypress/e2e/error-tracking.cy.ts +++ b/cypress/e2e/error-tracking.cy.ts @@ -3,7 +3,7 @@ import { start } from '../support/setup' describe('Exception capture', () => { it('manual exception capture', () => { start({ - decideResponseOverrides: { + remoteConfigOverrides: { autocaptureExceptions: false, }, url: './playground/cypress', @@ -33,7 +33,7 @@ describe('Exception capture', () => { }) start({ - decideResponseOverrides: { + remoteConfigOverrides: { autocaptureExceptions: true, }, url: './playground/cypress', diff --git a/cypress/e2e/opting-out.cy.ts b/cypress/e2e/opting-out.cy.ts index e7bada65a..3c488eba6 100644 --- a/cypress/e2e/opting-out.cy.ts +++ b/cypress/e2e/opting-out.cy.ts @@ -1,5 +1,5 @@ import { assertWhetherPostHogRequestsWereCalled, pollPhCaptures } from '../support/assertions' -import { start } from '../support/setup' +import { interceptRemoteConfig, start } from '../support/setup' function assertThatRecordingStarted() { cy.phCaptures({ full: true }).then((captures) => { @@ -15,16 +15,13 @@ function assertThatRecordingStarted() { describe('opting out', () => { describe('session recording', () => { beforeEach(() => { - cy.intercept('POST', '/decide/*', { - editorParams: {}, - featureFlags: ['session-recording-player'], - isAuthenticated: false, + interceptRemoteConfig({ sessionRecording: { endpoint: '/ses/', }, - capture_performance: true, + capturePerformance: true, autocapture_opt_out: true, - }).as('decide') + }) cy.visit('./playground/cypress') }) @@ -34,7 +31,7 @@ describe('opting out', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': false, - '@decide': false, + '@remote-config': false, '@session-recording': false, }) @@ -52,7 +49,7 @@ describe('opting out', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': false, - '@decide': true, + '@remote-config': true, '@session-recording': false, }) @@ -70,7 +67,7 @@ describe('opting out', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': false, - '@decide': true, + '@remote-config': true, '@session-recording': false, }) @@ -88,7 +85,7 @@ describe('opting out', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': false, - '@decide': true, + '@remote-config': true, '@session-recording': false, }) @@ -102,7 +99,7 @@ describe('opting out', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': true, - '@decide': true, + '@remote-config': true, // no call to session-recording yet }) @@ -118,7 +115,7 @@ describe('opting out', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': false, - '@decide': true, + '@remote-config': true, '@session-recording': false, }) @@ -135,7 +132,7 @@ describe('opting out', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': true, - '@decide': true, + '@remote-config': true, // no call to session-recording yet }) @@ -147,16 +144,14 @@ describe('opting out', () => { }) it('can override sampling when starting session recording', () => { - cy.intercept('POST', '/decide/*', { - autocapture_opt_out: true, - editorParams: {}, - isAuthenticated: false, + interceptRemoteConfig({ sessionRecording: { endpoint: '/ses/', // will never record a session with rate of 0 sampleRate: '0', }, - }).as('decide') + autocapture_opt_out: true, + }) cy.posthogInit({ opt_out_capturing_by_default: true, @@ -164,7 +159,7 @@ describe('opting out', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': false, - '@decide': true, + '@remote-config': true, '@session-recording': false, }) @@ -178,7 +173,7 @@ describe('opting out', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': true, - '@decide': true, + '@remote-config': true, // no call to session-recording yet }) @@ -190,16 +185,14 @@ describe('opting out', () => { }) it('can override linked_flags when starting session recording', () => { - cy.intercept('POST', '/decide/*', { - autocapture_opt_out: true, - editorParams: {}, - isAuthenticated: false, + interceptRemoteConfig({ sessionRecording: { endpoint: '/ses/', // a flag that doesn't exist, can never be recorded linkedFlag: 'i am a flag that does not exist', }, - }).as('decide') + autocapture_opt_out: true, + }) cy.posthogInit({ opt_out_capturing_by_default: true, @@ -207,7 +200,7 @@ describe('opting out', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': false, - '@decide': true, + '@remote-config': true, '@session-recording': false, }) @@ -221,7 +214,7 @@ describe('opting out', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': true, - '@decide': true, + '@remote-config': true, // no call to session-recording yet }) @@ -244,10 +237,8 @@ describe('opting out', () => { }) it('respects sampling when overriding linked_flags when starting session recording', () => { - cy.intercept('POST', '/decide/*', { + interceptRemoteConfig({ autocapture_opt_out: true, - editorParams: {}, - isAuthenticated: false, sessionRecording: { endpoint: '/ses/', // a flag that doesn't exist, can never be recorded @@ -255,7 +246,7 @@ describe('opting out', () => { // will never record a session with rate of 0 sampleRate: '0', }, - }).as('decide') + }) cy.posthogInit({ opt_out_capturing_by_default: true, @@ -263,7 +254,7 @@ describe('opting out', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': false, - '@decide': true, + '@remote-config': true, '@session-recording': false, }) @@ -277,7 +268,7 @@ describe('opting out', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': true, - '@decide': true, + '@remote-config': true, // no call to session-recording yet }) @@ -305,10 +296,8 @@ describe('opting out', () => { }) it('can override all ingestion controls when starting session recording', () => { - cy.intercept('POST', '/decide/*', { + interceptRemoteConfig({ autocapture_opt_out: true, - editorParams: {}, - isAuthenticated: false, sessionRecording: { endpoint: '/ses/', // a flag that doesn't exist, can never be recorded @@ -316,7 +305,7 @@ describe('opting out', () => { // will never record a session with rate of 0 sampleRate: '0', }, - }).as('decide') + }) cy.posthogInit({ opt_out_capturing_by_default: true, @@ -324,7 +313,7 @@ describe('opting out', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': false, - '@decide': true, + '@remote-config': true, '@session-recording': false, }) @@ -338,7 +327,7 @@ describe('opting out', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': true, - '@decide': true, + '@remote-config': true, // no call to session-recording yet }) @@ -361,16 +350,14 @@ describe('opting out', () => { }) it('sends a $pageview event when opting in', () => { - cy.intercept('POST', '/decide/*', { + interceptRemoteConfig({ autocapture_opt_out: true, - editorParams: {}, - isAuthenticated: false, sessionRecording: { endpoint: '/ses/', // will never record a session with rate of 0 sampleRate: '0', }, - }).as('decide') + }) cy.posthogInit({ opt_out_capturing_by_default: true, @@ -389,16 +376,14 @@ describe('opting out', () => { }) it('does not send a duplicate $pageview event when opting in', () => { - cy.intercept('POST', '/decide/*', { + interceptRemoteConfig({ autocapture_opt_out: true, - editorParams: {}, - isAuthenticated: false, sessionRecording: { endpoint: '/ses/', // will never record a session with rate of 0 sampleRate: '0', }, - }).as('decide') + }) cy.posthogInit({}) // Wait for the pageview timeout diff --git a/cypress/e2e/session-recording.cy.ts b/cypress/e2e/session-recording.cy.ts index 0875c32dd..8e12e604b 100644 --- a/cypress/e2e/session-recording.cy.ts +++ b/cypress/e2e/session-recording.cy.ts @@ -1,7 +1,7 @@ /// import { isNull } from '../../src/utils/type-utils' -import { start } from '../support/setup' +import { interceptRemoteConfig, start } from '../support/setup' import { assertWhetherPostHogRequestsWereCalled, pollPhCaptures } from '../support/assertions' interface RRWebCustomEvent { @@ -117,8 +117,7 @@ describe('Session recording', () => { options: { session_recording: {}, }, - decideResponseOverrides: { - isAuthenticated: false, + remoteConfigOverrides: { sessionRecording: { endpoint: '/ses/', }, @@ -179,8 +178,7 @@ describe('Session recording', () => { }) start({ - decideResponseOverrides: { - isAuthenticated: false, + remoteConfigOverrides: { sessionRecording: { endpoint: '/ses/', networkPayloadCapture: { recordBody: true }, @@ -247,6 +245,7 @@ describe('Session recording', () => { const expectedCaptureds: [RegExp, string][] = [ [/http:\/\/localhost:\d+\/playground\/cypress\//, 'navigation'], [/http:\/\/localhost:\d+\/static\/array.js/, 'script'], + [/http:\/\/localhost:\d+\/array\/\.+\/config.js/, 'script'], [ /http:\/\/localhost:\d+\/decide\/\?v=3&ip=1&_=\d+&ver=1\.\d\d\d\.\d+&compression=base64/, 'xmlhttprequest', @@ -345,8 +344,7 @@ describe('Session recording', () => { options: { session_recording: {}, }, - decideResponseOverrides: { - isAuthenticated: false, + remoteConfigOverrides: { sessionRecording: { endpoint: '/ses/', }, @@ -482,7 +480,7 @@ describe('Session recording', () => { cy.posthogInit({ session_recording: {}, }) - cy.wait('@decide') + cy.wait('@remote-config') cy.wait('@recorder-script') cy.get('body') @@ -651,8 +649,7 @@ describe('Session recording', () => { options: { session_recording: {}, }, - decideResponseOverrides: { - isAuthenticated: false, + remoteConfigOverrides: { sessionRecording: { endpoint: '/ses/', sampleRate: '0', @@ -679,20 +676,18 @@ describe('Session recording', () => { }) it('can override sampling when starting session recording', () => { - cy.intercept('POST', '/decide/*', { + interceptRemoteConfig({ autocapture_opt_out: true, - editorParams: {}, - isAuthenticated: false, sessionRecording: { endpoint: '/ses/', // will never record a session with rate of 0 sampleRate: '0', }, - }).as('decide') + }) assertWhetherPostHogRequestsWereCalled({ '@recorder-script': true, - '@decide': true, + '@remote-config': true, '@session-recording': false, }) @@ -704,7 +699,7 @@ describe('Session recording', () => { assertWhetherPostHogRequestsWereCalled({ '@recorder-script': true, - '@decide': true, + '@remote-config': true, // no call to session-recording yet }) @@ -729,8 +724,7 @@ describe('Session recording', () => { cy.resetPhCaptures() cy.reload(true).then(() => { start({ - decideResponseOverrides: { - isAuthenticated: false, + remoteConfigOverrides: { sessionRecording: { endpoint: '/ses/', sampleRate: '0', diff --git a/cypress/e2e/surveys.cy.ts b/cypress/e2e/surveys.cy.ts index cc80fb0bd..73cfec540 100644 --- a/cypress/e2e/surveys.cy.ts +++ b/cypress/e2e/surveys.cy.ts @@ -1,6 +1,7 @@ /// import { getPayload } from '../support/compression' import 'cypress-localstorage-commands' +import { interceptRemoteConfig } from '../support/setup' function onPageLoad(options = {}) { cy.posthog().then((ph) => { @@ -8,7 +9,7 @@ function onPageLoad(options = {}) { }) cy.posthogInit(options) - cy.wait('@decide') + cy.wait('@remote-config') cy.wait('@surveys') } @@ -63,12 +64,10 @@ describe('Surveys', () => { } beforeEach(() => { - cy.intercept('POST', '**/decide/*', { - editorParams: {}, - surveys: true, - isAuthenticated: false, + interceptRemoteConfig({ + surveys: [], autocapture_opt_out: true, - }).as('decide') + }) }) describe('Core display logic', () => { diff --git a/cypress/support/assertions.ts b/cypress/support/assertions.ts index fd2bac4f1..f6a56868d 100644 --- a/cypress/support/assertions.ts +++ b/cypress/support/assertions.ts @@ -18,7 +18,7 @@ export function pollPhCaptures(event, wait = 200, attempts = 0, maxAttempts = 50 /** * Receives an object with keys as the name of the route and values as whether the route should have been called. - * e.g. { '@recorder': true, '@decide': false } + * e.g. { '@recorder': true, '@remote-config': false } * the keys must match a `cy.intercept` alias **/ export function assertWhetherPostHogRequestsWereCalled(expectedCalls: Record): Chainable { diff --git a/cypress/support/e2e.ts b/cypress/support/e2e.ts index 813d60cef..2994b6c3d 100644 --- a/cypress/support/e2e.ts +++ b/cypress/support/e2e.ts @@ -10,7 +10,8 @@ Cypress.on('window:before:load', (win) => { }) beforeEach(() => { - cy.intercept('POST', '/decide/*').as('decide') + cy.intercept('GET', '/array/*/config*').as('remote-config') + cy.intercept('POST', '/decide/*').as('feature-flags') cy.intercept('POST', '/e/*', { status: 1 }).as('capture') cy.intercept('POST', '/ses/*', { status: 1 }).as('session-recording') cy.intercept('GET', '/surveys/*').as('surveys') diff --git a/cypress/support/setup.ts b/cypress/support/setup.ts index ae9326a78..90d64502c 100644 --- a/cypress/support/setup.ts +++ b/cypress/support/setup.ts @@ -1,24 +1,37 @@ -import { DecideResponse, PostHogConfig } from '../../src/types' +import { Compression, DecideResponse, PostHogConfig, RemoteConfig } from '../../src/types' import { EventEmitter } from 'events' +export const interceptRemoteConfig = (remoteConfigOverrides: Partial) => { + cy.intercept('GET', '/array/*/config*', remoteConfigOverrides).as('remote-config') + // We force the config.js to be a 404 as we don't want to test it + cy.intercept('GET', '/array/*/config.js', { statusCode: 404 }).as('remote-config-js') +} + +export const interceptFeatureFlags = (featureFlagsOverrides: Partial) => { + cy.intercept('POST', '/decide/*', featureFlagsOverrides).as('feature-flags') +} + export const start = ({ - waitForDecide = true, + waitForRemoteConfig = true, + waitForFeatureFlags = true, initPosthog = true, resetOnInit = false, options = {}, - decideResponseOverrides = { + remoteConfigOverrides = { sessionRecording: undefined, - isAuthenticated: false, capturePerformance: true, }, + featureFlagsOverrides = {}, url = './playground/cypress-full', }: { - waitForDecide?: boolean + waitForRemoteConfig?: boolean + waitForFeatureFlags?: boolean initPosthog?: boolean resetOnInit?: boolean options?: Partial - decideResponseOverrides?: Partial + featureFlagsOverrides?: Partial + remoteConfigOverrides?: Partial url?: string }) => { // sometimes we have too many listeners in this test environment @@ -26,15 +39,20 @@ export const start = ({ // we don't see the error in production, so it's fine to increase the limit here EventEmitter.prototype.setMaxListeners(100) - const decideResponse = { - editorParams: {}, - featureFlags: ['session-recording-player'], - supportedCompression: ['gzip-js'], - excludedDomains: [], + const remoteConfigResponse: Partial = { + supportedCompression: [Compression.GZipJS], autocaptureExceptions: false, - ...decideResponseOverrides, + hasFeatureFlags: true, + ...remoteConfigOverrides, } - cy.intercept('POST', '/decide/*', decideResponse).as('decide') + + const featureFlagsResponse: Partial = { + featureFlags: { 'session-recording-player': true }, + ...featureFlagsOverrides, + } + + interceptFeatureFlags(featureFlagsResponse) + interceptRemoteConfig(remoteConfigResponse) cy.visit(url) @@ -49,7 +67,12 @@ export const start = ({ cy.posthog().invoke('reset', true) } - if (waitForDecide) { - cy.wait('@decide') + if (waitForRemoteConfig) { + cy.wait('@remote-config-js') // JS fires before the normal config + cy.wait('@remote-config') + } + + if (waitForFeatureFlags) { + cy.wait('@feature-flags') } } diff --git a/playground/nextjs/src/posthog.ts b/playground/nextjs/src/posthog.ts index 89de3feb1..0f592eca1 100644 --- a/playground/nextjs/src/posthog.ts +++ b/playground/nextjs/src/posthog.ts @@ -63,7 +63,6 @@ if (typeof window !== 'undefined') { 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 diff --git a/src/__tests__/featureflags.test.ts b/src/__tests__/featureflags.test.ts index a54ada8fd..f2ca11828 100644 --- a/src/__tests__/featureflags.test.ts +++ b/src/__tests__/featureflags.test.ts @@ -272,73 +272,6 @@ describe('featureflags', () => { }) }) - describe('decide()', () => { - it('should not call decide if advanced_disable_decide is true', () => { - instance.config.advanced_disable_decide = true - featureFlags.decide() - - expect(instance._send_request).toHaveBeenCalledTimes(0) - }) - - it('should call decide', () => { - featureFlags.decide() - - expect(instance._send_request).toHaveBeenCalledTimes(1) - expect(instance._send_request.mock.calls[0][0].data.disable_flags).toBe(undefined) - - jest.runOnlyPendingTimers() - expect(instance._send_request).toHaveBeenCalledTimes(1) - }) - - it('should call decide with flags disabled if set', () => { - instance.config.advanced_disable_feature_flags_on_first_load = true - featureFlags.decide() - - expect(instance._send_request).toHaveBeenCalledTimes(1) - expect(instance._send_request.mock.calls[0][0].data.disable_flags).toBe(true) - }) - - it('should call decide with flags disabled if set generally', () => { - instance.config.advanced_disable_feature_flags = true - featureFlags.decide() - - expect(instance._send_request).toHaveBeenCalledTimes(1) - expect(instance._send_request.mock.calls[0][0].data.disable_flags).toBe(true) - }) - - it('should call decide once even if reload called before', () => { - featureFlags.reloadFeatureFlags() - featureFlags.decide() - - expect(instance._send_request).toHaveBeenCalledTimes(1) - expect(instance._send_request.mock.calls[0][0].data.disable_flags).toBe(undefined) - - jest.runOnlyPendingTimers() - expect(instance._send_request).toHaveBeenCalledTimes(1) - }) - - it('should not disable flags if reload was called on decide', () => { - instance.config.advanced_disable_feature_flags_on_first_load = true - featureFlags.reloadFeatureFlags() - featureFlags.decide() - - expect(instance._send_request).toHaveBeenCalledTimes(1) - expect(instance._send_request.mock.calls[0][0].data.disable_flags).toBe(undefined) - - jest.runOnlyPendingTimers() - expect(instance._send_request).toHaveBeenCalledTimes(1) - }) - - it('should always disable flags if set', () => { - instance.config.advanced_disable_feature_flags = true - featureFlags.reloadFeatureFlags() - featureFlags.decide() - - expect(instance._send_request).toHaveBeenCalledTimes(1) - expect(instance._send_request.mock.calls[0][0].data.disable_flags).toBe(true) - }) - }) - describe('onFeatureFlags', () => { beforeEach(() => { instance._send_request = jest.fn().mockImplementation(({ callback }) => diff --git a/src/__tests__/posthog-core.identify.test.ts b/src/__tests__/posthog-core.identify.test.ts index 885ec25f9..25b7bded4 100644 --- a/src/__tests__/posthog-core.identify.test.ts +++ b/src/__tests__/posthog-core.identify.test.ts @@ -9,6 +9,7 @@ describe('identify()', () => { beforeEach(() => { beforeSendMock = jest.fn().mockImplementation((e) => e) + const posthog = defaultPostHog().init( uuidv7(), { diff --git a/src/__tests__/posthog-core.loaded.test.ts b/src/__tests__/posthog-core.loaded.test.ts index 3ef6c6805..c66fd9d1e 100644 --- a/src/__tests__/posthog-core.loaded.test.ts +++ b/src/__tests__/posthog-core.loaded.test.ts @@ -38,9 +38,11 @@ describe('loaded() with flags', () => { }, }) - expect(instance._send_request).toHaveBeenCalledTimes(1) + jest.runOnlyPendingTimers() // Remote config load - expect(instance._send_request.mock.calls[0][0]).toMatchObject({ + expect(instance.featureFlags._callDecideEndpoint).toHaveBeenCalledTimes(1) + + expect(instance._send_request.mock.calls[1][0]).toMatchObject({ url: 'https://us.i.posthog.com/decide/?v=3', data: { groups: { org: 'bazinga' }, @@ -48,7 +50,7 @@ describe('loaded() with flags', () => { }) jest.runOnlyPendingTimers() // Once for callback jest.runOnlyPendingTimers() // Once for potential debounce - expect(instance._send_request).toHaveBeenCalledTimes(1) + expect(instance.featureFlags._callDecideEndpoint).toHaveBeenCalledTimes(1) }) it('does add follow up call due to group change', async () => { @@ -60,10 +62,12 @@ describe('loaded() with flags', () => { }, 100) }, }) - expect(instance.featureFlags._callDecideEndpoint).toHaveBeenCalledTimes(1) - expect(instance._send_request).toHaveBeenCalledTimes(1) - expect(instance._send_request.mock.calls[0][0]).toMatchObject({ + jest.runOnlyPendingTimers() // Remote config load + + expect(instance.featureFlags._callDecideEndpoint).toHaveBeenCalledTimes(2) + + expect(instance._send_request.mock.calls[1][0]).toMatchObject({ url: 'https://us.i.posthog.com/decide/?v=3', data: { groups: { org: 'bazinga' }, @@ -74,9 +78,8 @@ describe('loaded() with flags', () => { jest.runOnlyPendingTimers() // Once for potential debounce expect(instance.featureFlags._callDecideEndpoint).toHaveBeenCalledTimes(2) - expect(instance._send_request).toHaveBeenCalledTimes(2) - expect(instance._send_request.mock.calls[1][0]).toMatchObject({ + expect(instance._send_request.mock.calls[2][0]).toMatchObject({ url: 'https://us.i.posthog.com/decide/?v=3', data: { groups: { org: 'bazinga2' }, @@ -94,16 +97,17 @@ describe('loaded() with flags', () => { expect(instance.config.advanced_disable_feature_flags_on_first_load).toBe(true) + jest.runOnlyPendingTimers() // Once for remote-config callback + jest.runOnlyPendingTimers() // Once for potential debounce + expect(instance.featureFlags._callDecideEndpoint).toHaveBeenCalledTimes(1) - expect(instance._send_request).toHaveBeenCalledTimes(1) - expect(instance._send_request.mock.calls[0][0].data.disable_flags).toEqual(undefined) + expect(instance._send_request.mock.calls[1][0].data.disable_flags).toEqual(undefined) jest.runOnlyPendingTimers() // Once for callback jest.runOnlyPendingTimers() // Once for potential debounce expect(instance.featureFlags._callDecideEndpoint).toHaveBeenCalledTimes(1) - expect(instance._send_request).toHaveBeenCalledTimes(1) }) }) }) diff --git a/src/__tests__/posthog-core.ts b/src/__tests__/posthog-core.ts index 2ef22b6ee..35f16d35d 100644 --- a/src/__tests__/posthog-core.ts +++ b/src/__tests__/posthog-core.ts @@ -1106,17 +1106,17 @@ describe('posthog core', () => { expect(mockLogger.critical).toHaveBeenCalledWith('`loaded` function failed', expect.anything()) }) - describe('/decide', () => { + describe('remote-config', () => { it('is called by default', async () => { const sendRequestMock = jest.fn() - await createPosthogInstance(uuidv7(), { + await createPosthogInstance('testtoken', { loaded: (ph) => { ph._send_request = sendRequestMock }, }) expect(sendRequestMock.mock.calls[0][0]).toMatchObject({ - url: 'http://localhost/decide/?v=3', + url: 'http://localhost/array/testtoken/config', }) }) diff --git a/src/__tests__/remote-config.test.ts b/src/__tests__/remote-config.test.ts index 6fb265281..a2bd592c0 100644 --- a/src/__tests__/remote-config.test.ts +++ b/src/__tests__/remote-config.test.ts @@ -91,7 +91,7 @@ describe('RemoteConfigLoader', () => { it.each([ [true, true], [false, false], - [undefined, true], + [undefined, false], ])('conditionally reloads feature flags - hasFlags: %s, shouldReload: %s', (hasFeatureFlags, shouldReload) => { assignableWindow._POSTHOG_CONFIG = { hasFeatureFlags } as RemoteConfig new RemoteConfigLoader(posthog).load() diff --git a/src/posthog-core.ts b/src/posthog-core.ts index 725a98daf..4345ea711 100644 --- a/src/posthog-core.ts +++ b/src/posthog-core.ts @@ -622,7 +622,6 @@ export class PostHog { } new RemoteConfigLoader(this).load() - this.featureFlags.decide() } _start_queue_if_opted_in(): void { diff --git a/src/posthog-featureflags.ts b/src/posthog-featureflags.ts index e8d2e9c3e..abb76e540 100644 --- a/src/posthog-featureflags.ts +++ b/src/posthog-featureflags.ts @@ -90,31 +90,12 @@ export class PostHogFeatureFlags { private _reloadingDisabled: boolean = false private _additionalReloadRequested: boolean = false private _reloadDebouncer?: any - private _decideCalled: boolean = false private _flagsLoadedFromRemote: boolean = false constructor(private instance: PostHog) { this.featureFlagEventHandlers = [] } - decide(): void { - if (this.instance.config.__preview_remote_config) { - // If remote config is enabled we don't call decide and we mark it as called so that we don't simulate it - this._decideCalled = true - return - } - - // TRICKY: We want to disable flags if we don't have a queued reload, and one of the settings exist for disabling on first load - const disableFlags = - !this._reloadDebouncer && - (this.instance.config.advanced_disable_feature_flags || - this.instance.config.advanced_disable_feature_flags_on_first_load) - - this._callDecideEndpoint({ - disableFlags, - }) - } - get hasLoadedFlags(): boolean { return this._hasLoadedFlags } @@ -248,11 +229,6 @@ export class PostHogFeatureFlags { this._requestInFlight = false - if (!this._decideCalled) { - this._decideCalled = true - this.instance._onRemoteConfig(response.json ?? {}) - } - if (data.disable_flags) { // If flags are disabled then there is no need to call decide again (flags are the only thing that may change) return diff --git a/src/remote-config.ts b/src/remote-config.ts index 11c2a676e..dc609eaeb 100644 --- a/src/remote-config.ts +++ b/src/remote-config.ts @@ -4,7 +4,7 @@ import { RemoteConfig } from './types' import { createLogger } from './utils/logger' import { assignableWindow } from './utils/globals' -const logger = createLogger('[Decide]') +const logger = createLogger('[RemoteConfig]') export class RemoteConfigLoader { constructor(private readonly instance: PostHog) {} @@ -15,7 +15,7 @@ export class RemoteConfigLoader { return cb(assignableWindow._POSTHOG_CONFIG) }) } else { - logger.error('PostHog Extensions not found. Cannot load remote config.') + logger.info('PostHog Extensions not found. Cannot load remote config.') cb() } } @@ -31,14 +31,6 @@ export class RemoteConfigLoader { } load(): 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. - - if (!this.instance.config.__preview_remote_config) { - return - } - // 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) @@ -73,10 +65,10 @@ export class RemoteConfigLoader { logger.error('Failed to fetch remote config from PostHog.') return } + this.instance._onRemoteConfig(config) - // We only need to reload if we haven't already loaded the flags or if the request is in flight - if (config.hasFeatureFlags !== false) { + if (config.hasFeatureFlags && !this.instance.config.advanced_disable_feature_flags_on_first_load) { // 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.ensureFlagsLoaded() diff --git a/src/types.ts b/src/types.ts index 92dd5c7f7..522feed7c 100644 --- a/src/types.ts +++ b/src/types.ts @@ -1,6 +1,7 @@ import { PostHog } from './posthog-core' import type { SegmentAnalytics } from './extensions/segment-integration' import { recordOptions } from './extensions/replay/sessionrecording-utils' +import { SurveyType } from './posthog-surveys-types' export type Property = any export type Properties = Record @@ -544,19 +545,15 @@ export interface RemoteConfig { urlBlocklist?: SessionRecordingUrlTrigger[] eventTriggers?: string[] } - surveys?: boolean - toolbarParams: ToolbarParams - editorParams?: ToolbarParams /** @deprecated, renamed to toolbarParams, still present on older API responses */ - toolbarVersion: 'toolbar' /** @deprecated, moved to toolbarParams */ - isAuthenticated: boolean + surveys?: SurveyType[] siteApps: { id: string; url: string }[] 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) + hasFeatureFlags: boolean // Indicates if the team has any flags enabled (if not we don't need to load them) } -export interface DecideResponse extends RemoteConfig { +export interface DecideResponse { featureFlags: Record featureFlagPayloads: Record errorsWhileComputingFlags: boolean