Skip to content

Commit

Permalink
Fixes
Browse files Browse the repository at this point in the history
  • Loading branch information
benjackwhite committed Dec 3, 2024
1 parent 5a345c9 commit c32f364
Show file tree
Hide file tree
Showing 8 changed files with 54 additions and 57 deletions.
4 changes: 3 additions & 1 deletion rollup.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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('.')
Expand Down
8 changes: 4 additions & 4 deletions src/__tests__/decide.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {
Expand Down Expand Up @@ -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', () => {
Expand All @@ -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()
})

Expand All @@ -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()
})
})
Expand Down
8 changes: 4 additions & 4 deletions src/__tests__/personProcessing.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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')

Expand Down Expand Up @@ -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
Expand All @@ -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
Expand All @@ -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,
Expand Down
1 change: 1 addition & 0 deletions src/__tests__/posthog-core.loaded.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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: {} })),
})
Expand Down
22 changes: 11 additions & 11 deletions src/__tests__/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down Expand Up @@ -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 })

Expand All @@ -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/' })

Expand All @@ -336,53 +336,53 @@ 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')
})

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)
})

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/')
})

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/')
})
Expand Down
2 changes: 1 addition & 1 deletion src/decide.ts
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ export class Decide {
return
}

this.instance._afterDecideResponse(response)
this.instance._onRemoteConfig(response)
}

private onRemoteConfig(config?: RemoteConfig): void {
Expand Down
40 changes: 18 additions & 22 deletions src/posthog-core.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import {
CaptureOptions,
CaptureResult,
Compression,
DecideResponse,
EarlyAccessFeatureCallback,
EventName,
IsFeatureEnabledOptions,
Expand Down Expand Up @@ -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
Expand Down
26 changes: 12 additions & 14 deletions src/web-experiments.ts
Original file line number Diff line number Diff line change
@@ -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,
Expand All @@ -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'
Expand All @@ -31,7 +31,6 @@ export const webExperimentUrlValidationMap: Record<

export class WebExperiments {
instance: PostHog
private _featureFlags?: Record<string, string | boolean>
private _flagToExperiments?: Map<string, WebExperiment>

constructor(instance: PostHog) {
Expand Down Expand Up @@ -65,15 +64,18 @@ export class WebExperiments {
})
}

onRemoteConfig(response: DecideResponse) {
onRemoteConfig(config: RemoteConfig) {

Check failure on line 67 in src/web-experiments.ts

View workflow job for this annotation

GitHub Actions / Build and check ES5/ES6 support

'config' is declared but its value is never read.

Check failure on line 67 in src/web-experiments.ts

View workflow job for this annotation

GitHub Actions / Test on Chrome

'config' is declared but its value is never read.

Check failure on line 67 in src/web-experiments.ts

View workflow job for this annotation

GitHub Actions / Test with React

'config' is declared but its value is never read.

Check failure on line 67 in src/web-experiments.ts

View workflow job for this annotation

GitHub Actions / Cypress

'config' is declared but its value is never read.

Check failure on line 67 in src/web-experiments.ts

View workflow job for this annotation

GitHub Actions / Lint

'config' is defined but never used

Check failure on line 67 in src/web-experiments.ts

View workflow job for this annotation

GitHub Actions / Test on Safari

'config' is declared but its value is never read.

Check failure on line 67 in src/web-experiments.ts

View workflow job for this annotation

GitHub Actions / Unit tests

'config' is declared but its value is never read.
// 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() {
Expand Down Expand Up @@ -108,11 +110,7 @@ export class WebExperiments {
this._flagToExperiments = new Map<string, WebExperiment>()

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 `,
Expand All @@ -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,
Expand Down

0 comments on commit c32f364

Please sign in to comment.