Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Use token scoped remote config #1611

Merged
merged 5 commits into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion cypress/support/e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ Cypress.on('window:before:load', (win) => {
cy.spy(win.console, 'debug')

// NOTE: Temporary change whilst testing remote config
;(win as any)._POSTHOG_CONFIG = {}
;(win as any)._POSTHOG_REMOTE_CONFIG = {
test_token: {
config: {},
siteApps: [],
},
}
})

beforeEach(() => {
Expand Down
7 changes: 6 additions & 1 deletion src/__tests__/helpers/posthog-instance.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ export const createPosthogInstance = async (
const posthog = new PostHog()

// NOTE: Temporary change whilst testing remote config
assignableWindow._POSTHOG_CONFIG = {} as any
assignableWindow._POSTHOG_REMOTE_CONFIG = {
[token]: {
config: {},
siteApps: [],
},
} as any

// eslint-disable-next-line compat/compat
return await new Promise<PostHog>((resolve) =>
Expand Down
9 changes: 7 additions & 2 deletions src/__tests__/loader.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,12 @@ describe(`Module-based loader in Node env`, () => {

beforeEach(() => {
// NOTE: Temporary change whilst testing remote config
assignableWindow._POSTHOG_CONFIG = {} as any
assignableWindow._POSTHOG_REMOTE_CONFIG = {
'test-token': {
config: {},
siteApps: [],
},
} as any

jest.useFakeTimers()
jest.spyOn(posthog, '_send_request').mockReturnValue()
Expand Down Expand Up @@ -61,7 +66,7 @@ describe(`Module-based loader in Node env`, () => {
console.warn = jest.fn()
expect(posthog.init(`my-test`, undefined, 'sdk-1')).toBeInstanceOf(PostHog)
expect(posthog.init(``, undefined, 'sdk-2')).toBeInstanceOf(PostHog)
expect(console.error).toHaveBeenCalledTimes(1)
expect(console.error).toHaveBeenCalledTimes(2)
expect(console.error).toHaveBeenCalledWith(
'[PostHog.js]',
'PostHog was initialized without a token. This likely indicates a misconfiguration. Please check the first argument passed to posthog.init()'
Expand Down
12 changes: 9 additions & 3 deletions src/__tests__/posthog-core.identify.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,16 +10,22 @@ describe('identify()', () => {

beforeEach(() => {
beforeSendMock = jest.fn().mockImplementation((e) => e)
const token = uuidv7()
// NOTE: Temporary change whilst testing remote config
assignableWindow._POSTHOG_CONFIG = {} as any
assignableWindow._POSTHOG_REMOTE_CONFIG = {
[token]: {
config: {},
siteApps: [],
},
} as any

const posthog = defaultPostHog().init(
uuidv7(),
token,
{
api_host: 'https://test.com',
before_send: beforeSendMock,
},
uuidv7()
token
)

instance = Object.assign(posthog, {
Expand Down
12 changes: 9 additions & 3 deletions src/__tests__/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,13 +24,19 @@ describe('posthog core', () => {
}

const posthogWith = (config: Partial<PostHogConfig>, overrides?: Partial<PostHog>): PostHog => {
const posthog = defaultPostHog().init('testtoken', config, uuidv7())
// NOTE: Temporary change whilst testing remote config
const token = config.token || 'testtoken'
globals.assignableWindow._POSTHOG_REMOTE_CONFIG = {
[token]: {
config: {},
siteApps: [],
},
} as any
const posthog = defaultPostHog().init(token, config, uuidv7())
return Object.assign(posthog, overrides || {})
}

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

Expand Down
23 changes: 19 additions & 4 deletions src/__tests__/remote-config.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,16 @@ describe('RemoteConfigLoader', () => {

beforeEach(() => {
posthog.config.__preview_remote_config = true
assignableWindow._POSTHOG_CONFIG = undefined
assignableWindow._POSTHOG_REMOTE_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
assignableWindow._POSTHOG_REMOTE_CONFIG = {}
assignableWindow._POSTHOG_REMOTE_CONFIG[_ph.config.token] = {
config,
siteApps: [],
}
cb()
}
)
Expand All @@ -49,7 +53,12 @@ describe('RemoteConfigLoader', () => {
})

it('properly pulls from the window and uses it if set', () => {
assignableWindow._POSTHOG_CONFIG = config as RemoteConfig
assignableWindow._POSTHOG_REMOTE_CONFIG = {
[posthog.config.token]: {
config,
siteApps: [],
},
}
new RemoteConfigLoader(posthog).load()

expect(assignableWindow.__PosthogExtensions__.loadExternalDependency).not.toHaveBeenCalled()
Expand Down Expand Up @@ -93,7 +102,13 @@ describe('RemoteConfigLoader', () => {
[false, false],
[undefined, true],
])('conditionally reloads feature flags - hasFlags: %s, shouldReload: %s', (hasFeatureFlags, shouldReload) => {
assignableWindow._POSTHOG_CONFIG = { hasFeatureFlags } as RemoteConfig
assignableWindow._POSTHOG_REMOTE_CONFIG = {
[posthog.config.token]: {
config: { ...config, hasFeatureFlags },
siteApps: [],
},
}

new RemoteConfigLoader(posthog).load()

if (shouldReload) {
Expand Down
7 changes: 6 additions & 1 deletion src/__tests__/segment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,12 @@ describe(`Segment integration`, () => {
jest.setTimeout(500)

beforeEach(() => {
assignableWindow._POSTHOG_CONFIG = {} as any
assignableWindow._POSTHOG_REMOTE_CONFIG = {
'test-token': {
config: {},
siteApps: [],
},
} 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
Expand Down
72 changes: 49 additions & 23 deletions src/__tests__/site-apps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,8 +15,10 @@ describe('SiteApps', () => {
let emitCaptureEvent: ((eventName: string, eventPayload: CaptureResult) => void) | undefined
let removeCaptureHook = jest.fn()

const token = 'testtoken'

const defaultConfig: Partial<PostHogConfig> = {
token: 'testtoken',
token: token,
api_host: 'https://test.com',
persistence: 'memory',
}
Expand All @@ -40,7 +42,7 @@ describe('SiteApps', () => {
}),
}

delete assignableWindow._POSTHOG_JS_APPS
delete assignableWindow._POSTHOG_REMOTE_CONFIG
delete assignableWindow.POSTHOG_DEBUG

removeCaptureHook = jest.fn()
Expand Down Expand Up @@ -249,7 +251,21 @@ describe('SiteApps', () => {
})

it('does not load site apps if new global loader exists', () => {
assignableWindow._POSTHOG_JS_APPS = []
assignableWindow._POSTHOG_REMOTE_CONFIG = {
[token]: {
config: {},
siteApps: [
{
id: '1',
init: jest.fn(() => {
return {
processEvent: jest.fn(),
}
}),
},
],
},
} as any
siteAppsInstance.onRemoteConfig({
siteApps: [{ id: '1', url: '/site_app/1' }],
} as RemoteConfig)
Expand Down Expand Up @@ -289,26 +305,31 @@ describe('SiteApps', () => {

beforeEach(() => {
appConfigs = []
assignableWindow._POSTHOG_JS_APPS = [
{
id: '1',
init: jest.fn((config) => {
appConfigs.push(config)
return {
processEvent: jest.fn(),
}
}),
assignableWindow._POSTHOG_REMOTE_CONFIG = {
[token]: {
config: {},
siteApps: [
{
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(),
}
}),
},
],
},
{
id: '2',
init: jest.fn((config) => {
appConfigs.push(config)
return {
processEvent: jest.fn(),
}
}),
},
]
} as any

siteAppsInstance.init()
})
Expand All @@ -319,7 +340,12 @@ describe('SiteApps', () => {
})

it('does not sets up the eventCaptured listener if no site apps', () => {
assignableWindow._POSTHOG_JS_APPS = []
assignableWindow._POSTHOG_REMOTE_CONFIG = {
[token]: {
config: {},
siteApps: [],
},
} as any
siteAppsInstance.onRemoteConfig({} as RemoteConfig)
expect(posthog.on).not.toHaveBeenCalled()
})
Expand Down
14 changes: 9 additions & 5 deletions src/remote-config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,15 +4,19 @@ 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) {}

get remoteConfig(): RemoteConfig | undefined {
return assignableWindow._POSTHOG_REMOTE_CONFIG?.[this.instance.config.token]?.config
}

private _loadRemoteConfigJs(cb: (config?: RemoteConfig) => void): void {
if (assignableWindow.__PosthogExtensions__?.loadExternalDependency) {
assignableWindow.__PosthogExtensions__?.loadExternalDependency?.(this.instance, 'remote-config', () => {
return cb(assignableWindow._POSTHOG_CONFIG)
return cb(this.remoteConfig)
})
} else {
logger.error('PostHog Extensions not found. Cannot load remote config.')
Expand All @@ -33,9 +37,9 @@ export class RemoteConfigLoader {
load(): void {
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)
if (this.remoteConfig) {
logger.info('Using preloaded remote config', this.remoteConfig)
this.onRemoteConfig(this.remoteConfig)
return
}

Expand Down
17 changes: 8 additions & 9 deletions src/site-apps.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import { PostHog } from './posthog-core'
import { CaptureResult, Properties, RemoteConfig, SiteApp, SiteAppGlobals, SiteAppLoader } from './types'
import { assignableWindow } from './utils/globals'
import { createLogger } from './utils/logger'
import { isArray } from './utils/type-utils'

const logger = createLogger('[SiteApps]')

Expand Down Expand Up @@ -33,6 +32,10 @@ export class SiteApps {
}
}

get siteAppLoaders(): SiteAppLoader[] | undefined {
return assignableWindow._POSTHOG_REMOTE_CONFIG?.[this.instance.config.token]?.siteApps
}

init() {
if (this.isEnabled) {
const stop = this.instance._addCaptureHook(this.eventCollector.bind(this))
Expand Down Expand Up @@ -142,22 +145,18 @@ export class SiteApps {
}

onRemoteConfig(response: RemoteConfig): void {
if (isArray(assignableWindow._POSTHOG_JS_APPS)) {
if (this.siteAppLoaders?.length) {
if (!this.isEnabled) {
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) {
for (const app of this.siteAppLoaders) {
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))
}
// 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
}
Expand Down
10 changes: 8 additions & 2 deletions src/utils/globals.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,14 @@ export type AssignableWindow = Window &
typeof globalThis &
Record<string, any> & {
__PosthogExtensions__?: PostHogExtensions
_POSTHOG_CONFIG?: RemoteConfig
_POSTHOG_JS_APPS?: SiteAppLoader[]

_POSTHOG_REMOTE_CONFIG?: Record<
string,
{
config: RemoteConfig
siteApps: SiteAppLoader[]
}
>
}

/**
Expand Down
Loading