From 34c2cb6a663af546365eef8ec01b7a8cb4c3d32d Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Tue, 5 Nov 2024 11:51:04 -0800 Subject: [PATCH 01/22] update doFlags to take user object, update tag script to fetch remote flags --- .../src/experimentClient.ts | 19 ++++++--- packages/experiment-core/src/api/flag-api.ts | 12 +++++- packages/experiment-tag/src/experiment.ts | 39 ++++++++++++++++++- packages/experiment-tag/src/script.ts | 7 ++-- 4 files changed, 66 insertions(+), 11 deletions(-) diff --git a/packages/experiment-browser/src/experimentClient.ts b/packages/experiment-browser/src/experimentClient.ts index 0aabab8f..7d5aef1d 100644 --- a/packages/experiment-browser/src/experimentClient.ts +++ b/packages/experiment-browser/src/experimentClient.ts @@ -701,13 +701,20 @@ export class ExperimentClient implements Client { return variants; } - private async doFlags(): Promise { + public async doFlags(isWebExperiment?: boolean): Promise { try { - const flags = await this.flagApi.getFlags({ - libraryName: 'experiment-js-client', - libraryVersion: PACKAGE_VERSION, - timeoutMillis: this.config.fetchTimeoutMillis, - }); + const user: ExperimentUser = { + user_id: this.getUser().user_id, + device_id: this.getUser().device_id, + }; + const flags = await this.flagApi.getFlags( + { + libraryName: 'experiment-js-client', + libraryVersion: PACKAGE_VERSION, + timeoutMillis: this.config.fetchTimeoutMillis, + }, + isWebExperiment ? user : undefined, + ); this.flags.clear(); this.flags.putAll(flags); } catch (e) { diff --git a/packages/experiment-core/src/api/flag-api.ts b/packages/experiment-core/src/api/flag-api.ts index 23149fa6..09d95b66 100644 --- a/packages/experiment-core/src/api/flag-api.ts +++ b/packages/experiment-core/src/api/flag-api.ts @@ -1,5 +1,6 @@ import { EvaluationFlag } from '../evaluation/flag'; import { HttpClient } from '../transport/http'; +import { Base64 } from 'js-base64'; export type GetFlagsOptions = { libraryName: string; @@ -7,8 +8,12 @@ export type GetFlagsOptions = { evaluationMode?: string; timeoutMillis?: number; }; + export interface FlagApi { - getFlags(options?: GetFlagsOptions): Promise>; + getFlags( + options?: GetFlagsOptions, + user?: Record, + ): Promise>; } export class SdkFlagApi implements FlagApi { @@ -25,8 +30,10 @@ export class SdkFlagApi implements FlagApi { this.serverUrl = serverUrl; this.httpClient = httpClient; } + public async getFlags( options?: GetFlagsOptions, + user?: Record, ): Promise> { const headers: Record = { Authorization: `Api-Key ${this.deploymentKey}`, @@ -36,6 +43,9 @@ export class SdkFlagApi implements FlagApi { 'X-Amp-Exp-Library' ] = `${options.libraryName}/${options.libraryVersion}`; } + if (user) { + headers['X-Amp-Exp-User'] = Base64.encodeURL(JSON.stringify(user)); + } const response = await this.httpClient.request({ requestUrl: `${this.serverUrl}/sdk/v2/flags`, method: 'GET', diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index d81bab0e..9631a7e7 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -29,8 +29,12 @@ const appliedMutations: MutationController[] = []; let previousUrl: string | undefined; // Cache to track exposure for the current URL, should be cleared on URL change let urlExposureCache: { [url: string]: { [key: string]: string | undefined } }; +const localFlagKeys: Set = new Set(); -export const initializeExperiment = (apiKey: string, initialFlags: string) => { +export const initializeExperiment = async ( + apiKey: string, + initialFlags: string, +) => { const globalScope = getGlobalScope(); if (globalScope?.webExperiment) { return; @@ -75,6 +79,7 @@ export const initializeExperiment = (apiKey: string, initialFlags: string) => { if (urlParams['PREVIEW']) { const parsedFlags = JSON.parse(initialFlags); parsedFlags.forEach((flag: EvaluationFlag) => { + localFlagKeys.add(flag.key); if (flag.key in urlParams && urlParams[flag.key] in flag.variants) { // Strip the preview query param globalScope.history.replaceState( @@ -100,11 +105,14 @@ export const initializeExperiment = (apiKey: string, initialFlags: string) => { initialFlags = JSON.stringify(parsedFlags); } + // initialize the experiment with local flags only globalScope.webExperiment = Experiment.initialize(apiKey, { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore internalInstanceNameSuffix: 'web', fetchOnStart: false, + flagsServerUrl: 'https://flag.lab.amplitude.com?delivery_method=web', + pollOnStart: false, initialFlags: initialFlags, }); @@ -121,10 +129,24 @@ export const initializeExperiment = (apiKey: string, initialFlags: string) => { globalScope.webExperiment.addPlugin(globalScope.experimentIntegration); globalScope.webExperiment.setUser(user); + // evaluate based on local flags + apply variants const variants = globalScope.webExperiment.all(); setUrlChangeListener(); applyVariants(variants); + + // TODO for remote+local: have to differentiate between amp device_id and web_experiment_id + // TODO should have/check "isBlocking" metadata? + + // fetch remote flags - making doFlags() public? + // need to pass in user to doFlags + await globalScope.webExperiment.doFlags(); + + // apply remote variants + applyRemoteVariants(); + + // set up listener for remote flag fetch + re-eval + // how to listen to user identity change? should just add in into integration plugin? }; const applyVariants = (variants: Variants | undefined) => { @@ -171,6 +193,21 @@ const applyVariants = (variants: Variants | undefined) => { } }; +const applyRemoteVariants = () => { + const globalScope = getGlobalScope(); + if (!globalScope) { + return; + } + // apply variants of remote flags ONLY (all() and filter out by remote flag keys) + const remoteVariants = globalScope.webExperiment + .all() + .filter((flag: EvaluationFlag) => { + return !localFlagKeys.has(flag.key); + }); + + applyVariants(remoteVariants); +}; + const handleRedirect = (action, key: string, variant: Variant) => { const globalScope = getGlobalScope(); if (!globalScope) { diff --git a/packages/experiment-tag/src/script.ts b/packages/experiment-tag/src/script.ts index d83d8453..68adbad7 100644 --- a/packages/experiment-tag/src/script.ts +++ b/packages/experiment-tag/src/script.ts @@ -2,6 +2,7 @@ import { initializeExperiment } from './experiment'; const API_KEY = '{{DEPLOYMENT_KEY}}'; const initialFlags = '{{INITIAL_FLAGS}}'; -initializeExperiment(API_KEY, initialFlags); -// Remove anti-flicker css if it exists -document.getElementById('amp-exp-css')?.remove(); +initializeExperiment(API_KEY, initialFlags).then(() => { + // Remove anti-flicker css if it exists + document.getElementById('amp-exp-css')?.remove(); +}); From a463740a052e2d267c0e5a3614628a4d21230fe7 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Wed, 6 Nov 2024 15:18:35 -0800 Subject: [PATCH 02/22] fix applyVariants logic --- .../src/experimentClient.ts | 6 +- packages/experiment-tag/src/experiment.ts | 69 ++++++++++--------- .../experiment-tag/test/experiment.test.ts | 2 +- 3 files changed, 43 insertions(+), 34 deletions(-) diff --git a/packages/experiment-browser/src/experimentClient.ts b/packages/experiment-browser/src/experimentClient.ts index 7d5aef1d..2591ce77 100644 --- a/packages/experiment-browser/src/experimentClient.ts +++ b/packages/experiment-browser/src/experimentClient.ts @@ -701,7 +701,7 @@ export class ExperimentClient implements Client { return variants; } - public async doFlags(isWebExperiment?: boolean): Promise { + public async doFlags(): Promise { try { const user: ExperimentUser = { user_id: this.getUser().user_id, @@ -713,7 +713,9 @@ export class ExperimentClient implements Client { libraryVersion: PACKAGE_VERSION, timeoutMillis: this.config.fetchTimeoutMillis, }, - isWebExperiment ? user : undefined, + this.config?.['internalInstanceNameSuffix'] === 'web' + ? user + : undefined, ); this.flags.clear(); this.flags.putAll(flags); diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 9631a7e7..85f59429 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -29,7 +29,8 @@ const appliedMutations: MutationController[] = []; let previousUrl: string | undefined; // Cache to track exposure for the current URL, should be cleared on URL change let urlExposureCache: { [url: string]: { [key: string]: string | undefined } }; -const localFlagKeys: Set = new Set(); +const remoteFlagKeys: Set = new Set(); +const locaFlagKeys: Set = new Set(); export const initializeExperiment = async ( apiKey: string, @@ -75,11 +76,11 @@ export const initializeExperiment = async ( ); return; } + + const parsedFlags = JSON.parse(initialFlags); // force variant if in preview mode if (urlParams['PREVIEW']) { - const parsedFlags = JSON.parse(initialFlags); parsedFlags.forEach((flag: EvaluationFlag) => { - localFlagKeys.add(flag.key); if (flag.key in urlParams && urlParams[flag.key] in flag.variants) { // Strip the preview query param globalScope.history.replaceState( @@ -105,7 +106,24 @@ export const initializeExperiment = async ( initialFlags = JSON.stringify(parsedFlags); } - // initialize the experiment with local flags only + let remoteBlocking = false; + // get set of remote flag keys + parsedFlags.forEach((flag: EvaluationFlag) => { + if (flag?.metadata?.evaluationMode !== 'local') { + remoteFlagKeys.add(flag.key); + // make the remote flag locally evaluable + flag.metadata = flag.metadata || {}; + flag.metadata.evaluationMode = 'local'; + // check whether any remote flags are blocking + if (flag.metadata?.isBlocking) { + remoteBlocking = true; + } + } else { + locaFlagKeys.add(flag.key); + } + }); + + // initialize the experiment globalScope.webExperiment = Experiment.initialize(apiKey, { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore @@ -129,27 +147,28 @@ export const initializeExperiment = async ( globalScope.webExperiment.addPlugin(globalScope.experimentIntegration); globalScope.webExperiment.setUser(user); - // evaluate based on local flags + apply variants - const variants = globalScope.webExperiment.all(); - setUrlChangeListener(); - applyVariants(variants); - // TODO for remote+local: have to differentiate between amp device_id and web_experiment_id + // apply local variants + applyVariants(globalScope.webExperiment.all(), locaFlagKeys); + // TODO should have/check "isBlocking" metadata? + if (!remoteBlocking) { + // Remove anti-flicker css if remote flags are not blocking + globalScope.document.getElementById?.('amp-exp-css')?.remove(); + } - // fetch remote flags - making doFlags() public? - // need to pass in user to doFlags + // fetch remote flags await globalScope.webExperiment.doFlags(); // apply remote variants - applyRemoteVariants(); - - // set up listener for remote flag fetch + re-eval - // how to listen to user identity change? should just add in into integration plugin? + applyVariants(globalScope.webExperiment.all(), remoteFlagKeys); }; -const applyVariants = (variants: Variants | undefined) => { +const applyVariants = ( + variants: Variants | undefined, + flagKeys: Set | undefined = undefined +) => { if (!variants) { return; } @@ -164,6 +183,9 @@ const applyVariants = (variants: Variants | undefined) => { urlExposureCache[currentUrl] = {}; } for (const key in variants) { + if (flagKeys && !flagKeys.has(key)) { + continue; + } const variant = variants[key]; const isWebExperimentation = variant.metadata?.deliveryMethod === 'web'; if (isWebExperimentation) { @@ -193,21 +215,6 @@ const applyVariants = (variants: Variants | undefined) => { } }; -const applyRemoteVariants = () => { - const globalScope = getGlobalScope(); - if (!globalScope) { - return; - } - // apply variants of remote flags ONLY (all() and filter out by remote flag keys) - const remoteVariants = globalScope.webExperiment - .all() - .filter((flag: EvaluationFlag) => { - return !localFlagKeys.has(flag.key); - }); - - applyVariants(remoteVariants); -}; - const handleRedirect = (action, key: string, variant: Variant) => { const globalScope = getGlobalScope(); if (!globalScope) { diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index d9d5c1f5..cd7c31a9 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -35,7 +35,7 @@ describe('initializeExperiment', () => { replace: jest.fn(), search: '', }, - document: { referrer: '' }, + document: { referrer: ''}, history: { replaceState: jest.fn() }, }; // eslint-disable-next-line @typescript-eslint/ban-ts-comment From 348d85737ffae15a9b0a5fc580ac141a20058678 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Wed, 13 Nov 2024 09:59:31 -0800 Subject: [PATCH 03/22] create remote flag fetch test and clean up exisiting tests --- packages/experiment-tag/src/experiment.ts | 6 +- .../experiment-tag/test/experiment.test.ts | 153 +++++++++++++++--- .../test/util/mock-http-client.ts | 40 +++++ 3 files changed, 171 insertions(+), 28 deletions(-) create mode 100644 packages/experiment-tag/test/util/mock-http-client.ts diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 85f59429..13cf9394 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -11,6 +11,7 @@ import { Variant, Variants, AmplitudeIntegrationPlugin, + ExperimentConfig, } from '@amplitude/experiment-js-client'; import mutate, { MutationController } from 'dom-mutator'; @@ -35,6 +36,7 @@ const locaFlagKeys: Set = new Set(); export const initializeExperiment = async ( apiKey: string, initialFlags: string, + config: ExperimentConfig = {}, ) => { const globalScope = getGlobalScope(); if (globalScope?.webExperiment) { @@ -132,6 +134,7 @@ export const initializeExperiment = async ( flagsServerUrl: 'https://flag.lab.amplitude.com?delivery_method=web', pollOnStart: false, initialFlags: initialFlags, + ...config, }); // If no integration has been set, use an amplitude integration. @@ -152,7 +155,6 @@ export const initializeExperiment = async ( // apply local variants applyVariants(globalScope.webExperiment.all(), locaFlagKeys); - // TODO should have/check "isBlocking" metadata? if (!remoteBlocking) { // Remove anti-flicker css if remote flags are not blocking globalScope.document.getElementById?.('amp-exp-css')?.remove(); @@ -167,7 +169,7 @@ export const initializeExperiment = async ( const applyVariants = ( variants: Variants | undefined, - flagKeys: Set | undefined = undefined + flagKeys: Set | undefined = undefined, ) => { if (!variants) { return; diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index cd7c31a9..ba8c2670 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -1,8 +1,9 @@ -import * as coreUtil from '@amplitude/experiment-core'; +import * as experimentCore from '@amplitude/experiment-core'; import { ExperimentClient } from '@amplitude/experiment-js-client'; import { initializeExperiment } from 'src/experiment'; import * as experiment from 'src/experiment'; import * as util from 'src/util'; +import { MockHttpClient } from './util/mock-http-client'; jest.mock('src/messenger', () => { return { @@ -15,7 +16,7 @@ jest.mock('src/messenger', () => { jest.spyOn(experiment, 'setUrlChangeListener').mockReturnValue(undefined); describe('initializeExperiment', () => { - const mockGetGlobalScope = jest.spyOn(coreUtil, 'getGlobalScope'); + const mockGetGlobalScope = jest.spyOn(experimentCore, 'getGlobalScope'); jest.spyOn(ExperimentClient.prototype, 'setUser'); jest.spyOn(ExperimentClient.prototype, 'all'); const mockExposure = jest.spyOn(ExperimentClient.prototype, 'exposure'); @@ -23,7 +24,7 @@ describe('initializeExperiment', () => { let mockGlobal; beforeEach(() => { - jest.spyOn(coreUtil, 'isLocalStorageAvailable').mockReturnValue(true); + jest.spyOn(experimentCore, 'isLocalStorageAvailable').mockReturnValue(true); jest.clearAllMocks(); mockGlobal = { localStorage: { @@ -35,7 +36,7 @@ describe('initializeExperiment', () => { replace: jest.fn(), search: '', }, - document: { referrer: ''}, + document: { referrer: '' }, history: { replaceState: jest.fn() }, }; // eslint-disable-next-line @typescript-eslint/ban-ts-comment @@ -52,10 +53,7 @@ describe('initializeExperiment', () => { metadata: { deployed: true, evaluationMode: 'local', - experimentKey: 'exp-1', flagType: 'experiment', - flagVersion: 20, - urlMatch: ['http://test.com'], deliveryMethod: 'web', }, segments: [ @@ -111,7 +109,9 @@ describe('initializeExperiment', () => { }); test('experiment should not run without localStorage', () => { - jest.spyOn(coreUtil, 'isLocalStorageAvailable').mockReturnValue(false); + jest + .spyOn(experimentCore, 'isLocalStorageAvailable') + .mockReturnValue(false); initializeExperiment('2', ''); expect(mockGlobal.localStorage.getItem).toHaveBeenCalledTimes(0); }); @@ -125,10 +125,7 @@ describe('initializeExperiment', () => { metadata: { deployed: true, evaluationMode: 'local', - experimentKey: 'exp-1', flagType: 'experiment', - flagVersion: 20, - urlMatch: ['http://test.com'], deliveryMethod: 'web', }, segments: [ @@ -190,10 +187,7 @@ describe('initializeExperiment', () => { metadata: { deployed: true, evaluationMode: 'local', - experimentKey: 'exp-1', flagType: 'experiment', - flagVersion: 20, - urlMatch: ['http://test.com'], deliveryMethod: 'web', }, segments: [ @@ -261,7 +255,6 @@ describe('initializeExperiment', () => { evaluationMode: 'local', experimentKey: 'exp-1', flagType: 'experiment', - flagVersion: 20, deliveryMethod: 'web', }, segments: [ @@ -336,10 +329,7 @@ describe('initializeExperiment', () => { metadata: { deployed: true, evaluationMode: 'local', - experimentKey: 'exp-1', flagType: 'experiment', - flagVersion: 20, - urlMatch: ['http://test.com'], deliveryMethod: 'web', }, segments: [ @@ -418,9 +408,7 @@ describe('initializeExperiment', () => { metadata: { deployed: true, evaluationMode: 'local', - experimentKey: 'exp-1', flagType: 'experiment', - flagVersion: 1, deliveryMethod: 'web', }, segments: [ @@ -511,10 +499,7 @@ describe('initializeExperiment', () => { metadata: { deployed: true, evaluationMode: 'local', - experimentKey: 'exp-1', flagType: 'experiment', - flagVersion: 20, - urlMatch: ['http://test.com'], deliveryMethod: 'web', }, segments: [ @@ -576,9 +561,7 @@ describe('initializeExperiment', () => { metadata: { deployed: true, evaluationMode: 'local', - experimentKey: 'exp-1', flagType: 'experiment', - flagVersion: 20, deliveryMethod: 'web', }, segments: [ @@ -623,7 +606,7 @@ describe('initializeExperiment', () => { }, writable: true, }); - jest.spyOn(coreUtil, 'getGlobalScope'); + jest.spyOn(experimentCore, 'getGlobalScope'); initializeExperiment( '10', JSON.stringify([ @@ -735,4 +718,122 @@ describe('initializeExperiment', () => { ); expect(mockExposure).not.toHaveBeenCalled(); }); + + test('test remote evaluation successful', async () => { + const remoteFlags = [ + { + key: 'test', + metadata: { + deployed: true, + evaluationMode: 'local', + experimentKey: 'exp-1', + flagType: 'experiment', + flagVersion: 1, + deliveryMethod: 'web', + }, + segments: [ + { + metadata: { + segmentName: 'All Other Users', + }, + variant: 'treatment', + }, + ], + variants: { + control: { + key: 'control', + payload: [ + { + action: 'redirect', + data: { + url: 'http://test.com', + }, + }, + ], + value: 'control', + }, + off: { + key: 'off', + metadata: { + default: true, + }, + }, + treatment: { + key: 'treatment', + payload: [ + { + action: 'redirect', + data: { + url: 'http://test.com/2', + }, + }, + ], + value: 'treatment', + }, + }, + }, + ]; + + const initialRemoteFlags = [ + { + key: 'test', + metadata: { + deployed: true, + evaluationMode: 'remote', + flagType: 'experiment', + deliveryMethod: 'web', + }, + segments: [ + { + metadata: { + segmentName: 'All Other Users', + }, + variant: 'off', + }, + ], + variants: { + control: { + key: 'control', + payload: [ + { + action: 'redirect', + data: { + url: 'http://test.com', + }, + }, + ], + value: 'control', + }, + off: { + key: 'off', + metadata: { + default: true, + }, + }, + treatment: { + key: 'treatment', + payload: [ + { + action: 'redirect', + data: { + url: 'http://test.com/2', + }, + }, + ], + value: 'treatment', + }, + }, + }, + ]; + + const mockHttpClient = new MockHttpClient(JSON.stringify(remoteFlags)); + + await initializeExperiment('12', JSON.stringify(initialRemoteFlags), { + httpClient: mockHttpClient, + }); + expect(mockGlobal.location.replace).toHaveBeenCalledWith( + 'http://test.com/2', + ); + expect(mockExposure).toHaveBeenCalledWith('test'); + }); }); diff --git a/packages/experiment-tag/test/util/mock-http-client.ts b/packages/experiment-tag/test/util/mock-http-client.ts new file mode 100644 index 00000000..8cfaa3f9 --- /dev/null +++ b/packages/experiment-tag/test/util/mock-http-client.ts @@ -0,0 +1,40 @@ +export interface SimpleResponse { + status: number; + body: string; +} + +export interface HttpClient { + request( + requestUrl: string, + method: string, + headers: Record, + data: string, + timeoutMillis?: number, + ): Promise; +} + +export interface SimpleResponse { + status: number; + body: string; +} + +export class MockHttpClient implements HttpClient { + private response: SimpleResponse; + + constructor(responseBody: string, status = 200) { + this.response = { + status, + body: responseBody, + }; + } + + request( + requestUrl: string, + method: string, + headers: Record, + data: string, + timeoutMillis?: number, + ): Promise { + return Promise.resolve(this.response); + } +} From 936f27ad08672e27e9d85dd5c36a6e035711d7e6 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Wed, 13 Nov 2024 13:26:40 -0800 Subject: [PATCH 04/22] refactor unit tests code --- packages/experiment-tag/src/experiment.ts | 36 +- packages/experiment-tag/src/util.ts | 5 + .../experiment-tag/test/experiment.test.ts | 677 +++--------------- .../experiment-tag/test/util/create-flag.ts | 62 ++ .../test/util/mock-http-client.ts | 11 +- 5 files changed, 206 insertions(+), 585 deletions(-) create mode 100644 packages/experiment-tag/test/util/create-flag.ts diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 13cf9394..985fe2f3 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -58,14 +58,31 @@ export const initializeExperiment = async ( user = {}; } - // create new user if it does not exist, or it does not have device_id - if (Object.keys(user).length === 0 || !user.device_id) { - user = {}; - user.device_id = UUID(); - globalScope.localStorage.setItem( - experimentStorageName, - JSON.stringify(user), - ); + // create new user if it does not exist, or it does not have web_exp_id + if (Object.keys(user).length === 0) { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + if (!user.web_exp_id) { + if (user.device_id) { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + user.web_exp_id = user.device_id; + user.device_id = undefined; + globalScope.localStorage.setItem( + experimentStorageName, + JSON.stringify(user), + ); + } else { + user = {}; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + user.web_exp_id = UUID(); + } + globalScope.localStorage.setItem( + experimentStorageName, + JSON.stringify(user), + ); + } } const urlParams = getUrlParams(); @@ -160,6 +177,9 @@ export const initializeExperiment = async ( globalScope.document.getElementById?.('amp-exp-css')?.remove(); } + if (remoteFlagKeys.size === 0) { + return; + } // fetch remote flags await globalScope.webExperiment.doFlags(); diff --git a/packages/experiment-tag/src/util.ts b/packages/experiment-tag/src/util.ts index ec7ac8e4..37f95c1f 100644 --- a/packages/experiment-tag/src/util.ts +++ b/packages/experiment-tag/src/util.ts @@ -1,4 +1,5 @@ import { getGlobalScope } from '@amplitude/experiment-core'; +import { ExperimentUser } from '@amplitude/experiment-js-client'; export const getUrlParams = (): Record => { const globalScope = getGlobalScope(); @@ -87,3 +88,7 @@ export const concatenateQueryParamsOf = ( return resultUrlObj.toString(); }; + +export const hasUserOrDeviceId = (user: ExperimentUser): boolean => { + return !!(user.user_id || user.device_id); +}; diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index ba8c2670..5808813f 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -4,6 +4,7 @@ import { initializeExperiment } from 'src/experiment'; import * as experiment from 'src/experiment'; import * as util from 'src/util'; import { MockHttpClient } from './util/mock-http-client'; +import { createRedirectFlag } from './util/create-flag'; jest.mock('src/messenger', () => { return { @@ -45,66 +46,13 @@ describe('initializeExperiment', () => { }); test('should initialize experiment with empty user', () => { - initializeExperiment( - '1', - JSON.stringify([ - { - key: 'test', - metadata: { - deployed: true, - evaluationMode: 'local', - flagType: 'experiment', - deliveryMethod: 'web', - }, - segments: [ - { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'treatment', - }, - ], - variants: { - control: { - key: 'control', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com', - }, - }, - ], - value: 'control', - }, - off: { - key: 'off', - metadata: { - default: true, - }, - }, - treatment: { - key: 'treatment', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com/2', - }, - }, - ], - value: 'treatment', - }, - }, - }, - ]), - ); + initializeExperiment('1', JSON.stringify([])); expect(ExperimentClient.prototype.setUser).toHaveBeenCalledWith({ - device_id: 'mock', + web_exp_id: 'mock', }); expect(mockGlobal.localStorage.setItem).toHaveBeenCalledWith( 'EXP_1', - JSON.stringify({ device_id: 'mock' }), + JSON.stringify({ web_exp_id: 'mock' }), ); }); @@ -120,55 +68,7 @@ describe('initializeExperiment', () => { initializeExperiment( '3', JSON.stringify([ - { - key: 'test', - metadata: { - deployed: true, - evaluationMode: 'local', - flagType: 'experiment', - deliveryMethod: 'web', - }, - segments: [ - { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'treatment', - }, - ], - variants: { - control: { - key: 'control', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com', - }, - }, - ], - value: 'control', - }, - off: { - key: 'off', - metadata: { - default: true, - }, - }, - treatment: { - key: 'treatment', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com/2', - }, - }, - ], - value: 'treatment', - }, - }, - }, + createRedirectFlag('test', 'treatment', 'http://test.com/2'), ]), ); @@ -182,42 +82,7 @@ describe('initializeExperiment', () => { initializeExperiment( '4', JSON.stringify([ - { - key: 'test', - metadata: { - deployed: true, - evaluationMode: 'local', - flagType: 'experiment', - deliveryMethod: 'web', - }, - segments: [ - { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'control', - }, - ], - variants: { - control: { - key: 'control', - payload: [], - value: 'control', - }, - treatment: { - key: 'treatment', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com/2', - }, - }, - ], - value: 'treatment', - }, - }, - }, + createRedirectFlag('test', 'control', 'http://test.com/2'), ]), ); @@ -248,49 +113,7 @@ describe('initializeExperiment', () => { initializeExperiment( '5', JSON.stringify([ - { - key: 'test', - metadata: { - deployed: true, - evaluationMode: 'local', - experimentKey: 'exp-1', - flagType: 'experiment', - deliveryMethod: 'web', - }, - segments: [ - { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'treatment', - }, - ], - variants: { - control: { - key: 'control', - payload: [], - value: 'control', - }, - off: { - key: 'off', - metadata: { - default: true, - }, - }, - treatment: { - key: 'treatment', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com/2', - }, - }, - ], - value: 'treatment', - }, - }, - }, + createRedirectFlag('test', 'treatment', 'http://test.com/2'), ]), ); @@ -324,55 +147,7 @@ describe('initializeExperiment', () => { initializeExperiment( '6', JSON.stringify([ - { - key: 'test', - metadata: { - deployed: true, - evaluationMode: 'local', - flagType: 'experiment', - deliveryMethod: 'web', - }, - segments: [ - { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'control', - }, - ], - variants: { - control: { - key: 'treatment', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com', - }, - }, - ], - value: 'control', - }, - off: { - key: 'off', - metadata: { - default: true, - }, - }, - treatment: { - key: 'treatment', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com/2', - }, - }, - ], - value: 'treatment', - }, - }, - }, + createRedirectFlag('test', 'treatment', 'http://test.com/2'), ]), ); @@ -400,67 +175,35 @@ describe('initializeExperiment', () => { // @ts-ignore mockGetGlobalScope.mockReturnValue(mockGlobal); - initializeExperiment( - '7', - JSON.stringify([ - { - key: 'test', - metadata: { - deployed: true, - evaluationMode: 'local', - flagType: 'experiment', - deliveryMethod: 'web', - }, - segments: [ - { - conditions: [ - [ - { - op: 'regex does not match', - selector: ['context', 'page', 'url'], - values: ['^http:\\/\\/test.com/$'], - }, - ], - ], - metadata: { - segmentName: 'Page not targeted', - trackExposure: false, - }, - variant: 'off', - }, + const pageTargetingSegments = [ + { + conditions: [ + [ { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'treatment', + op: 'regex does not match', + selector: ['context', 'page', 'url'], + values: ['^http:\\/\\/test.com/$'], }, ], - variants: { - control: { - key: 'control', - payload: [], - value: 'control', - }, - off: { - key: 'off', - metadata: { - default: true, - }, - }, - treatment: { - key: 'treatment', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com/2', - }, - }, - ], - value: 'treatment', - }, - }, + ], + metadata: { + segmentName: 'Page not targeted', + trackExposure: false, }, + variant: 'off', + }, + ]; + + initializeExperiment( + '7', + JSON.stringify([ + createRedirectFlag( + 'test', + 'treatment', + 'http://test.com/2', + undefined, + pageTargetingSegments, + ), ]), ); @@ -494,55 +237,12 @@ describe('initializeExperiment', () => { initializeExperiment( '8', JSON.stringify([ - { - key: 'test', - metadata: { - deployed: true, - evaluationMode: 'local', - flagType: 'experiment', - deliveryMethod: 'web', - }, - segments: [ - { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'treatment', - }, - ], - variants: { - control: { - key: 'control', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com', - }, - }, - ], - value: 'control', - }, - off: { - key: 'off', - metadata: { - default: true, - }, - }, - treatment: { - key: 'treatment', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com/2?param3=c', - }, - }, - ], - value: 'treatment', - }, - }, - }, + createRedirectFlag( + 'test', + 'treatment', + 'http://test.com/2?param3=c', + 'http://test.com/', + ), ]), ); @@ -556,42 +256,7 @@ describe('initializeExperiment', () => { initializeExperiment( '9', JSON.stringify([ - { - key: 'test', - metadata: { - deployed: true, - evaluationMode: 'local', - flagType: 'experiment', - deliveryMethod: 'web', - }, - segments: [ - { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'control', - }, - ], - variants: { - control: { - key: 'control', - payload: [], - value: 'control', - }, - treatment: { - key: 'treatment', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com/2', - }, - }, - ], - value: 'treatment', - }, - }, - }, + createRedirectFlag('test', 'control', 'http://test.com/2?param3=c'), ]), ); @@ -607,53 +272,34 @@ describe('initializeExperiment', () => { writable: true, }); jest.spyOn(experimentCore, 'getGlobalScope'); - initializeExperiment( - '10', - JSON.stringify([ - { - key: 'test', - metadata: { - deployed: true, - evaluationMode: 'local', - flagType: 'experiment', - deliveryMethod: 'web', - }, - segments: [ - { - conditions: [ - [ - { - op: 'regex does not match', - selector: ['context', 'page', 'url'], - values: ['^http:\\/\\/test.*'], - }, - ], - ], - metadata: { - segmentName: 'Page not targeted', - trackExposure: false, - }, - variant: 'off', - }, + const pageTargetingSegments = [ + { + conditions: [ + [ { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'treatment', + op: 'regex does not match', + selector: ['context', 'page', 'url'], + values: ['^http:\\/\\/test.*'], }, ], - variants: { - off: { - key: 'off', - metadata: { - default: true, - }, - }, - treatment: { - value: 'treatment', - }, - }, + ], + metadata: { + segmentName: 'Page not targeted', + trackExposure: false, }, + variant: 'off', + }, + ]; + initializeExperiment( + '10', + JSON.stringify([ + createRedirectFlag( + 'test', + 'treatment', + 'http://test.com/2', + undefined, + pageTargetingSegments, + ), ]), ); expect(mockExposure).toHaveBeenCalledWith('test'); @@ -666,174 +312,65 @@ describe('initializeExperiment', () => { }, writable: true, }); - initializeExperiment( - '11', - JSON.stringify([ - { - key: 'test', - metadata: { - deployed: true, - evaluationMode: 'local', - flagType: 'experiment', - deliveryMethod: 'web', - }, - segments: [ - { - conditions: [ - [ - { - op: 'regex match', - selector: ['context', 'page', 'url'], - values: ['.*test\\.com$'], - }, - ], - ], - metadata: { - segmentName: 'Page is excluded', - trackExposure: false, - }, - variant: 'off', - }, + const pageTargetingSegments = [ + { + conditions: [ + [ { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'treatment', + op: 'regex match', + selector: ['context', 'page', 'url'], + values: ['.*test\\.com$'], }, ], - variants: { - off: { - key: 'off', - metadata: { - default: true, - }, - }, - treatment: { - key: 'treatment', - value: 'treatment', - }, - }, + ], + metadata: { + segmentName: 'Page is excluded', + trackExposure: false, }, + variant: 'off', + }, + ]; + initializeExperiment( + '11', + JSON.stringify([ + createRedirectFlag( + 'test', + 'treatment', + 'http://test.com/2', + undefined, + pageTargetingSegments, + ), ]), ); expect(mockExposure).not.toHaveBeenCalled(); }); test('test remote evaluation successful', async () => { - const remoteFlags = [ - { - key: 'test', - metadata: { - deployed: true, - evaluationMode: 'local', - experimentKey: 'exp-1', - flagType: 'experiment', - flagVersion: 1, - deliveryMethod: 'web', - }, - segments: [ - { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'treatment', - }, - ], - variants: { - control: { - key: 'control', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com', - }, - }, - ], - value: 'control', - }, - off: { - key: 'off', - metadata: { - default: true, - }, - }, - treatment: { - key: 'treatment', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com/2', - }, - }, - ], - value: 'treatment', - }, - }, - }, - ]; - const initialRemoteFlags = [ - { - key: 'test', - metadata: { - deployed: true, - evaluationMode: 'remote', - flagType: 'experiment', - deliveryMethod: 'web', - }, - segments: [ - { - metadata: { - segmentName: 'All Other Users', - }, - variant: 'off', - }, - ], - variants: { - control: { - key: 'control', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com', - }, - }, - ], - value: 'control', - }, - off: { - key: 'off', - metadata: { - default: true, - }, - }, - treatment: { - key: 'treatment', - payload: [ - { - action: 'redirect', - data: { - url: 'http://test.com/2', - }, - }, - ], - value: 'treatment', - }, - }, - }, + createRedirectFlag( + 'test', + 'treatment', + 'http://test.com/2', + undefined, + undefined, + 'remote', + ), + ]; + const remoteFlags = [ + createRedirectFlag('test', 'treatment', 'http://test.com/2'), ]; const mockHttpClient = new MockHttpClient(JSON.stringify(remoteFlags)); - await initializeExperiment('12', JSON.stringify(initialRemoteFlags), { + initializeExperiment('12', JSON.stringify(initialRemoteFlags), { httpClient: mockHttpClient, + }).then(() => { + expect(mockGlobal.location.replace).toHaveBeenCalledWith( + 'http://test.com/2', + ); + expect(mockExposure).toHaveBeenCalledWith('test'); }); - expect(mockGlobal.location.replace).toHaveBeenCalledWith( - 'http://test.com/2', - ); - expect(mockExposure).toHaveBeenCalledWith('test'); + // exposure should not have been called before the remote fetch resolves + expect(mockExposure).toHaveBeenCalledTimes(0); }); }); diff --git a/packages/experiment-tag/test/util/create-flag.ts b/packages/experiment-tag/test/util/create-flag.ts new file mode 100644 index 00000000..12bfcca3 --- /dev/null +++ b/packages/experiment-tag/test/util/create-flag.ts @@ -0,0 +1,62 @@ +export const createRedirectFlag = ( + flagKey = 'test', + variant: string, + treatmentUrl: string, + controlUrl: string | undefined = undefined, + segments: any[] = [], + evaluationMode = 'local', +) => { + const controlPayload = controlUrl + ? [ + { + action: 'redirect', + data: { + url: controlUrl, + }, + }, + ] + : []; + return { + key: flagKey, + metadata: { + deployed: true, + evaluationMode: evaluationMode, + flagType: 'experiment', + deliveryMethod: 'web', + }, + segments: [ + ...segments, + { + metadata: { + segmentName: 'All Other Users', + }, + variant: variant, + }, + ], + variants: { + control: { + key: 'control', + payload: controlPayload, + value: 'control', + }, + off: { + key: 'off', + metadata: { + default: true, + }, + }, + treatment: { + key: 'treatment', + payload: [ + { + action: 'redirect', + data: { + url: treatmentUrl, + }, + }, + ], + value: 'treatment', + }, + }, + }; +}; diff --git a/packages/experiment-tag/test/util/mock-http-client.ts b/packages/experiment-tag/test/util/mock-http-client.ts index 8cfaa3f9..3f0467ce 100644 --- a/packages/experiment-tag/test/util/mock-http-client.ts +++ b/packages/experiment-tag/test/util/mock-http-client.ts @@ -1,9 +1,11 @@ -export interface SimpleResponse { +// interfaces copied frm experiment-browser + +interface SimpleResponse { status: number; body: string; } -export interface HttpClient { +interface HttpClient { request( requestUrl: string, method: string, @@ -13,11 +15,6 @@ export interface HttpClient { ): Promise; } -export interface SimpleResponse { - status: number; - body: string; -} - export class MockHttpClient implements HttpClient { private response: SimpleResponse; From 27b71cdac0df992b747238376e77a1bdd22d77aa Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Wed, 13 Nov 2024 15:38:21 -0800 Subject: [PATCH 05/22] add unit tests --- packages/experiment-tag/src/experiment.ts | 20 ++- .../experiment-tag/test/experiment.test.ts | 164 +++++++++++++++--- .../experiment-tag/test/util/create-flag.ts | 60 +++++++ 3 files changed, 209 insertions(+), 35 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 985fe2f3..bab0d44c 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -120,6 +120,12 @@ export const initializeExperiment = async ( }; flag.segments = [...pageTargetingSegments, previewSegment]; + + if (flag?.metadata?.evaluationMode !== 'local') { + // make the remote flag locally evaluable + flag.metadata = flag.metadata || {}; + flag.metadata.evaluationMode = 'local'; + } } }); initialFlags = JSON.stringify(parsedFlags); @@ -130,9 +136,6 @@ export const initializeExperiment = async ( parsedFlags.forEach((flag: EvaluationFlag) => { if (flag?.metadata?.evaluationMode !== 'local') { remoteFlagKeys.add(flag.key); - // make the remote flag locally evaluable - flag.metadata = flag.metadata || {}; - flag.metadata.evaluationMode = 'local'; // check whether any remote flags are blocking if (flag.metadata?.isBlocking) { remoteBlocking = true; @@ -180,11 +183,14 @@ export const initializeExperiment = async ( if (remoteFlagKeys.size === 0) { return; } - // fetch remote flags - await globalScope.webExperiment.doFlags(); - // apply remote variants - applyVariants(globalScope.webExperiment.all(), remoteFlagKeys); + try { + await globalScope.webExperiment.doFlags(); + // apply remote variants + applyVariants(globalScope.webExperiment.all(), remoteFlagKeys); + } catch (error) { + console.warn('Error fetching remote flags:', error); + } }; const applyVariants = ( diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index 5808813f..35534e61 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -4,7 +4,10 @@ import { initializeExperiment } from 'src/experiment'; import * as experiment from 'src/experiment'; import * as util from 'src/util'; import { MockHttpClient } from './util/mock-http-client'; -import { createRedirectFlag } from './util/create-flag'; +import { createMutateFlag, createRedirectFlag } from './util/create-flag'; +import { stringify } from 'ts-jest'; + +let apiKey: number = 0; jest.mock('src/messenger', () => { return { @@ -25,6 +28,7 @@ describe('initializeExperiment', () => { let mockGlobal; beforeEach(() => { + apiKey++; jest.spyOn(experimentCore, 'isLocalStorageAvailable').mockReturnValue(true); jest.clearAllMocks(); mockGlobal = { @@ -46,7 +50,7 @@ describe('initializeExperiment', () => { }); test('should initialize experiment with empty user', () => { - initializeExperiment('1', JSON.stringify([])); + initializeExperiment(stringify(apiKey), JSON.stringify([])); expect(ExperimentClient.prototype.setUser).toHaveBeenCalledWith({ web_exp_id: 'mock', }); @@ -60,13 +64,13 @@ describe('initializeExperiment', () => { jest .spyOn(experimentCore, 'isLocalStorageAvailable') .mockReturnValue(false); - initializeExperiment('2', ''); + initializeExperiment(stringify(apiKey), ''); expect(mockGlobal.localStorage.getItem).toHaveBeenCalledTimes(0); }); test('treatment variant on control page - should redirect and call exposure', () => { initializeExperiment( - '3', + stringify(apiKey), JSON.stringify([ createRedirectFlag('test', 'treatment', 'http://test.com/2'), ]), @@ -80,7 +84,7 @@ describe('initializeExperiment', () => { test('control variant on control page - should not redirect but call exposure', () => { initializeExperiment( - '4', + stringify(apiKey), JSON.stringify([ createRedirectFlag('test', 'control', 'http://test.com/2'), ]), @@ -111,7 +115,7 @@ describe('initializeExperiment', () => { mockGetGlobalScope.mockReturnValue(mockGlobal); initializeExperiment( - '5', + stringify(apiKey), JSON.stringify([ createRedirectFlag('test', 'treatment', 'http://test.com/2'), ]), @@ -145,7 +149,7 @@ describe('initializeExperiment', () => { mockGetGlobalScope.mockReturnValue(mockGlobal); initializeExperiment( - '6', + stringify(apiKey), JSON.stringify([ createRedirectFlag('test', 'treatment', 'http://test.com/2'), ]), @@ -195,7 +199,7 @@ describe('initializeExperiment', () => { ]; initializeExperiment( - '7', + stringify(apiKey), JSON.stringify([ createRedirectFlag( 'test', @@ -235,7 +239,7 @@ describe('initializeExperiment', () => { mockGetGlobalScope.mockReturnValue(mockGlobal); initializeExperiment( - '8', + stringify(apiKey), JSON.stringify([ createRedirectFlag( 'test', @@ -254,7 +258,7 @@ describe('initializeExperiment', () => { test('should behave as control variant when payload is empty', () => { initializeExperiment( - '9', + stringify(apiKey), JSON.stringify([ createRedirectFlag('test', 'control', 'http://test.com/2?param3=c'), ]), @@ -291,7 +295,7 @@ describe('initializeExperiment', () => { }, ]; initializeExperiment( - '10', + stringify(apiKey), JSON.stringify([ createRedirectFlag( 'test', @@ -331,7 +335,7 @@ describe('initializeExperiment', () => { }, ]; initializeExperiment( - '11', + stringify(apiKey), JSON.stringify([ createRedirectFlag( 'test', @@ -345,32 +349,136 @@ describe('initializeExperiment', () => { expect(mockExposure).not.toHaveBeenCalled(); }); - test('test remote evaluation successful', async () => { - const initialRemoteFlags = [ + test('remote evaluation - fetch successful', async () => { + const initialFlags = [ + // remote flag + createMutateFlag('test-2', 'treatment', [], [], [], 'remote'), + // local flag + createMutateFlag('test-1', 'treatment'), + ]; + const remoteFlags = [createMutateFlag('test-2', 'treatment')]; + + const mockHttpClient = new MockHttpClient(JSON.stringify(remoteFlags)); + + initializeExperiment(stringify(apiKey), JSON.stringify(initialFlags), { + httpClient: mockHttpClient, + }).then(() => { + // check remote flag variant actions called after successful fetch + expect(mockExposure).toHaveBeenCalledTimes(2); + expect(mockExposure).toHaveBeenCalledWith('test-2'); + }); + // check local flag variant actions called + expect(mockExposure).toHaveBeenCalledTimes(1); + expect(mockExposure).toHaveBeenCalledWith('test-1'); + }); + + test('remote evaluation - fetch fail, local evaluation success', async () => { + const initialFlags = [ + // remote flag + createMutateFlag('test-2', 'treatment', [], [], [], 'remote'), + // local flag + createMutateFlag('test-1', 'treatment'), + ]; + const remoteFlags = [createMutateFlag('test-2', 'treatment')]; + + const mockHttpClient = new MockHttpClient(JSON.stringify(remoteFlags), 404); + + initializeExperiment(stringify(apiKey), JSON.stringify(initialFlags), { + httpClient: mockHttpClient, + }).then(() => { + // check remote fetch failed safely + expect(mockExposure).toHaveBeenCalledTimes(1); + }); + // check local flag variant actions called + expect(mockExposure).toHaveBeenCalledTimes(1); + expect(mockExposure).toHaveBeenCalledWith('test-1'); + }); + + test('remote evaluation - fetch fail, test no variant actions called', async () => { + const initialFlags = [ + // remote flag + createMutateFlag('test', 'treatment', [], [], [], 'remote'), + ]; + const remoteFlags = [createMutateFlag('test', 'treatment')]; + + const mockHttpClient = new MockHttpClient(JSON.stringify(remoteFlags), 404); + + initializeExperiment(stringify(apiKey), JSON.stringify(initialFlags), { + httpClient: mockHttpClient, + }).then(() => { + // check remote fetch failed safely + expect(mockExposure).toHaveBeenCalledTimes(0); + }); + // check local flag variant actions called + expect(mockExposure).toHaveBeenCalledTimes(0); + }); + + test('remote evaluation - test preview successful, does not fetch remote flags', async () => { + const mockGlobal = { + localStorage: { + getItem: jest.fn().mockReturnValue(undefined), + setItem: jest.fn(), + }, + location: { + href: 'http://test.com/', + replace: jest.fn(), + search: '?test=treatment&PREVIEW=true', + }, + document: { referrer: '' }, + history: { replaceState: jest.fn() }, + }; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + mockGetGlobalScope.mockReturnValue(mockGlobal); + const initialFlags = [ + // remote flag + createMutateFlag('test', 'treatment', [], [], [], 'remote'), + ]; + const remoteFlags = [createMutateFlag('test', 'treatment')]; + + const mockHttpClient = new MockHttpClient(JSON.stringify(remoteFlags), 200); + + const doFlagsMock = jest.spyOn(ExperimentClient.prototype, 'doFlags'); + initializeExperiment(stringify(apiKey), JSON.stringify(initialFlags), { + httpClient: mockHttpClient, + }).then(() => { + // check remote fetch not called + expect(doFlagsMock).toHaveBeenCalledTimes(0); + }); + // check local flag variant actions called + expect(mockExposure).toHaveBeenCalledTimes(1); + expect(mockExposure).toHaveBeenCalledWith('test'); + }); + + test('remote evaluation - fetch successful, fetched flag overwrites initial flag', async () => { + const initialFlags = [ + // remote flag createRedirectFlag( 'test', - 'treatment', + 'control', 'http://test.com/2', undefined, - undefined, + [], 'remote', ), ]; const remoteFlags = [ createRedirectFlag('test', 'treatment', 'http://test.com/2'), ]; + const mockHttpClient = new MockHttpClient(JSON.stringify(remoteFlags), 200); - const mockHttpClient = new MockHttpClient(JSON.stringify(remoteFlags)); - - initializeExperiment('12', JSON.stringify(initialRemoteFlags), { - httpClient: mockHttpClient, - }).then(() => { - expect(mockGlobal.location.replace).toHaveBeenCalledWith( - 'http://test.com/2', - ); - expect(mockExposure).toHaveBeenCalledWith('test'); - }); - // exposure should not have been called before the remote fetch resolves - expect(mockExposure).toHaveBeenCalledTimes(0); + await initializeExperiment( + stringify(apiKey), + JSON.stringify(initialFlags), + { + httpClient: mockHttpClient, + }, + ); + // check treatment variant called + expect(mockExposure).toHaveBeenCalledTimes(1); + expect(mockExposure).toHaveBeenCalledWith('test'); + expect(mockGlobal.location.replace).toHaveBeenCalledWith( + 'http://test.com/2', + ); }); }); diff --git a/packages/experiment-tag/test/util/create-flag.ts b/packages/experiment-tag/test/util/create-flag.ts index 12bfcca3..9dd0ad1e 100644 --- a/packages/experiment-tag/test/util/create-flag.ts +++ b/packages/experiment-tag/test/util/create-flag.ts @@ -60,3 +60,63 @@ export const createRedirectFlag = ( }, }; }; + +export const createMutateFlag = ( + flagKey = 'test', + variant: string, + treatmentMutations: any[] = [], + controlMutations: any[] = [], + segments: any[] = [], + evaluationMode = 'local', +) => { + return { + key: flagKey, + metadata: { + deployed: true, + evaluationMode: evaluationMode, + flagType: 'experiment', + deliveryMethod: 'web', + }, + segments: [ + ...segments, + { + metadata: { + segmentName: 'All Other Users', + }, + variant: variant, + }, + ], + variants: { + control: { + key: 'control', + payload: [ + { + action: 'mutate', + data: { + mutations: controlMutations, + }, + }, + ], + value: 'control', + }, + off: { + key: 'off', + metadata: { + default: true, + }, + }, + treatment: { + key: 'treatment', + payload: [ + { + action: 'mutate', + data: { + mutations: treatmentMutations, + }, + }, + ], + value: 'treatment', + }, + }, + }; +}; From 987031e28329f7bdedc2af56de34fdca45c79316 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Mon, 18 Nov 2024 14:49:05 -0800 Subject: [PATCH 06/22] update unit tests, update getFlags with deliveryMethod arg --- .../src/experimentClient.ts | 7 +-- packages/experiment-core/src/api/flag-api.ts | 6 ++- packages/experiment-tag/src/experiment.ts | 7 +-- .../experiment-tag/test/experiment.test.ts | 46 +++++++++++++++---- .../test/util/mock-http-client.ts | 4 ++ 5 files changed, 53 insertions(+), 17 deletions(-) diff --git a/packages/experiment-browser/src/experimentClient.ts b/packages/experiment-browser/src/experimentClient.ts index 2591ce77..c6dc14a1 100644 --- a/packages/experiment-browser/src/experimentClient.ts +++ b/packages/experiment-browser/src/experimentClient.ts @@ -703,6 +703,8 @@ export class ExperimentClient implements Client { public async doFlags(): Promise { try { + const isWebExperiment = + this.config?.['internalInstanceNameSuffix'] === 'web'; const user: ExperimentUser = { user_id: this.getUser().user_id, device_id: this.getUser().device_id, @@ -713,9 +715,8 @@ export class ExperimentClient implements Client { libraryVersion: PACKAGE_VERSION, timeoutMillis: this.config.fetchTimeoutMillis, }, - this.config?.['internalInstanceNameSuffix'] === 'web' - ? user - : undefined, + isWebExperiment ? user : undefined, + isWebExperiment ? 'web' : undefined, ); this.flags.clear(); this.flags.putAll(flags); diff --git a/packages/experiment-core/src/api/flag-api.ts b/packages/experiment-core/src/api/flag-api.ts index 09d95b66..aefccb49 100644 --- a/packages/experiment-core/src/api/flag-api.ts +++ b/packages/experiment-core/src/api/flag-api.ts @@ -13,6 +13,7 @@ export interface FlagApi { getFlags( options?: GetFlagsOptions, user?: Record, + deliveryMethod?: string | undefined, ): Promise>; } @@ -34,6 +35,7 @@ export class SdkFlagApi implements FlagApi { public async getFlags( options?: GetFlagsOptions, user?: Record, + deliveryMethod?: string | undefined, ): Promise> { const headers: Record = { Authorization: `Api-Key ${this.deploymentKey}`, @@ -47,7 +49,9 @@ export class SdkFlagApi implements FlagApi { headers['X-Amp-Exp-User'] = Base64.encodeURL(JSON.stringify(user)); } const response = await this.httpClient.request({ - requestUrl: `${this.serverUrl}/sdk/v2/flags`, + requestUrl: + `${this.serverUrl}/sdk/v2/flags` + + (deliveryMethod ? `?delivery_method=${deliveryMethod}` : ''), method: 'GET', headers: headers, timeoutMillis: options?.timeoutMillis, diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index bab0d44c..12ec310d 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -151,8 +151,6 @@ export const initializeExperiment = async ( // @ts-ignore internalInstanceNameSuffix: 'web', fetchOnStart: false, - flagsServerUrl: 'https://flag.lab.amplitude.com?delivery_method=web', - pollOnStart: false, initialFlags: initialFlags, ...config, }); @@ -194,10 +192,10 @@ export const initializeExperiment = async ( }; const applyVariants = ( - variants: Variants | undefined, + variants: Variants, flagKeys: Set | undefined = undefined, ) => { - if (!variants) { + if (Object.keys(variants).length === 0) { return; } const globalScope = getGlobalScope(); @@ -242,7 +240,6 @@ const applyVariants = ( } } }; - const handleRedirect = (action, key: string, variant: Variant) => { const globalScope = getGlobalScope(); if (!globalScope) { diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index 35534e61..34324976 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -1,13 +1,15 @@ import * as experimentCore from '@amplitude/experiment-core'; import { ExperimentClient } from '@amplitude/experiment-js-client'; +import { Base64 } from 'js-base64'; import { initializeExperiment } from 'src/experiment'; import * as experiment from 'src/experiment'; import * as util from 'src/util'; -import { MockHttpClient } from './util/mock-http-client'; -import { createMutateFlag, createRedirectFlag } from './util/create-flag'; import { stringify } from 'ts-jest'; -let apiKey: number = 0; +import { createMutateFlag, createRedirectFlag } from './util/create-flag'; +import { MockHttpClient } from './util/mock-http-client'; + +let apiKey = 0; jest.mock('src/messenger', () => { return { @@ -28,9 +30,9 @@ describe('initializeExperiment', () => { let mockGlobal; beforeEach(() => { + jest.clearAllMocks(); apiKey++; jest.spyOn(experimentCore, 'isLocalStorageAvailable').mockReturnValue(true); - jest.clearAllMocks(); mockGlobal = { localStorage: { getItem: jest.fn().mockReturnValue(undefined), @@ -49,6 +51,10 @@ describe('initializeExperiment', () => { mockGetGlobalScope.mockReturnValue(mockGlobal); }); + afterEach(() => { + jest.clearAllMocks(); + }); + test('should initialize experiment with empty user', () => { initializeExperiment(stringify(apiKey), JSON.stringify([])); expect(ExperimentClient.prototype.setUser).toHaveBeenCalledWith({ @@ -349,7 +355,31 @@ describe('initializeExperiment', () => { expect(mockExposure).not.toHaveBeenCalled(); }); - test('remote evaluation - fetch successful', async () => { + test('remote evaluation - request web remote flags', () => { + const mockUser = { user_id: 'user_id', device_id: 'device_id' }; + jest.spyOn(ExperimentClient.prototype, 'getUser').mockReturnValue(mockUser); + + const initialFlags = [ + // remote flag + createMutateFlag('test-2', 'treatment', [], [], [], 'remote'), + ]; + + const mockHttpClient = new MockHttpClient(JSON.stringify([])); + + initializeExperiment(stringify(apiKey), JSON.stringify(initialFlags), { + httpClient: mockHttpClient, + }).then(() => { + expect(mockHttpClient.requestUrl).toBe( + 'https://flag.lab.amplitude.com/sdk/v2/flags?delivery_method=web', + ); + // check flag fetch called with correct query param and header + expect(mockHttpClient.requestHeader['X-Amp-Exp-User']).toBe( + Base64.encodeURL(JSON.stringify(mockUser)), + ); + }); + }); + + test('remote evaluation - fetch successful', () => { const initialFlags = [ // remote flag createMutateFlag('test-2', 'treatment', [], [], [], 'remote'), @@ -372,7 +402,7 @@ describe('initializeExperiment', () => { expect(mockExposure).toHaveBeenCalledWith('test-1'); }); - test('remote evaluation - fetch fail, local evaluation success', async () => { + test('remote evaluation - fetch fail, local evaluation success', () => { const initialFlags = [ // remote flag createMutateFlag('test-2', 'treatment', [], [], [], 'remote'), @@ -394,7 +424,7 @@ describe('initializeExperiment', () => { expect(mockExposure).toHaveBeenCalledWith('test-1'); }); - test('remote evaluation - fetch fail, test no variant actions called', async () => { + test('remote evaluation - fetch fail, test no variant actions called', () => { const initialFlags = [ // remote flag createMutateFlag('test', 'treatment', [], [], [], 'remote'), @@ -413,7 +443,7 @@ describe('initializeExperiment', () => { expect(mockExposure).toHaveBeenCalledTimes(0); }); - test('remote evaluation - test preview successful, does not fetch remote flags', async () => { + test('remote evaluation - test preview successful, does not fetch remote flags', () => { const mockGlobal = { localStorage: { getItem: jest.fn().mockReturnValue(undefined), diff --git a/packages/experiment-tag/test/util/mock-http-client.ts b/packages/experiment-tag/test/util/mock-http-client.ts index 3f0467ce..5cd86767 100644 --- a/packages/experiment-tag/test/util/mock-http-client.ts +++ b/packages/experiment-tag/test/util/mock-http-client.ts @@ -17,6 +17,8 @@ interface HttpClient { export class MockHttpClient implements HttpClient { private response: SimpleResponse; + public requestUrl; + public requestHeader; constructor(responseBody: string, status = 200) { this.response = { @@ -32,6 +34,8 @@ export class MockHttpClient implements HttpClient { data: string, timeoutMillis?: number, ): Promise { + this.requestUrl = requestUrl; + this.requestHeader = headers; return Promise.resolve(this.response); } } From f729fe41bd68a2c0ba8f78eb5536891f81b93d4b Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Mon, 18 Nov 2024 14:56:31 -0800 Subject: [PATCH 07/22] fix lint --- packages/experiment-core/src/api/flag-api.ts | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/experiment-core/src/api/flag-api.ts b/packages/experiment-core/src/api/flag-api.ts index aefccb49..9cc00698 100644 --- a/packages/experiment-core/src/api/flag-api.ts +++ b/packages/experiment-core/src/api/flag-api.ts @@ -1,6 +1,7 @@ +import { Base64 } from 'js-base64'; + import { EvaluationFlag } from '../evaluation/flag'; import { HttpClient } from '../transport/http'; -import { Base64 } from 'js-base64'; export type GetFlagsOptions = { libraryName: string; From a93d63a638fc529da9159537aee5859a0ff0816f Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Mon, 18 Nov 2024 15:17:24 -0800 Subject: [PATCH 08/22] fix tests --- packages/experiment-tag/test/experiment.test.ts | 7 +++---- 1 file changed, 3 insertions(+), 4 deletions(-) diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index 34324976..846b6a8b 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -23,9 +23,11 @@ jest.spyOn(experiment, 'setUrlChangeListener').mockReturnValue(undefined); describe('initializeExperiment', () => { const mockGetGlobalScope = jest.spyOn(experimentCore, 'getGlobalScope'); + const mockExposure = jest.spyOn(ExperimentClient.prototype, 'exposure'); + const mockUser = { user_id: 'user_id', device_id: 'device_id' }; jest.spyOn(ExperimentClient.prototype, 'setUser'); jest.spyOn(ExperimentClient.prototype, 'all'); - const mockExposure = jest.spyOn(ExperimentClient.prototype, 'exposure'); + jest.spyOn(ExperimentClient.prototype, 'getUser').mockReturnValue(mockUser); jest.spyOn(util, 'UUID').mockReturnValue('mock'); let mockGlobal; @@ -356,9 +358,6 @@ describe('initializeExperiment', () => { }); test('remote evaluation - request web remote flags', () => { - const mockUser = { user_id: 'user_id', device_id: 'device_id' }; - jest.spyOn(ExperimentClient.prototype, 'getUser').mockReturnValue(mockUser); - const initialFlags = [ // remote flag createMutateFlag('test-2', 'treatment', [], [], [], 'remote'), From 6a9180099370eddc5891f1a6b1222505b02dc575 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Mon, 18 Nov 2024 15:19:58 -0800 Subject: [PATCH 09/22] fix doFlags --- packages/experiment-browser/src/experimentClient.ts | 4 ++-- packages/experiment-tag/test/experiment.test.ts | 7 ++++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/packages/experiment-browser/src/experimentClient.ts b/packages/experiment-browser/src/experimentClient.ts index c6dc14a1..753061f4 100644 --- a/packages/experiment-browser/src/experimentClient.ts +++ b/packages/experiment-browser/src/experimentClient.ts @@ -706,8 +706,8 @@ export class ExperimentClient implements Client { const isWebExperiment = this.config?.['internalInstanceNameSuffix'] === 'web'; const user: ExperimentUser = { - user_id: this.getUser().user_id, - device_id: this.getUser().device_id, + user_id: this.getUser()?.user_id, + device_id: this.getUser()?.device_id, }; const flags = await this.flagApi.getFlags( { diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index 846b6a8b..34324976 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -23,11 +23,9 @@ jest.spyOn(experiment, 'setUrlChangeListener').mockReturnValue(undefined); describe('initializeExperiment', () => { const mockGetGlobalScope = jest.spyOn(experimentCore, 'getGlobalScope'); - const mockExposure = jest.spyOn(ExperimentClient.prototype, 'exposure'); - const mockUser = { user_id: 'user_id', device_id: 'device_id' }; jest.spyOn(ExperimentClient.prototype, 'setUser'); jest.spyOn(ExperimentClient.prototype, 'all'); - jest.spyOn(ExperimentClient.prototype, 'getUser').mockReturnValue(mockUser); + const mockExposure = jest.spyOn(ExperimentClient.prototype, 'exposure'); jest.spyOn(util, 'UUID').mockReturnValue('mock'); let mockGlobal; @@ -358,6 +356,9 @@ describe('initializeExperiment', () => { }); test('remote evaluation - request web remote flags', () => { + const mockUser = { user_id: 'user_id', device_id: 'device_id' }; + jest.spyOn(ExperimentClient.prototype, 'getUser').mockReturnValue(mockUser); + const initialFlags = [ // remote flag createMutateFlag('test-2', 'treatment', [], [], [], 'remote'), From d8a5109f3e47868740c407f833265539f8055862 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Mon, 18 Nov 2024 15:57:25 -0800 Subject: [PATCH 10/22] fix web remote eval preview unit test --- packages/experiment-tag/src/experiment.ts | 6 +++--- packages/experiment-tag/test/experiment.test.ts | 9 ++------- 2 files changed, 5 insertions(+), 10 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 12ec310d..96b9b6bb 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -30,8 +30,6 @@ const appliedMutations: MutationController[] = []; let previousUrl: string | undefined; // Cache to track exposure for the current URL, should be cleared on URL change let urlExposureCache: { [url: string]: { [key: string]: string | undefined } }; -const remoteFlagKeys: Set = new Set(); -const locaFlagKeys: Set = new Set(); export const initializeExperiment = async ( apiKey: string, @@ -132,6 +130,9 @@ export const initializeExperiment = async ( } let remoteBlocking = false; + const remoteFlagKeys: Set = new Set(); + const locaFlagKeys: Set = new Set(); + // get set of remote flag keys parsedFlags.forEach((flag: EvaluationFlag) => { if (flag?.metadata?.evaluationMode !== 'local') { @@ -150,7 +151,6 @@ export const initializeExperiment = async ( // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore internalInstanceNameSuffix: 'web', - fetchOnStart: false, initialFlags: initialFlags, ...config, }); diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index 34324976..1e667800 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -19,19 +19,18 @@ jest.mock('src/messenger', () => { }; }); -jest.spyOn(experiment, 'setUrlChangeListener').mockReturnValue(undefined); - describe('initializeExperiment', () => { const mockGetGlobalScope = jest.spyOn(experimentCore, 'getGlobalScope'); jest.spyOn(ExperimentClient.prototype, 'setUser'); jest.spyOn(ExperimentClient.prototype, 'all'); + jest.spyOn(experiment, 'setUrlChangeListener').mockReturnValue(undefined); const mockExposure = jest.spyOn(ExperimentClient.prototype, 'exposure'); jest.spyOn(util, 'UUID').mockReturnValue('mock'); let mockGlobal; beforeEach(() => { - jest.clearAllMocks(); apiKey++; + jest.clearAllMocks(); jest.spyOn(experimentCore, 'isLocalStorageAvailable').mockReturnValue(true); mockGlobal = { localStorage: { @@ -51,10 +50,6 @@ describe('initializeExperiment', () => { mockGetGlobalScope.mockReturnValue(mockGlobal); }); - afterEach(() => { - jest.clearAllMocks(); - }); - test('should initialize experiment with empty user', () => { initializeExperiment(stringify(apiKey), JSON.stringify([])); expect(ExperimentClient.prototype.setUser).toHaveBeenCalledWith({ From c40f7ed71906ba341d0af1ad4049467f550c8436 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Mon, 18 Nov 2024 16:03:54 -0800 Subject: [PATCH 11/22] remove unused util --- packages/experiment-tag/src/util.ts | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/experiment-tag/src/util.ts b/packages/experiment-tag/src/util.ts index 37f95c1f..03cc3c50 100644 --- a/packages/experiment-tag/src/util.ts +++ b/packages/experiment-tag/src/util.ts @@ -88,7 +88,3 @@ export const concatenateQueryParamsOf = ( return resultUrlObj.toString(); }; - -export const hasUserOrDeviceId = (user: ExperimentUser): boolean => { - return !!(user.user_id || user.device_id); -}; From 7db6dd433d1785e3fb5e55dfed8b0da562f20cf4 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Mon, 18 Nov 2024 16:04:17 -0800 Subject: [PATCH 12/22] remove unused import --- packages/experiment-tag/src/util.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/experiment-tag/src/util.ts b/packages/experiment-tag/src/util.ts index 03cc3c50..ec7ac8e4 100644 --- a/packages/experiment-tag/src/util.ts +++ b/packages/experiment-tag/src/util.ts @@ -1,5 +1,4 @@ import { getGlobalScope } from '@amplitude/experiment-core'; -import { ExperimentUser } from '@amplitude/experiment-js-client'; export const getUrlParams = (): Record => { const globalScope = getGlobalScope(); From 2285a9d4e30fb4472a93552d6ad6a6f035fac6a1 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Wed, 20 Nov 2024 11:43:59 -0800 Subject: [PATCH 13/22] fix doFlags user creation --- packages/experiment-browser/src/experimentClient.ts | 9 +++++---- packages/experiment-tag/src/experiment.ts | 1 + 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/packages/experiment-browser/src/experimentClient.ts b/packages/experiment-browser/src/experimentClient.ts index 753061f4..5c633385 100644 --- a/packages/experiment-browser/src/experimentClient.ts +++ b/packages/experiment-browser/src/experimentClient.ts @@ -705,9 +705,10 @@ export class ExperimentClient implements Client { try { const isWebExperiment = this.config?.['internalInstanceNameSuffix'] === 'web'; - const user: ExperimentUser = { - user_id: this.getUser()?.user_id, - device_id: this.getUser()?.device_id, + const user = this.addContext(this.getUser()); + const userAndDeviceId: ExperimentUser = { + user_id: user?.user_id, + device_id: user?.device_id, }; const flags = await this.flagApi.getFlags( { @@ -715,7 +716,7 @@ export class ExperimentClient implements Client { libraryVersion: PACKAGE_VERSION, timeoutMillis: this.config.fetchTimeoutMillis, }, - isWebExperiment ? user : undefined, + isWebExperiment ? userAndDeviceId : undefined, isWebExperiment ? 'web' : undefined, ); this.flags.clear(); diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 96b9b6bb..e2e94237 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -61,6 +61,7 @@ export const initializeExperiment = async ( // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore if (!user.web_exp_id) { + // if user has device_id, migrate it to web_exp_id if (user.device_id) { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore From eea5ef4721e49662d31f5f21c26c7f21b5ff33df Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Wed, 20 Nov 2024 14:49:49 -0800 Subject: [PATCH 14/22] fix web_exp_id generation for backwards compatability --- packages/experiment-tag/src/experiment.ts | 24 +++++-------------- .../experiment-tag/test/experiment.test.ts | 3 ++- 2 files changed, 8 insertions(+), 19 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index e2e94237..8b394687 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -7,7 +7,6 @@ import { } from '@amplitude/experiment-core'; import { Experiment, - ExperimentUser, Variant, Variants, AmplitudeIntegrationPlugin, @@ -47,7 +46,7 @@ export const initializeExperiment = async ( previousUrl = undefined; urlExposureCache = {}; const experimentStorageName = `EXP_${apiKey.slice(0, 10)}`; - let user: ExperimentUser; + let user; try { user = JSON.parse( globalScope.localStorage.getItem(experimentStorageName) || '{}', @@ -56,26 +55,15 @@ export const initializeExperiment = async ( user = {}; } - // create new user if it does not exist, or it does not have web_exp_id - if (Object.keys(user).length === 0) { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - if (!user.web_exp_id) { + // create new user if it does not exist, or it does not have device_id or web_exp_id + if (Object.keys(user).length === 0 || !user.device_id || !user.web_exp_id) { + if (!user.device_id || !user.web_exp_id) { // if user has device_id, migrate it to web_exp_id if (user.device_id) { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore user.web_exp_id = user.device_id; - user.device_id = undefined; - globalScope.localStorage.setItem( - experimentStorageName, - JSON.stringify(user), - ); } else { - user = {}; - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - user.web_exp_id = UUID(); + const uuid = UUID(); + user = { device_id: uuid, web_exp_id: uuid }; } globalScope.localStorage.setItem( experimentStorageName, diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index 1e667800..82946dab 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -53,11 +53,12 @@ describe('initializeExperiment', () => { test('should initialize experiment with empty user', () => { initializeExperiment(stringify(apiKey), JSON.stringify([])); expect(ExperimentClient.prototype.setUser).toHaveBeenCalledWith({ + device_id: 'mock', web_exp_id: 'mock', }); expect(mockGlobal.localStorage.setItem).toHaveBeenCalledWith( 'EXP_1', - JSON.stringify({ web_exp_id: 'mock' }), + JSON.stringify({ device_id: 'mock', web_exp_id: 'mock' }), ); }); From a379647a06da6516f0ca996a775ef8f4e53bb28a Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Wed, 20 Nov 2024 15:45:35 -0800 Subject: [PATCH 15/22] nit: formatting --- packages/experiment-tag/src/experiment.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 8b394687..4b30b732 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -229,6 +229,7 @@ const applyVariants = ( } } }; + const handleRedirect = (action, key: string, variant: Variant) => { const globalScope = getGlobalScope(); if (!globalScope) { From a2ad01ed91434b85a0ec0dacbc337607ce4de3d5 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Mon, 25 Nov 2024 15:24:25 -0800 Subject: [PATCH 16/22] refactor parsing initial flags, add antiflicker for remote blocking flags --- packages/experiment-tag/src/experiment.ts | 98 +++++++++++++---------- 1 file changed, 55 insertions(+), 43 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 4b30b732..d4a79e28 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -83,57 +83,69 @@ export const initializeExperiment = async ( return; } - const parsedFlags = JSON.parse(initialFlags); - // force variant if in preview mode - if (urlParams['PREVIEW']) { - parsedFlags.forEach((flag: EvaluationFlag) => { - if (flag.key in urlParams && urlParams[flag.key] in flag.variants) { - // Strip the preview query param - globalScope.history.replaceState( - {}, - '', - removeQueryParams(globalScope.location.href, ['PREVIEW', flag.key]), - ); - - // Keep page targeting segments - const pageTargetingSegments = flag.segments.filter((segment) => - isPageTargetingSegment(segment), - ); - - // Create or update the preview segment - const previewSegment = { - metadata: { segmentName: 'preview' }, - variant: urlParams[flag.key], - }; - - flag.segments = [...pageTargetingSegments, previewSegment]; - - if (flag?.metadata?.evaluationMode !== 'local') { - // make the remote flag locally evaluable - flag.metadata = flag.metadata || {}; - flag.metadata.evaluationMode = 'local'; - } - } - }); - initialFlags = JSON.stringify(parsedFlags); - } - - let remoteBlocking = false; + let isRemoteBlocking = false; const remoteFlagKeys: Set = new Set(); - const locaFlagKeys: Set = new Set(); + const localFlagKeys: Set = new Set(); + const parsedFlags = JSON.parse(initialFlags); - // get set of remote flag keys parsedFlags.forEach((flag: EvaluationFlag) => { + // force variant if in preview mode + if ( + urlParams['PREVIEW'] && + flag.key in urlParams && + urlParams[flag.key] in flag.variants + ) { + // Strip the preview query param + globalScope.history.replaceState( + {}, + '', + removeQueryParams(globalScope.location.href, ['PREVIEW', flag.key]), + ); + + // Keep page targeting segments + const pageTargetingSegments = flag.segments.filter((segment) => + isPageTargetingSegment(segment), + ); + + // Create or update the preview segment + const previewSegment = { + metadata: { segmentName: 'preview' }, + variant: urlParams[flag.key], + }; + + flag.segments = [...pageTargetingSegments, previewSegment]; + + if (flag?.metadata?.evaluationMode !== 'local') { + // make the remote flag locally evaluable + flag.metadata = flag.metadata || {}; + flag.metadata.evaluationMode = 'local'; + } + } + + // parse through remote flags if (flag?.metadata?.evaluationMode !== 'local') { remoteFlagKeys.add(flag.key); // check whether any remote flags are blocking - if (flag.metadata?.isBlocking) { - remoteBlocking = true; + if (!isRemoteBlocking && flag.metadata?.isBlocking) { + isRemoteBlocking = true; + // Apply anti-flicker css if any remote flags are blocking + if (!globalScope.document.getElementById('amp-exp-css')) { + const id = 'amp-exp-css'; + const s = document.createElement('style'); + s.id = id; + s.innerText = + '* { visibility: hidden !important; background-image: none !important; }'; + document.head.appendChild(s); + globalScope.window.setTimeout(function () { + s.remove(); + }, 1000); + } } } else { - locaFlagKeys.add(flag.key); + localFlagKeys.add(flag.key); } }); + initialFlags = JSON.stringify(parsedFlags); // initialize the experiment globalScope.webExperiment = Experiment.initialize(apiKey, { @@ -160,9 +172,9 @@ export const initializeExperiment = async ( setUrlChangeListener(); // apply local variants - applyVariants(globalScope.webExperiment.all(), locaFlagKeys); + applyVariants(globalScope.webExperiment.all(), localFlagKeys); - if (!remoteBlocking) { + if (!isRemoteBlocking) { // Remove anti-flicker css if remote flags are not blocking globalScope.document.getElementById?.('amp-exp-css')?.remove(); } From bfce7c6d272d922976efa1178c4bd4f8d003e0f4 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Wed, 27 Nov 2024 13:38:53 -0800 Subject: [PATCH 17/22] update getflags options, exclude x-amp-exp-user header when no user/device id, make doFlags private --- .../src/experimentClient.ts | 25 ++++++++----------- packages/experiment-core/src/api/flag-api.ts | 20 +++++++-------- packages/experiment-tag/src/experiment.ts | 9 +++---- 3 files changed, 25 insertions(+), 29 deletions(-) diff --git a/packages/experiment-browser/src/experimentClient.ts b/packages/experiment-browser/src/experimentClient.ts index 5c633385..a032e24c 100644 --- a/packages/experiment-browser/src/experimentClient.ts +++ b/packages/experiment-browser/src/experimentClient.ts @@ -701,24 +701,21 @@ export class ExperimentClient implements Client { return variants; } - public async doFlags(): Promise { + private async doFlags(): Promise { try { const isWebExperiment = this.config?.['internalInstanceNameSuffix'] === 'web'; const user = this.addContext(this.getUser()); - const userAndDeviceId: ExperimentUser = { - user_id: user?.user_id, - device_id: user?.device_id, - }; - const flags = await this.flagApi.getFlags( - { - libraryName: 'experiment-js-client', - libraryVersion: PACKAGE_VERSION, - timeoutMillis: this.config.fetchTimeoutMillis, - }, - isWebExperiment ? userAndDeviceId : undefined, - isWebExperiment ? 'web' : undefined, - ); + const flags = await this.flagApi.getFlags({ + libraryName: 'experiment-js-client', + libraryVersion: PACKAGE_VERSION, + timeoutMillis: this.config.fetchTimeoutMillis, + deliveryMethod: isWebExperiment ? 'web' : undefined, + user: + isWebExperiment && (user?.user_id || user?.device_id) + ? { user_id: user?.user_id, device_id: user?.device_id } + : undefined, + }); this.flags.clear(); this.flags.putAll(flags); } catch (e) { diff --git a/packages/experiment-core/src/api/flag-api.ts b/packages/experiment-core/src/api/flag-api.ts index 9cc00698..49392ba3 100644 --- a/packages/experiment-core/src/api/flag-api.ts +++ b/packages/experiment-core/src/api/flag-api.ts @@ -8,14 +8,12 @@ export type GetFlagsOptions = { libraryVersion: string; evaluationMode?: string; timeoutMillis?: number; + user?: Record; + deliveryMethod?: string | undefined; }; export interface FlagApi { - getFlags( - options?: GetFlagsOptions, - user?: Record, - deliveryMethod?: string | undefined, - ): Promise>; + getFlags(options?: GetFlagsOptions): Promise>; } export class SdkFlagApi implements FlagApi { @@ -35,8 +33,6 @@ export class SdkFlagApi implements FlagApi { public async getFlags( options?: GetFlagsOptions, - user?: Record, - deliveryMethod?: string | undefined, ): Promise> { const headers: Record = { Authorization: `Api-Key ${this.deploymentKey}`, @@ -46,13 +42,17 @@ export class SdkFlagApi implements FlagApi { 'X-Amp-Exp-Library' ] = `${options.libraryName}/${options.libraryVersion}`; } - if (user) { - headers['X-Amp-Exp-User'] = Base64.encodeURL(JSON.stringify(user)); + if (options?.user) { + headers['X-Amp-Exp-User'] = Base64.encodeURL( + JSON.stringify(options.user), + ); } const response = await this.httpClient.request({ requestUrl: `${this.serverUrl}/sdk/v2/flags` + - (deliveryMethod ? `?delivery_method=${deliveryMethod}` : ''), + (options?.deliveryMethod + ? `?delivery_method=${options.deliveryMethod}` + : ''), method: 'GET', headers: headers, timeoutMillis: options?.timeoutMillis, diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index d4a79e28..9a9c20c4 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -85,7 +85,7 @@ export const initializeExperiment = async ( let isRemoteBlocking = false; const remoteFlagKeys: Set = new Set(); - const localFlagKeys: Set = new Set(); + const locaFlagKeys: Set = new Set(); const parsedFlags = JSON.parse(initialFlags); parsedFlags.forEach((flag: EvaluationFlag) => { @@ -122,7 +122,6 @@ export const initializeExperiment = async ( } } - // parse through remote flags if (flag?.metadata?.evaluationMode !== 'local') { remoteFlagKeys.add(flag.key); // check whether any remote flags are blocking @@ -142,7 +141,7 @@ export const initializeExperiment = async ( } } } else { - localFlagKeys.add(flag.key); + locaFlagKeys.add(flag.key); } }); initialFlags = JSON.stringify(parsedFlags); @@ -172,7 +171,7 @@ export const initializeExperiment = async ( setUrlChangeListener(); // apply local variants - applyVariants(globalScope.webExperiment.all(), localFlagKeys); + applyVariants(globalScope.webExperiment.all(), locaFlagKeys); if (!isRemoteBlocking) { // Remove anti-flicker css if remote flags are not blocking @@ -223,7 +222,7 @@ const applyVariants = ( const isControlPayload = !variant.payload || (payloadIsArray && variant.payload.length === 0); if (shouldTrackExposure && isControlPayload) { - globalScope.webExperiment.exposure(key); + exposureWithDedupe(key, variant); continue; } From ac60b534c57135ed6de40f506daa5486269f181f Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Wed, 27 Nov 2024 13:47:21 -0800 Subject: [PATCH 18/22] fix: test --- packages/experiment-tag/test/experiment.test.ts | 7 ++++--- 1 file changed, 4 insertions(+), 3 deletions(-) diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index 82946dab..915be47e 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -461,10 +461,11 @@ describe('initializeExperiment', () => { createMutateFlag('test', 'treatment', [], [], [], 'remote'), ]; const remoteFlags = [createMutateFlag('test', 'treatment')]; - const mockHttpClient = new MockHttpClient(JSON.stringify(remoteFlags), 200); - - const doFlagsMock = jest.spyOn(ExperimentClient.prototype, 'doFlags'); + const doFlagsMock = jest.spyOn( + ExperimentClient.prototype as any, + 'doFlags', + ); initializeExperiment(stringify(apiKey), JSON.stringify(initialFlags), { httpClient: mockHttpClient, }).then(() => { From e8b064fd53bde30986e5c745a682df96c5094a28 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Mon, 16 Dec 2024 15:56:00 -0800 Subject: [PATCH 19/22] refactor and add comment for setting IDs --- packages/experiment-tag/src/experiment.ts | 39 +++++++++++++++-------- 1 file changed, 25 insertions(+), 14 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 9a9c20c4..2532ce44 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -63,6 +63,7 @@ export const initializeExperiment = async ( user.web_exp_id = user.device_id; } else { const uuid = UUID(); + // both IDs are set for backwards compatibility, to be removed in future update user = { device_id: uuid, web_exp_id: uuid }; } globalScope.localStorage.setItem( @@ -125,20 +126,10 @@ export const initializeExperiment = async ( if (flag?.metadata?.evaluationMode !== 'local') { remoteFlagKeys.add(flag.key); // check whether any remote flags are blocking - if (!isRemoteBlocking && flag.metadata?.isBlocking) { + if (!isRemoteBlocking && flag.metadata?.blockingEvaluation) { isRemoteBlocking = true; // Apply anti-flicker css if any remote flags are blocking - if (!globalScope.document.getElementById('amp-exp-css')) { - const id = 'amp-exp-css'; - const s = document.createElement('style'); - s.id = id; - s.innerText = - '* { visibility: hidden !important; background-image: none !important; }'; - document.head.appendChild(s); - globalScope.window.setTimeout(function () { - s.remove(); - }, 1000); - } + applyAntiFlickerCss(); } } else { locaFlagKeys.add(flag.key); @@ -152,6 +143,10 @@ export const initializeExperiment = async ( // @ts-ignore internalInstanceNameSuffix: 'web', initialFlags: initialFlags, + // timeout for fetching remote flags + fetchTimeoutMillis: 5000, + pollOnStart: false, + fetchOnStart: false, ...config, }); @@ -184,11 +179,11 @@ export const initializeExperiment = async ( try { await globalScope.webExperiment.doFlags(); - // apply remote variants - applyVariants(globalScope.webExperiment.all(), remoteFlagKeys); } catch (error) { console.warn('Error fetching remote flags:', error); } + // apply remote variants, if fetch is unsuccessful, use localStorage flags + applyVariants(globalScope.webExperiment.all(), remoteFlagKeys); }; const applyVariants = ( @@ -430,3 +425,19 @@ const exposureWithDedupe = (key: string, variant: Variant) => { urlExposureCache[currentUrl][key] = variant.key; } }; + +const applyAntiFlickerCss = () => { + const globalScope = getGlobalScope(); + if (!globalScope) return; + if (!globalScope.document.getElementById('amp-exp-css')) { + const id = 'amp-exp-css'; + const s = document.createElement('style'); + s.id = id; + s.innerText = + '* { visibility: hidden !important; background-image: none !important; }'; + document.head.appendChild(s); + globalScope.window.setTimeout(function () { + s.remove(); + }, 1000); + } +}; From c8bd924e5064774a922f40e6fa69a139b0311614 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Wed, 18 Dec 2024 10:40:33 -0800 Subject: [PATCH 20/22] make all remote flags locally evaluable, only applyVariants present in initialFlags --- packages/experiment-tag/src/experiment.ts | 69 ++++++++++--------- .../experiment-tag/test/experiment.test.ts | 17 ++--- .../experiment-tag/test/util/create-flag.ts | 4 +- 3 files changed, 47 insertions(+), 43 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 2532ce44..bf346d06 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -86,55 +86,62 @@ export const initializeExperiment = async ( let isRemoteBlocking = false; const remoteFlagKeys: Set = new Set(); - const locaFlagKeys: Set = new Set(); + const localFlagKeys: Set = new Set(); const parsedFlags = JSON.parse(initialFlags); parsedFlags.forEach((flag: EvaluationFlag) => { - // force variant if in preview mode + const { key, variants, segments, metadata = {} } = flag; + + // Force variant if in preview mode if ( urlParams['PREVIEW'] && - flag.key in urlParams && - urlParams[flag.key] in flag.variants + key in urlParams && + urlParams[key] in variants ) { - // Strip the preview query param + // Remove preview-related query parameters from the URL globalScope.history.replaceState( {}, '', - removeQueryParams(globalScope.location.href, ['PREVIEW', flag.key]), + removeQueryParams(globalScope.location.href, ['PREVIEW', key]), ); - // Keep page targeting segments - const pageTargetingSegments = flag.segments.filter((segment) => - isPageTargetingSegment(segment), - ); + // Retain only page-targeting segments + const pageTargetingSegments = segments.filter(isPageTargetingSegment); - // Create or update the preview segment + // Add or update the preview segment const previewSegment = { metadata: { segmentName: 'preview' }, - variant: urlParams[flag.key], + variant: urlParams[key], }; + // Update the flag's segments to include the preview segment flag.segments = [...pageTargetingSegments, previewSegment]; - if (flag?.metadata?.evaluationMode !== 'local') { - // make the remote flag locally evaluable - flag.metadata = flag.metadata || {}; - flag.metadata.evaluationMode = 'local'; - } + // make all preview flags local + metadata.evaluationMode = 'local'; } - if (flag?.metadata?.evaluationMode !== 'local') { - remoteFlagKeys.add(flag.key); - // check whether any remote flags are blocking - if (!isRemoteBlocking && flag.metadata?.blockingEvaluation) { + if (metadata.evaluationMode !== 'local') { + remoteFlagKeys.add(key); + + // allow local evaluation for remote flags + metadata.evaluationMode = 'local'; + + // Check if any remote flags are blocking + if (!isRemoteBlocking && metadata.blockingEvaluation) { isRemoteBlocking = true; - // Apply anti-flicker css if any remote flags are blocking + + // Apply anti-flicker CSS to prevent UI flicker applyAntiFlickerCss(); } } else { - locaFlagKeys.add(flag.key); + // Add locally evaluable flags to the local flag set + localFlagKeys.add(key); } + + flag.metadata = metadata; }); + initialFlags = JSON.stringify(parsedFlags); // initialize the experiment @@ -144,7 +151,7 @@ export const initializeExperiment = async ( internalInstanceNameSuffix: 'web', initialFlags: initialFlags, // timeout for fetching remote flags - fetchTimeoutMillis: 5000, + fetchTimeoutMillis: 1000, pollOnStart: false, fetchOnStart: false, ...config, @@ -163,10 +170,10 @@ export const initializeExperiment = async ( globalScope.webExperiment.addPlugin(globalScope.experimentIntegration); globalScope.webExperiment.setUser(user); - setUrlChangeListener(); + setUrlChangeListener(new Set([...localFlagKeys, ...remoteFlagKeys])); // apply local variants - applyVariants(globalScope.webExperiment.all(), locaFlagKeys); + applyVariants(globalScope.webExperiment.all(), localFlagKeys); if (!isRemoteBlocking) { // Remove anti-flicker css if remote flags are not blocking @@ -182,7 +189,7 @@ export const initializeExperiment = async ( } catch (error) { console.warn('Error fetching remote flags:', error); } - // apply remote variants, if fetch is unsuccessful, use localStorage flags + // apply remote variants - if fetch is unsuccessful, fallback order: 1. localStorage flags, 2. initial flags applyVariants(globalScope.webExperiment.all(), remoteFlagKeys); }; @@ -357,7 +364,7 @@ const handleInject = (action, key: string, variant: Variant) => { exposureWithDedupe(key, variant); }; -export const setUrlChangeListener = () => { +export const setUrlChangeListener = (flagKeys: Set) => { const globalScope = getGlobalScope(); if (!globalScope) { return; @@ -365,7 +372,7 @@ export const setUrlChangeListener = () => { // Add URL change listener for back/forward navigation globalScope.addEventListener('popstate', () => { revertMutations(); - applyVariants(globalScope.webExperiment.all()); + applyVariants(globalScope.webExperiment.all(), flagKeys); }); // Create wrapper functions for pushState and replaceState @@ -379,7 +386,7 @@ export const setUrlChangeListener = () => { const result = originalPushState.apply(this, args); // Revert mutations and apply variants revertMutations(); - applyVariants(globalScope.webExperiment.all()); + applyVariants(globalScope.webExperiment.all(), flagKeys); previousUrl = globalScope.location.href; return result; }; @@ -390,7 +397,7 @@ export const setUrlChangeListener = () => { const result = originalReplaceState.apply(this, args); // Revert mutations and apply variants revertMutations(); - applyVariants(globalScope.webExperiment.all()); + applyVariants(globalScope.webExperiment.all(), flagKeys); previousUrl = globalScope.location.href; return result; }; diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index 915be47e..d844964b 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -398,7 +398,7 @@ describe('initializeExperiment', () => { expect(mockExposure).toHaveBeenCalledWith('test-1'); }); - test('remote evaluation - fetch fail, local evaluation success', () => { + test('remote evaluation - fetch fail, locally evaluate remote and local flags success', () => { const initialFlags = [ // remote flag createMutateFlag('test-2', 'treatment', [], [], [], 'remote'), @@ -413,27 +413,27 @@ describe('initializeExperiment', () => { httpClient: mockHttpClient, }).then(() => { // check remote fetch failed safely - expect(mockExposure).toHaveBeenCalledTimes(1); + expect(mockExposure).toHaveBeenCalledTimes(2); }); // check local flag variant actions called expect(mockExposure).toHaveBeenCalledTimes(1); expect(mockExposure).toHaveBeenCalledWith('test-1'); }); - test('remote evaluation - fetch fail, test no variant actions called', () => { + test('remote evaluation - fetch fail, test initialFlags variant actions called', () => { const initialFlags = [ // remote flag createMutateFlag('test', 'treatment', [], [], [], 'remote'), ]; - const remoteFlags = [createMutateFlag('test', 'treatment')]; - const mockHttpClient = new MockHttpClient(JSON.stringify(remoteFlags), 404); + const mockHttpClient = new MockHttpClient('', 404); initializeExperiment(stringify(apiKey), JSON.stringify(initialFlags), { httpClient: mockHttpClient, }).then(() => { - // check remote fetch failed safely - expect(mockExposure).toHaveBeenCalledTimes(0); + // check remote variant actions applied + expect(mockExposure).toHaveBeenCalledTimes(1); + expect(mockExposure).toHaveBeenCalledWith('test'); }); // check local flag variant actions called expect(mockExposure).toHaveBeenCalledTimes(0); @@ -472,9 +472,6 @@ describe('initializeExperiment', () => { // check remote fetch not called expect(doFlagsMock).toHaveBeenCalledTimes(0); }); - // check local flag variant actions called - expect(mockExposure).toHaveBeenCalledTimes(1); - expect(mockExposure).toHaveBeenCalledWith('test'); }); test('remote evaluation - fetch successful, fetched flag overwrites initial flag', async () => { diff --git a/packages/experiment-tag/test/util/create-flag.ts b/packages/experiment-tag/test/util/create-flag.ts index 9dd0ad1e..ea782ca8 100644 --- a/packages/experiment-tag/test/util/create-flag.ts +++ b/packages/experiment-tag/test/util/create-flag.ts @@ -1,6 +1,6 @@ export const createRedirectFlag = ( flagKey = 'test', - variant: string, + variant: 'treatment' | 'control' | 'off', treatmentUrl: string, controlUrl: string | undefined = undefined, segments: any[] = [], @@ -63,7 +63,7 @@ export const createRedirectFlag = ( export const createMutateFlag = ( flagKey = 'test', - variant: string, + variant: 'treatment' | 'control' | 'off', treatmentMutations: any[] = [], controlMutations: any[] = [], segments: any[] = [], From 22317920d1a5734029ebdc0dd2056ec1303aaa2a Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Fri, 3 Jan 2025 11:25:38 -0800 Subject: [PATCH 21/22] add server zone config --- packages/experiment-tag/src/script.ts | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/packages/experiment-tag/src/script.ts b/packages/experiment-tag/src/script.ts index 68adbad7..321859c5 100644 --- a/packages/experiment-tag/src/script.ts +++ b/packages/experiment-tag/src/script.ts @@ -2,7 +2,11 @@ import { initializeExperiment } from './experiment'; const API_KEY = '{{DEPLOYMENT_KEY}}'; const initialFlags = '{{INITIAL_FLAGS}}'; -initializeExperiment(API_KEY, initialFlags).then(() => { - // Remove anti-flicker css if it exists - document.getElementById('amp-exp-css')?.remove(); -}); +const serverZone = '{{SERVER_ZONE}}'; + +initializeExperiment(API_KEY, initialFlags, { serverZone: serverZone }).then( + () => { + // Remove anti-flicker css if it exists + document.getElementById('amp-exp-css')?.remove(); + }, +); From 6d07684eca65b808d18642d1555b2d027a973d85 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Mon, 6 Jan 2025 14:50:07 -0800 Subject: [PATCH 22/22] update backwards compatibility --- packages/experiment-tag/src/experiment.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 52254876..dcc3e370 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -65,6 +65,8 @@ export const initializeExperiment = async ( // if user has device_id, migrate it to web_exp_id if (user.device_id) { user.web_exp_id = user.device_id; + } else if (user.web_exp_id) { + user.device_id = user.web_exp_id; } else { const uuid = UUID(); // both IDs are set for backwards compatibility, to be removed in future update