Skip to content

Commit

Permalink
fix: Load RemoteConfig by default but don't use it (#1607)
Browse files Browse the repository at this point in the history
* Fixes

* Fixes

* Fix cypress tests
  • Loading branch information
benjackwhite authored Dec 17, 2024
1 parent 6f9e089 commit 9b4c24e
Show file tree
Hide file tree
Showing 7 changed files with 52 additions and 31 deletions.
3 changes: 3 additions & 0 deletions cypress/support/e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@ Cypress.on('window:before:load', (win) => {
cy.spy(win.console, 'warn')
cy.spy(win.console, 'log')
cy.spy(win.console, 'debug')

// NOTE: Temporary change whilst testing remote config
;(win as any)._POSTHOG_CONFIG = {}
})

beforeEach(() => {
Expand Down
4 changes: 4 additions & 0 deletions src/__tests__/helpers/posthog-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@

import { PostHog, init_as_module } from '../../posthog-core'
import { PostHogConfig } from '../../types'
import { assignableWindow } from '../../utils/globals'
import { uuidv7 } from '../../uuidv7'

export const createPosthogInstance = async (
Expand All @@ -14,6 +15,9 @@ export const createPosthogInstance = async (
// creates another instance.
const posthog = new PostHog()

// NOTE: Temporary change whilst testing remote config
assignableWindow._POSTHOG_CONFIG = {} as any

// eslint-disable-next-line compat/compat
return await new Promise<PostHog>((resolve) =>
posthog.init(
Expand Down
5 changes: 4 additions & 1 deletion src/__tests__/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,12 +9,15 @@ import { PostHog } from '../posthog-core'
import { defaultPostHog } from './helpers/posthog-instance'

import sinon from 'sinon'
import { window } from '../utils/globals'
import { assignableWindow, window } from '../utils/globals'

describe(`Module-based loader in Node env`, () => {
const posthog = defaultPostHog()

beforeEach(() => {
// NOTE: Temporary change whilst testing remote config
assignableWindow._POSTHOG_CONFIG = {} as any

jest.useFakeTimers()
jest.spyOn(posthog, '_send_request').mockReturnValue()
jest.spyOn(window!.console, 'log').mockImplementation()
Expand Down
4 changes: 4 additions & 0 deletions src/__tests__/posthog-core.identify.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@ import { USER_STATE } from '../constants'
import { uuidv7 } from '../uuidv7'
import { createPosthogInstance, defaultPostHog } from './helpers/posthog-instance'
import { PostHog } from '../posthog-core'
import { assignableWindow } from '../utils/globals'

describe('identify()', () => {
let instance: PostHog
let beforeSendMock: jest.Mock

beforeEach(() => {
beforeSendMock = jest.fn().mockImplementation((e) => e)
// NOTE: Temporary change whilst testing remote config
assignableWindow._POSTHOG_CONFIG = {} as any

const posthog = defaultPostHog().init(
uuidv7(),
{
Expand Down
2 changes: 2 additions & 0 deletions src/__tests__/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,8 @@ describe('posthog core', () => {
}

beforeEach(() => {
// NOTE: Temporary change whilst testing remote config
globals.assignableWindow._POSTHOG_CONFIG = {} as any
jest.useFakeTimers().setSystemTime(baseUTCDateTime)
})

Expand Down
3 changes: 3 additions & 0 deletions src/__tests__/segment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import { beforeEach, describe, expect, it, jest } from '@jest/globals'
import { PostHog } from '../posthog-core'
import { SegmentContext, SegmentPlugin } from '../extensions/segment-integration'
import { USER_STATE } from '../constants'
import { assignableWindow } from '../utils/globals'

describe(`Segment integration`, () => {
let segment: any
Expand All @@ -21,6 +22,8 @@ describe(`Segment integration`, () => {
jest.setTimeout(500)

beforeEach(() => {
assignableWindow._POSTHOG_CONFIG = {} as any

// Create something that looks like the Segment Analytics 2.0 API. We
// could use the actual client, but it's a little more tricky and we'd
// want to mock out the network requests, for which we don't have a good
Expand Down
62 changes: 32 additions & 30 deletions src/remote-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,40 +31,36 @@ 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)
this.onRemoteConfig(assignableWindow._POSTHOG_CONFIG)
return
}

if (this.instance.config.advanced_disable_decide) {
// This setting is essentially saying "dont call external APIs" hence we respect it here
logger.warn('Remote config is disabled. Falling back to local config.')
return
}
try {
// 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
}

// 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)
})
if (this.instance.config.advanced_disable_decide) {
// This setting is essentially saying "dont call external APIs" hence we respect it here
logger.warn('Remote config is disabled. Falling back to local config.')
return
}

this.onRemoteConfig(config)
})
// 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)
})
return
}

this.onRemoteConfig(config)
})
} catch (error) {
logger.error('Error loading remote config', error)
}
}

private onRemoteConfig(config?: RemoteConfig): void {
Expand All @@ -73,6 +69,12 @@ export class RemoteConfigLoader {
logger.error('Failed to fetch remote config from PostHog.')
return
}

if (!this.instance.config.__preview_remote_config) {
logger.info('__preview_remote_config is disabled. Logging config instead', config)
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
Expand Down

0 comments on commit 9b4c24e

Please sign in to comment.