From d7ebd34a81ca1a379ef35ec75e92c5d57ce7aa12 Mon Sep 17 00:00:00 2001 From: eric Date: Mon, 9 Jan 2023 15:52:22 -0500 Subject: [PATCH 1/8] initial --- posthog-core/src/index.ts | 30 ++++++++++++++++++++++++++++++ posthog-core/src/types.ts | 6 ++++++ 2 files changed, 36 insertions(+) diff --git a/posthog-core/src/index.ts b/posthog-core/src/index.ts index d98041fa..add183ea 100644 --- a/posthog-core/src/index.ts +++ b/posthog-core/src/index.ts @@ -7,6 +7,7 @@ import { PosthogCoreOptions, PostHogEventProperties, PostHogPersistedProperty, + JsonType, } from './types' import { assert, @@ -32,6 +33,7 @@ export abstract class PostHogCore { private captureMode: 'form' | 'json' private sendFeatureFlagEvent: boolean private flagCallReported: { [key: string]: boolean } = {} + private flagPayloadCallReported: { [key: string]: boolean } = {} private removeDebugCallback?: () => void // internal @@ -478,6 +480,34 @@ export abstract class PostHogCore { return response } + getFeatureFlagPayload(key: string): JsonType | undefined { + const payloads = this.getFeatureFlagPayloads() + + if (!payloads) { + return undefined + } + + let response = payloads[key] + if (response === undefined) { + response = null + } + + if (this.sendFeatureFlagEvent && !this.flagPayloadCallReported[key]) { + this.flagPayloadCallReported[key] = true + this.capture('$feature_flag_payload_called', { + $feature_flag: key, + $feature_flag_payload: response, + }) + } + + return response + } + + getFeatureFlagPayloads(): PostHogDecideResponse['feautreFlagPayloads'] | undefined { + let payloads = this.getPersistedProperty(PostHogPersistedProperty.FeatureFlagPayloads) + return payloads + } + getFeatureFlags(): PostHogDecideResponse['featureFlags'] | undefined { let flags = this.getPersistedProperty(PostHogPersistedProperty.FeatureFlags) const overriddenFlags = this.getPersistedProperty( diff --git a/posthog-core/src/types.ts b/posthog-core/src/types.ts index 2e7b18f0..88cb0efe 100644 --- a/posthog-core/src/types.ts +++ b/posthog-core/src/types.ts @@ -34,6 +34,7 @@ export enum PostHogPersistedProperty { DistinctId = 'distinct_id', Props = 'props', FeatureFlags = 'feature_flags', + FeatureFlagPayloads = 'feature_flag_payloads', OverrideFeatureFlags = 'override_feature_flags', Queue = 'queue', OptedOut = 'opted_out', @@ -86,5 +87,10 @@ export type PostHogDecideResponse = { featureFlags: { [key: string]: string | boolean } + feautreFlagPayloads: { + [key: string]: JsonType + } sessionRecording: boolean } + +export type JsonType = string | number | boolean | null | { [key: string]: JsonType } | Array \ No newline at end of file From 7ea7ed7029f129588c150d7fa2abbc90170082dc Mon Sep 17 00:00:00 2001 From: eric Date: Mon, 9 Jan 2023 17:56:54 -0500 Subject: [PATCH 2/8] tests --- posthog-core/src/index.ts | 20 +++++++---- posthog-core/src/types.ts | 2 +- .../test/posthog.featureflags.spec.ts | 35 +++++++++++++++++++ 3 files changed, 50 insertions(+), 7 deletions(-) diff --git a/posthog-core/src/index.ts b/posthog-core/src/index.ts index add183ea..90fa9b2d 100644 --- a/posthog-core/src/index.ts +++ b/posthog-core/src/index.ts @@ -438,6 +438,10 @@ export abstract class PostHogCore { this.setKnownFeatureFlags(res.featureFlags) } + if (res.featureFlagPayloads) { + this.setKnownFeatureFlagPayloads(res.featureFlagPayloads) + } + return res }) .finally(() => { @@ -454,6 +458,13 @@ export abstract class PostHogCore { this._events.emit('featureflags', featureFlags) } + private setKnownFeatureFlagPayloads(featureFlagPayloads: PostHogDecideResponse['featureFlagPayloads']): void { + this.setPersistedProperty( + PostHogPersistedProperty.FeatureFlagPayloads, + featureFlagPayloads + ) + } + getFeatureFlag(key: string): boolean | string | undefined { const featureFlags = this.getFeatureFlags() @@ -488,10 +499,7 @@ export abstract class PostHogCore { } let response = payloads[key] - if (response === undefined) { - response = null - } - + if (this.sendFeatureFlagEvent && !this.flagPayloadCallReported[key]) { this.flagPayloadCallReported[key] = true this.capture('$feature_flag_payload_called', { @@ -503,8 +511,8 @@ export abstract class PostHogCore { return response } - getFeatureFlagPayloads(): PostHogDecideResponse['feautreFlagPayloads'] | undefined { - let payloads = this.getPersistedProperty(PostHogPersistedProperty.FeatureFlagPayloads) + getFeatureFlagPayloads(): PostHogDecideResponse['featureFlagPayloads'] | undefined { + let payloads = this.getPersistedProperty(PostHogPersistedProperty.FeatureFlagPayloads) return payloads } diff --git a/posthog-core/src/types.ts b/posthog-core/src/types.ts index 88cb0efe..83d48a2b 100644 --- a/posthog-core/src/types.ts +++ b/posthog-core/src/types.ts @@ -87,7 +87,7 @@ export type PostHogDecideResponse = { featureFlags: { [key: string]: string | boolean } - feautreFlagPayloads: { + featureFlagPayloads: { [key: string]: JsonType } sessionRecording: boolean diff --git a/posthog-core/test/posthog.featureflags.spec.ts b/posthog-core/test/posthog.featureflags.spec.ts index 0ce5ee67..e2185c44 100644 --- a/posthog-core/test/posthog.featureflags.spec.ts +++ b/posthog-core/test/posthog.featureflags.spec.ts @@ -15,6 +15,13 @@ describe('PostHog Core', () => { 'feature-variant': 'variant', }) + const createMockFeatureFlagPayloads = (): any => ({ + 'feature-1': { + 'color': 'blue' + }, + 'feature-variant': 5 + }) + beforeEach(() => { ;[posthog, mocks] = createTestClient('TEST_API_KEY', { flushAt: 1 }, (_mocks) => { _mocks.fetch.mockImplementation((url) => { @@ -25,6 +32,7 @@ describe('PostHog Core', () => { json: () => Promise.resolve({ featureFlags: createMockFeatureFlags(), + featureFlagPayloads: createMockFeatureFlagPayloads() }), }) } @@ -46,10 +54,18 @@ describe('PostHog Core', () => { expect(posthog.getFeatureFlags()).toEqual(undefined) }) + it('getFeatureFlagPayloads should return undefined if not loaded', () => { + expect(posthog.getFeatureFlagPayloads()).toEqual(undefined) + }) + it('getFeatureFlag should return undefined if not loaded', () => { expect(posthog.getFeatureFlag('my-flag')).toEqual(undefined) expect(posthog.getFeatureFlag('feature-1')).toEqual(undefined) }) + + it('getFeatureFlagPayload should return undefined if not loaded', () => { + expect(posthog.getFeatureFlagPayload("my-flag")).toEqual(undefined) + }) it('isFeatureEnabled should return undefined if not loaded', () => { expect(posthog.isFeatureEnabled('my-flag')).toEqual(undefined) @@ -97,6 +113,13 @@ describe('PostHog Core', () => { 'feature-2': true, 'feature-variant': 'variant', }) + + expect(posthog.getFeatureFlagPayloads()).toEqual({ + 'feature-1': { + 'color': 'blue' + }, + 'feature-variant': 5 + }) }) it('should return the value of a flag', async () => { @@ -104,6 +127,15 @@ describe('PostHog Core', () => { expect(posthog.getFeatureFlag('feature-variant')).toEqual('variant') expect(posthog.getFeatureFlag('feature-missing')).toEqual(false) }) + + it('should return payload of matched flags only', async () => { + expect(posthog.getFeatureFlagPayload('feature-variant')).toEqual(5) + expect(posthog.getFeatureFlagPayload('feature-1')).toEqual({ + 'color': 'blue' + }) + + expect(posthog.getFeatureFlagPayload('feature-2')).toEqual(undefined) + }) describe('when errored out', () => { beforeEach(() => { @@ -140,6 +172,9 @@ describe('PostHog Core', () => { expect(posthog.isFeatureEnabled('feature-1')).toEqual(undefined) expect(posthog.isFeatureEnabled('feature-variant')).toEqual(undefined) expect(posthog.isFeatureEnabled('feature-missing')).toEqual(undefined) + + expect(posthog.getFeatureFlagPayloads()).toEqual(undefined) + expect(posthog.getFeatureFlagPayload('feature-1')).toEqual(undefined) }) }) From 31425839b2d05f7a1e8f92a08e6fa393050f9d64 Mon Sep 17 00:00:00 2001 From: eric Date: Mon, 9 Jan 2023 17:59:42 -0500 Subject: [PATCH 3/8] prettier --- posthog-core/src/index.ts | 6 ++++-- posthog-core/src/types.ts | 2 +- .../test/posthog.featureflags.spec.ts | 20 +++++++++---------- 3 files changed, 15 insertions(+), 13 deletions(-) diff --git a/posthog-core/src/index.ts b/posthog-core/src/index.ts index 90fa9b2d..1890b660 100644 --- a/posthog-core/src/index.ts +++ b/posthog-core/src/index.ts @@ -499,7 +499,7 @@ export abstract class PostHogCore { } let response = payloads[key] - + if (this.sendFeatureFlagEvent && !this.flagPayloadCallReported[key]) { this.flagPayloadCallReported[key] = true this.capture('$feature_flag_payload_called', { @@ -512,7 +512,9 @@ export abstract class PostHogCore { } getFeatureFlagPayloads(): PostHogDecideResponse['featureFlagPayloads'] | undefined { - let payloads = this.getPersistedProperty(PostHogPersistedProperty.FeatureFlagPayloads) + let payloads = this.getPersistedProperty( + PostHogPersistedProperty.FeatureFlagPayloads + ) return payloads } diff --git a/posthog-core/src/types.ts b/posthog-core/src/types.ts index 83d48a2b..282ebd41 100644 --- a/posthog-core/src/types.ts +++ b/posthog-core/src/types.ts @@ -93,4 +93,4 @@ export type PostHogDecideResponse = { sessionRecording: boolean } -export type JsonType = string | number | boolean | null | { [key: string]: JsonType } | Array \ No newline at end of file +export type JsonType = string | number | boolean | null | { [key: string]: JsonType } | Array diff --git a/posthog-core/test/posthog.featureflags.spec.ts b/posthog-core/test/posthog.featureflags.spec.ts index e2185c44..60c17f8f 100644 --- a/posthog-core/test/posthog.featureflags.spec.ts +++ b/posthog-core/test/posthog.featureflags.spec.ts @@ -17,9 +17,9 @@ describe('PostHog Core', () => { const createMockFeatureFlagPayloads = (): any => ({ 'feature-1': { - 'color': 'blue' + color: 'blue', }, - 'feature-variant': 5 + 'feature-variant': 5, }) beforeEach(() => { @@ -32,7 +32,7 @@ describe('PostHog Core', () => { json: () => Promise.resolve({ featureFlags: createMockFeatureFlags(), - featureFlagPayloads: createMockFeatureFlagPayloads() + featureFlagPayloads: createMockFeatureFlagPayloads(), }), }) } @@ -62,9 +62,9 @@ describe('PostHog Core', () => { expect(posthog.getFeatureFlag('my-flag')).toEqual(undefined) expect(posthog.getFeatureFlag('feature-1')).toEqual(undefined) }) - + it('getFeatureFlagPayload should return undefined if not loaded', () => { - expect(posthog.getFeatureFlagPayload("my-flag")).toEqual(undefined) + expect(posthog.getFeatureFlagPayload('my-flag')).toEqual(undefined) }) it('isFeatureEnabled should return undefined if not loaded', () => { @@ -116,9 +116,9 @@ describe('PostHog Core', () => { expect(posthog.getFeatureFlagPayloads()).toEqual({ 'feature-1': { - 'color': 'blue' + color: 'blue', }, - 'feature-variant': 5 + 'feature-variant': 5, }) }) @@ -127,11 +127,11 @@ describe('PostHog Core', () => { expect(posthog.getFeatureFlag('feature-variant')).toEqual('variant') expect(posthog.getFeatureFlag('feature-missing')).toEqual(false) }) - + it('should return payload of matched flags only', async () => { expect(posthog.getFeatureFlagPayload('feature-variant')).toEqual(5) expect(posthog.getFeatureFlagPayload('feature-1')).toEqual({ - 'color': 'blue' + color: 'blue', }) expect(posthog.getFeatureFlagPayload('feature-2')).toEqual(undefined) @@ -172,7 +172,7 @@ describe('PostHog Core', () => { expect(posthog.isFeatureEnabled('feature-1')).toEqual(undefined) expect(posthog.isFeatureEnabled('feature-variant')).toEqual(undefined) expect(posthog.isFeatureEnabled('feature-missing')).toEqual(undefined) - + expect(posthog.getFeatureFlagPayloads()).toEqual(undefined) expect(posthog.getFeatureFlagPayload('feature-1')).toEqual(undefined) }) From 136cc4a5f6cd6deadd6a2966c8273d1c4f97cee2 Mon Sep 17 00:00:00 2001 From: eric Date: Wed, 11 Jan 2023 15:19:46 -0500 Subject: [PATCH 4/8] remove event call --- posthog-core/src/index.ts | 9 +++------ 1 file changed, 3 insertions(+), 6 deletions(-) diff --git a/posthog-core/src/index.ts b/posthog-core/src/index.ts index 1890b660..d1c7b8d6 100644 --- a/posthog-core/src/index.ts +++ b/posthog-core/src/index.ts @@ -500,12 +500,9 @@ export abstract class PostHogCore { let response = payloads[key] - if (this.sendFeatureFlagEvent && !this.flagPayloadCallReported[key]) { - this.flagPayloadCallReported[key] = true - this.capture('$feature_flag_payload_called', { - $feature_flag: key, - $feature_flag_payload: response, - }) + // Undefined means a loading or missing data issue. Null means evaluation happened and there was no match + if (response === undefined) { + return null } return response From d0d075420409f6dcbf6c53931707c9c98acaf98f Mon Sep 17 00:00:00 2001 From: eric Date: Wed, 11 Jan 2023 16:05:43 -0500 Subject: [PATCH 5/8] update test --- posthog-core/test/posthog.featureflags.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog-core/test/posthog.featureflags.spec.ts b/posthog-core/test/posthog.featureflags.spec.ts index 60c17f8f..45697f37 100644 --- a/posthog-core/test/posthog.featureflags.spec.ts +++ b/posthog-core/test/posthog.featureflags.spec.ts @@ -134,7 +134,7 @@ describe('PostHog Core', () => { color: 'blue', }) - expect(posthog.getFeatureFlagPayload('feature-2')).toEqual(undefined) + expect(posthog.getFeatureFlagPayload('feature-2')).toEqual(null) }) describe('when errored out', () => { From d3ea98bf6dbbba834d33e93becd817f5ebc713b2 Mon Sep 17 00:00:00 2001 From: eric Date: Tue, 17 Jan 2023 09:26:20 -0500 Subject: [PATCH 6/8] remove unused signature --- posthog-core/src/index.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/posthog-core/src/index.ts b/posthog-core/src/index.ts index 0d924f14..4a9a65ca 100644 --- a/posthog-core/src/index.ts +++ b/posthog-core/src/index.ts @@ -33,7 +33,6 @@ export abstract class PostHogCore { private captureMode: 'form' | 'json' private sendFeatureFlagEvent: boolean private flagCallReported: { [key: string]: boolean } = {} - private flagPayloadCallReported: { [key: string]: boolean } = {} private removeDebugCallback?: () => void // internal From af96642939d58d58307b1f9c81339fe72180480f Mon Sep 17 00:00:00 2001 From: eric Date: Tue, 17 Jan 2023 09:28:18 -0500 Subject: [PATCH 7/8] run prettier --- posthog-core/test/posthog.featureflags.spec.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/posthog-core/test/posthog.featureflags.spec.ts b/posthog-core/test/posthog.featureflags.spec.ts index e894e12e..b4b83fb6 100644 --- a/posthog-core/test/posthog.featureflags.spec.ts +++ b/posthog-core/test/posthog.featureflags.spec.ts @@ -21,7 +21,7 @@ describe('PostHog Core', () => { }, 'feature-variant': 5, }) - + const errorAPIResponse = Promise.resolve({ status: 400, text: () => Promise.resolve('error'), From 44da55efc653735b120588d0d00bb843aad693ac Mon Sep 17 00:00:00 2001 From: eric Date: Tue, 17 Jan 2023 09:58:19 -0500 Subject: [PATCH 8/8] merge logic --- posthog-core/src/index.ts | 12 +++--------- 1 file changed, 3 insertions(+), 9 deletions(-) diff --git a/posthog-core/src/index.ts b/posthog-core/src/index.ts index 4a9a65ca..ff470a72 100644 --- a/posthog-core/src/index.ts +++ b/posthog-core/src/index.ts @@ -435,25 +435,19 @@ export abstract class PostHogCore { .then((res) => { if (res.featureFlags) { let newFeatureFlags = res.featureFlags + let newFeatureFlagPayloads = res.featureFlagPayloads if (res.errorsWhileComputingFlags) { // if not all flags were computed, we upsert flags instead of replacing them const currentFlags = this.getPersistedProperty( PostHogPersistedProperty.FeatureFlags ) - newFeatureFlags = { ...currentFlags, ...res.featureFlags } - } - this.setKnownFeatureFlags(newFeatureFlags) - } - - if (res.featureFlagPayloads) { - let newFeatureFlagPayloads = res.featureFlagPayloads - if (res.errorsWhileComputingFlags) { - // if not all flags were computed, we upsert flags instead of replacing them const currentFlagPayloads = this.getPersistedProperty( PostHogPersistedProperty.FeatureFlagPayloads ) + newFeatureFlags = { ...currentFlags, ...res.featureFlags } newFeatureFlagPayloads = { ...currentFlagPayloads, ...res.featureFlagPayloads } } + this.setKnownFeatureFlags(newFeatureFlags) this.setKnownFeatureFlagPayloads(newFeatureFlagPayloads) }