From 34c2cb6a663af546365eef8ec01b7a8cb4c3d32d Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Tue, 5 Nov 2024 11:51:04 -0800 Subject: [PATCH 01/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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/33] 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 b83f13137fc43bfb8b6dd8de522ced7ebee93203 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Fri, 27 Dec 2024 13:16:49 -0800 Subject: [PATCH 21/33] feat: modularized WebExperiment class --- packages/experiment-tag/src/experiment.ts | 768 +++++++++++----------- packages/experiment-tag/src/script.ts | 5 +- 2 files changed, 399 insertions(+), 374 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index bf346d06..f5e459ac 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -11,6 +11,7 @@ import { Variants, AmplitudeIntegrationPlugin, ExperimentConfig, + ExperimentClient, } from '@amplitude/experiment-js-client'; import mutate, { MutationController } from 'dom-mutator'; @@ -24,427 +25,450 @@ import { concatenateQueryParamsOf, } from './util'; -const appliedInjections: Set = new Set(); -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 } }; - -export const initializeExperiment = async ( - apiKey: string, - initialFlags: string, - config: ExperimentConfig = {}, -) => { - const globalScope = getGlobalScope(); - if (globalScope?.webExperiment) { - return; - } - WindowMessenger.setup(); - if (!isLocalStorageAvailable() || !globalScope) { - return; - } - previousUrl = undefined; - urlExposureCache = {}; - const experimentStorageName = `EXP_${apiKey.slice(0, 10)}`; - let user; - try { - user = JSON.parse( - globalScope.localStorage.getItem(experimentStorageName) || '{}', - ); - } catch (error) { - user = {}; +export class WebExperiment { + private readonly apiKey: string; + private initialFlags: string; + private readonly config: ExperimentConfig; + private readonly globalScope = getGlobalScope(); + private appliedInjections: Set = new Set(); + private appliedMutations: MutationController[] = []; + private previousUrl: string | undefined; + // Cache to track exposure for the current URL, should be cleared on URL change + private urlExposureCache: { + [url: string]: { [key: string]: string | undefined }; + } = {}; + private experimentClient: ExperimentClient | undefined; + + constructor( + apiKey: string, + initialFlags: string, + config: ExperimentConfig = {}, + ) { + this.apiKey = apiKey; + this.initialFlags = initialFlags; + this.config = config; } - // 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) { - 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( - experimentStorageName, - JSON.stringify(user), + public async initializeExperiment() { + if (this.globalScope?.webExperiment) { + return; + } + + WindowMessenger.setup(); + if (!isLocalStorageAvailable() || !this.globalScope) { + return; + } + + this.globalScope.webExperiment = this; + this.previousUrl = undefined; + this.urlExposureCache = {}; + const experimentStorageName = `EXP_${this.apiKey.slice(0, 10)}`; + let user; + try { + user = JSON.parse( + this.globalScope.localStorage.getItem(experimentStorageName) || '{}', ); + } catch (error) { + user = {}; } - } - const urlParams = getUrlParams(); - // if in visual edit mode, remove the query param - if (urlParams['VISUAL_EDITOR']) { - globalScope.history.replaceState( - {}, - '', - removeQueryParams(globalScope.location.href, ['VISUAL_EDITOR']), - ); - return; - } + // 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) { + 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 }; + } + this.globalScope.localStorage.setItem( + experimentStorageName, + JSON.stringify(user), + ); + } + } - let isRemoteBlocking = false; - const remoteFlagKeys: Set = new Set(); - const localFlagKeys: Set = new Set(); - const parsedFlags = JSON.parse(initialFlags); - - parsedFlags.forEach((flag: EvaluationFlag) => { - const { key, variants, segments, metadata = {} } = flag; - - // Force variant if in preview mode - if ( - urlParams['PREVIEW'] && - key in urlParams && - urlParams[key] in variants - ) { - // Remove preview-related query parameters from the URL - globalScope.history.replaceState( + const urlParams = getUrlParams(); + // if in visual edit mode, remove the query param + if (urlParams['VISUAL_EDITOR']) { + this.globalScope.history.replaceState( {}, '', - removeQueryParams(globalScope.location.href, ['PREVIEW', key]), + removeQueryParams(this.globalScope.location.href, ['VISUAL_EDITOR']), ); - - // Retain only page-targeting segments - const pageTargetingSegments = segments.filter(isPageTargetingSegment); - - // Add or update the preview segment - const previewSegment = { - metadata: { segmentName: 'preview' }, - variant: urlParams[key], - }; - - // Update the flag's segments to include the preview segment - flag.segments = [...pageTargetingSegments, previewSegment]; - - // make all preview flags local - metadata.evaluationMode = 'local'; + return; } - if (metadata.evaluationMode !== 'local') { - remoteFlagKeys.add(key); + let isRemoteBlocking = false; + const remoteFlagKeys: Set = new Set(); + const localFlagKeys: Set = new Set(); + const parsedFlags = JSON.parse(this.initialFlags); + + parsedFlags.forEach((flag: EvaluationFlag) => { + const { key, variants, segments, metadata = {} } = flag; + + // Force variant if in preview mode + if ( + urlParams['PREVIEW'] && + key in urlParams && + urlParams[key] in variants + ) { + // Remove preview-related query parameters from the URL + this.globalScope?.history.replaceState( + {}, + '', + removeQueryParams(this.globalScope.location.href, ['PREVIEW', key]), + ); + + // Retain only page-targeting segments + const pageTargetingSegments = segments.filter( + this.isPageTargetingSegment, + ); + + // Add or update the preview segment + const previewSegment = { + metadata: { segmentName: 'preview' }, + variant: urlParams[key], + }; + + // Update the flag's segments to include the preview segment + flag.segments = [...pageTargetingSegments, previewSegment]; + + // make all preview flags local + metadata.evaluationMode = 'local'; + } - // allow local evaluation for remote flags - metadata.evaluationMode = 'local'; + if (metadata.evaluationMode !== 'local') { + remoteFlagKeys.add(key); - // Check if any remote flags are blocking - if (!isRemoteBlocking && metadata.blockingEvaluation) { - isRemoteBlocking = true; + // allow local evaluation for remote flags + metadata.evaluationMode = 'local'; - // Apply anti-flicker CSS to prevent UI flicker - applyAntiFlickerCss(); + // Check if any remote flags are blocking + if (!isRemoteBlocking && metadata.blockingEvaluation) { + isRemoteBlocking = true; + + // Apply anti-flicker CSS to prevent UI flicker + this.applyAntiFlickerCss(); + } + } else { + // Add locally evaluable flags to the local flag set + localFlagKeys.add(key); } - } else { - // Add locally evaluable flags to the local flag set - localFlagKeys.add(key); + + flag.metadata = metadata; + }); + + this.initialFlags = JSON.stringify(parsedFlags); + + // initialize the experiment + this.experimentClient = Experiment.initialize(this.apiKey, { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + internalInstanceNameSuffix: 'web', + initialFlags: this.initialFlags, + // timeout for fetching remote flags + fetchTimeoutMillis: 1000, + pollOnStart: false, + fetchOnStart: false, + ...this.config, + }); + + // If no integration has been set, use an amplitude integration. + if (!this.globalScope.experimentIntegration) { + const connector = AnalyticsConnector.getInstance('$default_instance'); + this.globalScope.experimentIntegration = new AmplitudeIntegrationPlugin( + this.apiKey, + connector, + 0, + ); } + this.globalScope.experimentIntegration.type = 'integration'; + this.experimentClient.addPlugin(this.globalScope.experimentIntegration); + this.experimentClient.setUser(user); - flag.metadata = metadata; - }); - - initialFlags = JSON.stringify(parsedFlags); - - // initialize the experiment - globalScope.webExperiment = Experiment.initialize(apiKey, { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - internalInstanceNameSuffix: 'web', - initialFlags: initialFlags, - // timeout for fetching remote flags - fetchTimeoutMillis: 1000, - pollOnStart: false, - fetchOnStart: false, - ...config, - }); - - // If no integration has been set, use an amplitude integration. - if (!globalScope.experimentIntegration) { - const connector = AnalyticsConnector.getInstance('$default_instance'); - globalScope.experimentIntegration = new AmplitudeIntegrationPlugin( - apiKey, - connector, - 0, - ); - } - globalScope.experimentIntegration.type = 'integration'; - globalScope.webExperiment.addPlugin(globalScope.experimentIntegration); - globalScope.webExperiment.setUser(user); + this.setUrlChangeListener(new Set([...localFlagKeys, ...remoteFlagKeys])); - setUrlChangeListener(new Set([...localFlagKeys, ...remoteFlagKeys])); + // apply local variants + this.applyVariants(this.experimentClient.all(), localFlagKeys); - // apply local variants - applyVariants(globalScope.webExperiment.all(), localFlagKeys); + if (!isRemoteBlocking) { + // Remove anti-flicker css if remote flags are not blocking + this.globalScope.document.getElementById?.('amp-exp-css')?.remove(); + } - if (!isRemoteBlocking) { - // Remove anti-flicker css if remote flags are not blocking - globalScope.document.getElementById?.('amp-exp-css')?.remove(); - } + if (remoteFlagKeys.size === 0) { + return; + } - if (remoteFlagKeys.size === 0) { - return; + try { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + await this.experimentClient.doFlags(); + } catch (error) { + console.warn('Error fetching remote flags:', error); + } + // apply remote variants - if fetch is unsuccessful, fallback order: 1. localStorage flags, 2. initial flags + this.applyVariants(this.experimentClient.all(), remoteFlagKeys); } - try { - await globalScope.webExperiment.doFlags(); - } catch (error) { - console.warn('Error fetching remote flags:', error); - } - // apply remote variants - if fetch is unsuccessful, fallback order: 1. localStorage flags, 2. initial flags - applyVariants(globalScope.webExperiment.all(), remoteFlagKeys); -}; - -const applyVariants = ( - variants: Variants, - flagKeys: Set | undefined = undefined, -) => { - if (Object.keys(variants).length === 0) { - return; - } - const globalScope = getGlobalScope(); - if (!globalScope) { - return; - } - const currentUrl = urlWithoutParamsAndAnchor(globalScope.location.href); - // Initialize the cache if on a new URL - if (!urlExposureCache?.[currentUrl]) { - urlExposureCache = {}; - urlExposureCache[currentUrl] = {}; - } - for (const key in variants) { - if (flagKeys && !flagKeys.has(key)) { - continue; + public applyVariants( + variants: Variants, + flagKeys: Set | undefined = undefined, + ) { + if (Object.keys(variants).length === 0) { + return; } - const variant = variants[key]; - const isWebExperimentation = variant.metadata?.deliveryMethod === 'web'; - if (isWebExperimentation) { - const shouldTrackExposure = - (variant.metadata?.['trackExposure'] as boolean) ?? true; - // if payload is falsy or empty array, consider it as control variant - const payloadIsArray = Array.isArray(variant.payload); - const isControlPayload = - !variant.payload || (payloadIsArray && variant.payload.length === 0); - if (shouldTrackExposure && isControlPayload) { - exposureWithDedupe(key, variant); + const globalScope = getGlobalScope(); + if (!globalScope) { + return; + } + const currentUrl = urlWithoutParamsAndAnchor(globalScope.location.href); + // Initialize the cache if on a new URL + if (!this.urlExposureCache?.[currentUrl]) { + this.urlExposureCache = {}; + this.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) { + const shouldTrackExposure = + (variant.metadata?.['trackExposure'] as boolean) ?? true; + // if payload is falsy or empty array, consider it as control variant + const payloadIsArray = Array.isArray(variant.payload); + const isControlPayload = + !variant.payload || (payloadIsArray && variant.payload.length === 0); + if (shouldTrackExposure && isControlPayload) { + this.exposureWithDedupe(key, variant); + continue; + } - if (payloadIsArray) { - for (const action of variant.payload) { - if (action.action === 'redirect') { - handleRedirect(action, key, variant); - } else if (action.action === 'mutate') { - handleMutate(action, key, variant); - } else if (action.action === 'inject') { - handleInject(action, key, variant); + if (payloadIsArray) { + for (const action of variant.payload) { + if (action.action === 'redirect') { + this.handleRedirect(action, key, variant); + } else if (action.action === 'mutate') { + this.handleMutate(action, key, variant); + } else if (action.action === 'inject') { + this.handleInject(action, key, variant); + } } } } } } -}; - -const handleRedirect = (action, key: string, variant: Variant) => { - const globalScope = getGlobalScope(); - if (!globalScope) { - return; - } - const referrerUrl = urlWithoutParamsAndAnchor( - previousUrl || globalScope.document.referrer, - ); - const redirectUrl = action?.data?.url; - const currentUrl = urlWithoutParamsAndAnchor(globalScope.location.href); + private handleRedirect(action, key: string, variant: Variant) { + const globalScope = getGlobalScope(); + if (!globalScope) { + return; + } + const referrerUrl = urlWithoutParamsAndAnchor( + this.previousUrl || globalScope.document.referrer, + ); + const redirectUrl = action?.data?.url; - // prevent infinite redirection loop - if (currentUrl === referrerUrl) { - return; - } + const currentUrl = urlWithoutParamsAndAnchor(globalScope.location.href); - const targetUrl = concatenateQueryParamsOf( - globalScope.location.href, - redirectUrl, - ); + // prevent infinite redirection loop + if (currentUrl === referrerUrl) { + return; + } - exposureWithDedupe(key, variant); + const targetUrl = concatenateQueryParamsOf( + globalScope.location.href, + redirectUrl, + ); - // set previous url - relevant for SPA if redirect happens before push/replaceState is complete - previousUrl = globalScope.location.href; - // perform redirection - globalScope.location.replace(targetUrl); -}; + this.exposureWithDedupe(key, variant); -const handleMutate = (action, key: string, variant: Variant) => { - const globalScope = getGlobalScope(); - if (!globalScope) { - return; - } - const mutations = action.data?.mutations; - mutations.forEach((m) => { - appliedMutations.push(mutate.declarative(m)); - }); - exposureWithDedupe(key, variant); -}; - -const revertMutations = () => { - while (appliedMutations.length > 0) { - appliedMutations.pop()?.revert(); + // set previous url - relevant for SPA if redirect happens before push/replaceState is complete + this.previousUrl = globalScope.location.href; + // perform redirection + globalScope.location.replace(targetUrl); } -}; -const handleInject = (action, key: string, variant: Variant) => { - const globalScope = getGlobalScope(); - if (!globalScope) { - return; - } - // Validate and transform ID - let id = action.data.id; - if (!id || typeof id !== 'string' || id.length === 0) { - return; - } - // Replace the `-` characters in the UUID to support function name - id = id.replace(/-/g, ''); - // Check for repeat invocations - if (appliedInjections.has(id)) { - return; - } - // Create JS - const rawJs = action.data.js; - let script: HTMLScriptElement | undefined; - if (rawJs) { - script = globalScope.document.createElement('script'); - if (script) { - script.innerHTML = `function ${id}(html, utils, id){${rawJs}};`; - script.id = `js-${id}`; - globalScope.document.head.appendChild(script); + private handleMutate(action, key: string, variant: Variant) { + const globalScope = getGlobalScope(); + if (!globalScope) { + return; } + const mutations = action.data?.mutations; + mutations.forEach((m) => { + this.appliedMutations.push(mutate.declarative(m)); + }); + this.exposureWithDedupe(key, variant); } - // Create CSS - const rawCss = action.data.css; - let style: HTMLStyleElement | undefined; - if (rawCss) { - style = globalScope.document.createElement('style'); - if (style) { - style.innerHTML = rawCss; - style.id = `css-${id}`; - globalScope.document.head.appendChild(style); + + public revertMutations() { + while (this.appliedMutations.length > 0) { + this.appliedMutations.pop()?.revert(); } } - // Create HTML - const rawHtml = action.data.html; - let html: Element | undefined; - if (rawHtml) { - html = - new DOMParser().parseFromString(rawHtml, 'text/html').body - .firstElementChild ?? undefined; - } - // Inject - const utils = getInjectUtils(); - appliedInjections.add(id); - try { - const fn = globalScope[id]; - if (fn && typeof fn === 'function') { - fn(html, utils, id); + + private handleInject(action, key: string, variant: Variant) { + const globalScope = getGlobalScope(); + if (!globalScope) { + return; } - } catch (e) { - script?.remove(); - console.error( - `Experiment inject failed for ${key} variant ${variant.key}. Reason:`, - e, - ); - } - // Push mutation to remove CSS and any custom state cleanup set in utils. - appliedMutations.push({ - revert: () => { - if (utils.remove) utils.remove(); - style?.remove(); + // Validate and transform ID + let id = action.data.id; + if (!id || typeof id !== 'string' || id.length === 0) { + return; + } + // Replace the `-` characters in the UUID to support function name + id = id.replace(/-/g, ''); + // Check for repeat invocations + if (this.appliedInjections.has(id)) { + return; + } + // Create JS + const rawJs = action.data.js; + let script: HTMLScriptElement | undefined; + if (rawJs) { + script = globalScope.document.createElement('script'); + if (script) { + script.innerHTML = `function ${id}(html, utils, id){${rawJs}};`; + script.id = `js-${id}`; + globalScope.document.head.appendChild(script); + } + } + // Create CSS + const rawCss = action.data.css; + let style: HTMLStyleElement | undefined; + if (rawCss) { + style = globalScope.document.createElement('style'); + if (style) { + style.innerHTML = rawCss; + style.id = `css-${id}`; + globalScope.document.head.appendChild(style); + } + } + // Create HTML + const rawHtml = action.data.html; + let html: Element | undefined; + if (rawHtml) { + html = + new DOMParser().parseFromString(rawHtml, 'text/html').body + .firstElementChild ?? undefined; + } + // Inject + const utils = getInjectUtils(); + this.appliedInjections.add(id); + try { + const fn = globalScope[id]; + if (fn && typeof fn === 'function') { + fn(html, utils, id); + } + } catch (e) { script?.remove(); - appliedInjections.delete(id); - }, - }); - exposureWithDedupe(key, variant); -}; - -export const setUrlChangeListener = (flagKeys: Set) => { - const globalScope = getGlobalScope(); - if (!globalScope) { - return; + console.error( + `Experiment inject failed for ${key} variant ${variant.key}. Reason:`, + e, + ); + } + // Push mutation to remove CSS and any custom state cleanup set in utils. + this.appliedMutations.push({ + revert: () => { + if (utils.remove) utils.remove(); + style?.remove(); + script?.remove(); + this.appliedInjections.delete(id); + }, + }); + this.exposureWithDedupe(key, variant); } - // Add URL change listener for back/forward navigation - globalScope.addEventListener('popstate', () => { - revertMutations(); - applyVariants(globalScope.webExperiment.all(), flagKeys); - }); - - // Create wrapper functions for pushState and replaceState - const wrapHistoryMethods = () => { - const originalPushState = history.pushState; - const originalReplaceState = history.replaceState; - - // Wrapper for pushState - history.pushState = function (...args) { - // Call the original pushState - const result = originalPushState.apply(this, args); - // Revert mutations and apply variants - revertMutations(); - applyVariants(globalScope.webExperiment.all(), flagKeys); - previousUrl = globalScope.location.href; - return result; + + public setUrlChangeListener(flagKeys: Set) { + const globalScope = getGlobalScope(); + if (!globalScope) { + return; + } + // Add URL change listener for back/forward navigation + globalScope.addEventListener('popstate', () => { + this.revertMutations(); + this.applyVariants(this.experimentClient?.all() || {}, flagKeys); + }); + + const handleUrlChange = () => { + this.revertMutations(); + this.applyVariants(this.experimentClient?.all() || {}, flagKeys); + this.previousUrl = globalScope.location.href; }; - // Wrapper for replaceState - history.replaceState = function (...args) { - // Call the original replaceState - const result = originalReplaceState.apply(this, args); - // Revert mutations and apply variants - revertMutations(); - applyVariants(globalScope.webExperiment.all(), flagKeys); - previousUrl = globalScope.location.href; - return result; + // Create wrapper functions for pushState and replaceState + const wrapHistoryMethods = () => { + const originalPushState = history.pushState; + const originalReplaceState = history.replaceState; + + // Wrapper for pushState + history.pushState = function (...args) { + // Call the original pushState + const result = originalPushState.apply(this, args); + // Revert mutations and apply variants + handleUrlChange(); + return result; + }; + + // Wrapper for replaceState + history.replaceState = function (...args) { + // Call the original replaceState + const result = originalReplaceState.apply(this, args); + // Revert mutations and apply variants + handleUrlChange(); + return result; + }; }; - }; - - // Initialize the wrapper - wrapHistoryMethods(); -}; - -const isPageTargetingSegment = (segment: EvaluationSegment) => { - return ( - segment.metadata?.trackExposure === false && - (segment.metadata?.segmentName === 'Page not targeted' || - segment.metadata?.segmentName === 'Page is excluded') - ); -}; - -const exposureWithDedupe = (key: string, variant: Variant) => { - const globalScope = getGlobalScope(); - if (!globalScope) return; - - const shouldTrackVariant = variant.metadata?.['trackExposure'] ?? true; - const currentUrl = urlWithoutParamsAndAnchor(globalScope.location.href); - - // if on the same base URL, only track exposure if variant has changed or has not been tracked - const hasTrackedVariant = - urlExposureCache?.[currentUrl]?.[key] === variant.key; - const shouldTrackExposure = shouldTrackVariant && !hasTrackedVariant; - - if (shouldTrackExposure) { - globalScope.webExperiment.exposure(key); - urlExposureCache[currentUrl][key] = variant.key; + + // Initialize the wrapper + wrapHistoryMethods(); + } + + private isPageTargetingSegment(segment: EvaluationSegment) { + return ( + segment.metadata?.trackExposure === false && + (segment.metadata?.segmentName === 'Page not targeted' || + segment.metadata?.segmentName === 'Page is excluded') + ); + } + + private exposureWithDedupe(key: string, variant: Variant) { + const globalScope = getGlobalScope(); + if (!globalScope) return; + + const shouldTrackVariant = variant.metadata?.['trackExposure'] ?? true; + const currentUrl = urlWithoutParamsAndAnchor(globalScope.location.href); + + // if on the same base URL, only track exposure if variant has changed or has not been tracked + const hasTrackedVariant = + this.urlExposureCache?.[currentUrl]?.[key] === variant.key; + const shouldTrackExposure = shouldTrackVariant && !hasTrackedVariant; + + if (shouldTrackExposure) { + this.experimentClient?.exposure(key); + this.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); + + private 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); + } } -}; +} diff --git a/packages/experiment-tag/src/script.ts b/packages/experiment-tag/src/script.ts index 68adbad7..e65f30d3 100644 --- a/packages/experiment-tag/src/script.ts +++ b/packages/experiment-tag/src/script.ts @@ -1,8 +1,9 @@ -import { initializeExperiment } from './experiment'; +import { WebExperiment } from './experiment'; const API_KEY = '{{DEPLOYMENT_KEY}}'; const initialFlags = '{{INITIAL_FLAGS}}'; -initializeExperiment(API_KEY, initialFlags).then(() => { +const webExperimentClient = new WebExperiment(API_KEY, initialFlags); +webExperimentClient.initializeExperiment().then(() => { // Remove anti-flicker css if it exists document.getElementById('amp-exp-css')?.remove(); }); From 4ffb6fa509756a5b7030b907c8a9f4048ae7571c Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Fri, 27 Dec 2024 13:17:10 -0800 Subject: [PATCH 22/33] update tests --- .../experiment-tag/test/experiment.test.ts | 108 ++++++++++++------ 1 file changed, 75 insertions(+), 33 deletions(-) diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index d844964b..c780a96d 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -1,8 +1,7 @@ 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 { WebExperiment } from 'src/experiment'; import * as util from 'src/util'; import { stringify } from 'ts-jest'; @@ -23,7 +22,9 @@ describe('initializeExperiment', () => { const mockGetGlobalScope = jest.spyOn(experimentCore, 'getGlobalScope'); jest.spyOn(ExperimentClient.prototype, 'setUser'); jest.spyOn(ExperimentClient.prototype, 'all'); - jest.spyOn(experiment, 'setUrlChangeListener').mockReturnValue(undefined); + jest + .spyOn(WebExperiment.prototype, 'setUrlChangeListener') + .mockReturnValue(undefined); const mockExposure = jest.spyOn(ExperimentClient.prototype, 'exposure'); jest.spyOn(util, 'UUID').mockReturnValue('mock'); let mockGlobal; @@ -51,32 +52,41 @@ describe('initializeExperiment', () => { }); test('should initialize experiment with empty user', () => { - initializeExperiment(stringify(apiKey), JSON.stringify([])); + const webExperiment = new WebExperiment( + stringify(apiKey), + JSON.stringify([]), + ); + webExperiment.initializeExperiment(); expect(ExperimentClient.prototype.setUser).toHaveBeenCalledWith({ device_id: 'mock', web_exp_id: 'mock', }); expect(mockGlobal.localStorage.setItem).toHaveBeenCalledWith( - 'EXP_1', + 'EXP_' + stringify(apiKey), JSON.stringify({ device_id: 'mock', web_exp_id: 'mock' }), ); }); test('experiment should not run without localStorage', () => { + const webExperiment = new WebExperiment( + stringify(apiKey), + JSON.stringify([]), + ); jest .spyOn(experimentCore, 'isLocalStorageAvailable') .mockReturnValue(false); - initializeExperiment(stringify(apiKey), ''); + webExperiment.initializeExperiment(); expect(mockGlobal.localStorage.getItem).toHaveBeenCalledTimes(0); }); test('treatment variant on control page - should redirect and call exposure', () => { - initializeExperiment( + const webExperiment = new WebExperiment( stringify(apiKey), JSON.stringify([ createRedirectFlag('test', 'treatment', 'http://test.com/2'), ]), ); + webExperiment.initializeExperiment(); expect(mockGlobal.location.replace).toHaveBeenCalledWith( 'http://test.com/2', @@ -85,13 +95,13 @@ describe('initializeExperiment', () => { }); test('control variant on control page - should not redirect but call exposure', () => { - initializeExperiment( + const webExperiment = new WebExperiment( stringify(apiKey), JSON.stringify([ createRedirectFlag('test', 'control', 'http://test.com/2'), ]), ); - + webExperiment.initializeExperiment(); expect(mockGlobal.location.replace).toBeCalledTimes(0); expect(mockExposure).toHaveBeenCalledWith('test'); expect(mockGlobal.history.replaceState).toBeCalledTimes(0); @@ -116,13 +126,13 @@ describe('initializeExperiment', () => { // @ts-ignore mockGetGlobalScope.mockReturnValue(mockGlobal); - initializeExperiment( + const webExperiment = new WebExperiment( stringify(apiKey), JSON.stringify([ createRedirectFlag('test', 'treatment', 'http://test.com/2'), ]), ); - + webExperiment.initializeExperiment(); expect(mockGlobal.location.replace).toHaveBeenCalledTimes(0); expect(mockGlobal.history.replaceState).toHaveBeenCalledWith( {}, @@ -150,12 +160,13 @@ describe('initializeExperiment', () => { // @ts-ignore mockGetGlobalScope.mockReturnValue(mockGlobal); - initializeExperiment( + const webExperiment = new WebExperiment( stringify(apiKey), JSON.stringify([ createRedirectFlag('test', 'treatment', 'http://test.com/2'), ]), ); + webExperiment.initializeExperiment(); expect(mockGlobal.location.replace).toHaveBeenCalledWith( 'http://test.com/2', @@ -200,7 +211,7 @@ describe('initializeExperiment', () => { }, ]; - initializeExperiment( + const webExperiment = new WebExperiment( stringify(apiKey), JSON.stringify([ createRedirectFlag( @@ -212,6 +223,7 @@ describe('initializeExperiment', () => { ), ]), ); + webExperiment.initializeExperiment(); expect(mockGlobal.location.replace).toHaveBeenCalledTimes(0); expect(mockExposure).toHaveBeenCalledTimes(0); @@ -240,7 +252,7 @@ describe('initializeExperiment', () => { // @ts-ignore mockGetGlobalScope.mockReturnValue(mockGlobal); - initializeExperiment( + const webExperiment = new WebExperiment( stringify(apiKey), JSON.stringify([ createRedirectFlag( @@ -251,6 +263,7 @@ describe('initializeExperiment', () => { ), ]), ); + webExperiment.initializeExperiment(); expect(mockGlobal.location.replace).toHaveBeenCalledWith( 'http://test.com/2?param3=c¶m1=a¶m2=b', @@ -259,12 +272,13 @@ describe('initializeExperiment', () => { }); test('should behave as control variant when payload is empty', () => { - initializeExperiment( + const webExperiment = new WebExperiment( stringify(apiKey), JSON.stringify([ createRedirectFlag('test', 'control', 'http://test.com/2?param3=c'), ]), ); + webExperiment.initializeExperiment(); expect(mockGlobal.location.replace).not.toHaveBeenCalled(); expect(mockExposure).toHaveBeenCalledWith('test'); @@ -296,7 +310,7 @@ describe('initializeExperiment', () => { variant: 'off', }, ]; - initializeExperiment( + const webExperiment = new WebExperiment( stringify(apiKey), JSON.stringify([ createRedirectFlag( @@ -308,6 +322,7 @@ describe('initializeExperiment', () => { ), ]), ); + webExperiment.initializeExperiment(); expect(mockExposure).toHaveBeenCalledWith('test'); }); @@ -336,7 +351,7 @@ describe('initializeExperiment', () => { variant: 'off', }, ]; - initializeExperiment( + const webExperiment = new WebExperiment( stringify(apiKey), JSON.stringify([ createRedirectFlag( @@ -348,6 +363,7 @@ describe('initializeExperiment', () => { ), ]), ); + webExperiment.initializeExperiment(); expect(mockExposure).not.toHaveBeenCalled(); }); @@ -362,9 +378,14 @@ describe('initializeExperiment', () => { const mockHttpClient = new MockHttpClient(JSON.stringify([])); - initializeExperiment(stringify(apiKey), JSON.stringify(initialFlags), { - httpClient: mockHttpClient, - }).then(() => { + const webExperiment = new WebExperiment( + stringify(apiKey), + JSON.stringify(initialFlags), + { + httpClient: mockHttpClient, + }, + ); + webExperiment.initializeExperiment().then(() => { expect(mockHttpClient.requestUrl).toBe( 'https://flag.lab.amplitude.com/sdk/v2/flags?delivery_method=web', ); @@ -386,9 +407,14 @@ describe('initializeExperiment', () => { const mockHttpClient = new MockHttpClient(JSON.stringify(remoteFlags)); - initializeExperiment(stringify(apiKey), JSON.stringify(initialFlags), { - httpClient: mockHttpClient, - }).then(() => { + const webExperiment = new WebExperiment( + stringify(apiKey), + JSON.stringify(initialFlags), + { + httpClient: mockHttpClient, + }, + ); + webExperiment.initializeExperiment().then(() => { // check remote flag variant actions called after successful fetch expect(mockExposure).toHaveBeenCalledTimes(2); expect(mockExposure).toHaveBeenCalledWith('test-2'); @@ -409,9 +435,14 @@ describe('initializeExperiment', () => { const mockHttpClient = new MockHttpClient(JSON.stringify(remoteFlags), 404); - initializeExperiment(stringify(apiKey), JSON.stringify(initialFlags), { - httpClient: mockHttpClient, - }).then(() => { + const webExperiment = new WebExperiment( + stringify(apiKey), + JSON.stringify(initialFlags), + { + httpClient: mockHttpClient, + }, + ); + webExperiment.initializeExperiment().then(() => { // check remote fetch failed safely expect(mockExposure).toHaveBeenCalledTimes(2); }); @@ -428,9 +459,14 @@ describe('initializeExperiment', () => { const mockHttpClient = new MockHttpClient('', 404); - initializeExperiment(stringify(apiKey), JSON.stringify(initialFlags), { - httpClient: mockHttpClient, - }).then(() => { + const webExperiment = new WebExperiment( + stringify(apiKey), + JSON.stringify(initialFlags), + { + httpClient: mockHttpClient, + }, + ); + webExperiment.initializeExperiment().then(() => { // check remote variant actions applied expect(mockExposure).toHaveBeenCalledTimes(1); expect(mockExposure).toHaveBeenCalledWith('test'); @@ -466,9 +502,14 @@ describe('initializeExperiment', () => { ExperimentClient.prototype as any, 'doFlags', ); - initializeExperiment(stringify(apiKey), JSON.stringify(initialFlags), { - httpClient: mockHttpClient, - }).then(() => { + const webExperiment = new WebExperiment( + stringify(apiKey), + JSON.stringify(initialFlags), + { + httpClient: mockHttpClient, + }, + ); + webExperiment.initializeExperiment().then(() => { // check remote fetch not called expect(doFlagsMock).toHaveBeenCalledTimes(0); }); @@ -491,13 +532,14 @@ describe('initializeExperiment', () => { ]; const mockHttpClient = new MockHttpClient(JSON.stringify(remoteFlags), 200); - await initializeExperiment( + const webExperiment = new WebExperiment( stringify(apiKey), JSON.stringify(initialFlags), { httpClient: mockHttpClient, }, ); + await webExperiment.initializeExperiment(); // check treatment variant called expect(mockExposure).toHaveBeenCalledTimes(1); expect(mockExposure).toHaveBeenCalledWith('test'); From ba6e8b8d37704ebe76a6bc2bc18f1449af6f04ae Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Mon, 30 Dec 2024 13:48:47 -0800 Subject: [PATCH 23/33] Add additional methods --- packages/experiment-tag/src/experiment.ts | 286 +++++++++++++++------- packages/experiment-tag/src/util.ts | 27 +- 2 files changed, 221 insertions(+), 92 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index f5e459ac..ec5bdd99 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -8,7 +8,6 @@ import { import { Experiment, Variant, - Variants, AmplitudeIntegrationPlugin, ExperimentConfig, ExperimentClient, @@ -18,6 +17,7 @@ import mutate, { MutationController } from 'dom-mutator'; import { getInjectUtils } from './inject-utils'; import { WindowMessenger } from './messenger'; import { + convertEvaluationVariantToVariant, getUrlParams, removeQueryParams, urlWithoutParamsAndAnchor, @@ -25,19 +25,22 @@ import { concatenateQueryParamsOf, } from './util'; +const PAGE_NOT_TARGETED = 'Page not targeted'; +const PAGE_IS_EXCLUDED = 'Page is excluded'; + export class WebExperiment { private readonly apiKey: string; - private initialFlags: string; + private readonly initialFlags: []; private readonly config: ExperimentConfig; private readonly globalScope = getGlobalScope(); + private experimentClient: ExperimentClient | undefined; private appliedInjections: Set = new Set(); - private appliedMutations: MutationController[] = []; + private appliedMutations: Record = {}; private previousUrl: string | undefined; // Cache to track exposure for the current URL, should be cleared on URL change - private urlExposureCache: { - [url: string]: { [key: string]: string | undefined }; - } = {}; - private experimentClient: ExperimentClient | undefined; + private urlExposureCache: Record> = + {}; + private flagVariantMap: Record> = {}; constructor( apiKey: string, @@ -45,8 +48,16 @@ export class WebExperiment { config: ExperimentConfig = {}, ) { this.apiKey = apiKey; - this.initialFlags = initialFlags; this.config = config; + this.initialFlags = JSON.parse(initialFlags); + this.initialFlags.forEach((flag: EvaluationFlag) => { + const variants = flag.variants; + this.flagVariantMap[flag.key] = {}; + Object.keys(variants).forEach((variantKey) => { + this.flagVariantMap[flag.key][variantKey] = + convertEvaluationVariantToVariant(flag.variants[variantKey]); + }); + }); } public async initializeExperiment() { @@ -102,11 +113,10 @@ export class WebExperiment { } let isRemoteBlocking = false; - const remoteFlagKeys: Set = new Set(); - const localFlagKeys: Set = new Set(); - const parsedFlags = JSON.parse(this.initialFlags); + const remoteFlagKeys: string[] = []; + const localFlagKeys: string[] = []; - parsedFlags.forEach((flag: EvaluationFlag) => { + this.initialFlags.forEach((flag: EvaluationFlag) => { const { key, variants, segments, metadata = {} } = flag; // Force variant if in preview mode @@ -141,7 +151,7 @@ export class WebExperiment { } if (metadata.evaluationMode !== 'local') { - remoteFlagKeys.add(key); + remoteFlagKeys.push(key); // allow local evaluation for remote flags metadata.evaluationMode = 'local'; @@ -155,20 +165,20 @@ export class WebExperiment { } } else { // Add locally evaluable flags to the local flag set - localFlagKeys.add(key); + localFlagKeys.push(key); } flag.metadata = metadata; }); - this.initialFlags = JSON.stringify(parsedFlags); + const initialFlagsString = JSON.stringify(this.initialFlags); // initialize the experiment this.experimentClient = Experiment.initialize(this.apiKey, { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore internalInstanceNameSuffix: 'web', - initialFlags: this.initialFlags, + initialFlags: initialFlagsString, // timeout for fetching remote flags fetchTimeoutMillis: 1000, pollOnStart: false, @@ -189,17 +199,17 @@ export class WebExperiment { this.experimentClient.addPlugin(this.globalScope.experimentIntegration); this.experimentClient.setUser(user); - this.setUrlChangeListener(new Set([...localFlagKeys, ...remoteFlagKeys])); + this.setUrlChangeListener([...localFlagKeys, ...remoteFlagKeys]); // apply local variants - this.applyVariants(this.experimentClient.all(), localFlagKeys); + this.applyVariants(localFlagKeys); if (!isRemoteBlocking) { // Remove anti-flicker css if remote flags are not blocking this.globalScope.document.getElementById?.('amp-exp-css')?.remove(); } - if (remoteFlagKeys.size === 0) { + if (remoteFlagKeys.length === 0) { return; } @@ -211,13 +221,22 @@ export class WebExperiment { console.warn('Error fetching remote flags:', error); } // apply remote variants - if fetch is unsuccessful, fallback order: 1. localStorage flags, 2. initial flags - this.applyVariants(this.experimentClient.all(), remoteFlagKeys); + this.applyVariants(remoteFlagKeys); } - public applyVariants( - variants: Variants, - flagKeys: Set | undefined = undefined, - ) { + /** + * Get the underlying ExperimentClient instance. + */ + public getExperimentClient(): ExperimentClient | undefined { + return this.experimentClient; + } + + /** + * Apply variants to the page. + * @param flagKeys + */ + public applyVariants(flagKeys: string[] | undefined = undefined) { + const variants = this.experimentClient?.all() || {}; if (Object.keys(variants).length === 0) { return; } @@ -232,7 +251,7 @@ export class WebExperiment { this.urlExposureCache[currentUrl] = {}; } for (const key in variants) { - if (flagKeys && !flagKeys.has(key)) { + if (flagKeys && !flagKeys.includes(key)) { continue; } const variant = variants[key]; @@ -250,16 +269,148 @@ export class WebExperiment { } if (payloadIsArray) { + this.handleVariantAction(key, variant); + } + } + } + } + + /** + * Revert mutations applied by the experiment. + * @param flagKeys + */ + public revertMutations(flagKeys: string[] | undefined = undefined) { + if (!flagKeys) { + flagKeys = Object.keys(this.appliedMutations); + } + for (const key of flagKeys) { + this.appliedMutations[key]?.forEach((mutationController) => { + mutationController.revert(); + }); + delete this.appliedMutations[key]; + } + } + + /** + * Get redirect URLs for flags. + * @param flagKeys + */ + public getRedirectUrls( + flagKeys: string[], + ): Record> { + const redirectUrlMap: Record> = {}; + for (const key of flagKeys) { + if (this.flagVariantMap[key] === undefined) { + continue; + } + const variants = this.flagVariantMap[key]; + const redirectUrls = {}; + Object.keys(variants).forEach((variantKey) => { + const variant = variants[variantKey]; + const payload = variant.payload; + if (payload && Array.isArray(payload)) { for (const action of variant.payload) { if (action.action === 'redirect') { - this.handleRedirect(action, key, variant); - } else if (action.action === 'mutate') { - this.handleMutate(action, key, variant); - } else if (action.action === 'inject') { - this.handleInject(action, key, variant); + const url = action.data?.url; + if (url) { + redirectUrls[key] = action.data.url; + } } } } + }); + } + return redirectUrlMap; + } + + /** + * Preview the effect of a variant on the page. + * @param key + * @param variant + */ + public previewVariant(key: string, variant: string) { + if (this.appliedMutations[key]) { + this.revertMutations([key]); + } + let flag: EvaluationFlag | undefined; + this.initialFlags.forEach((f: EvaluationFlag) => { + if (f.key === key) { + flag = f; + } + }); + if (!flag) { + return; + } + const variantObject = convertEvaluationVariantToVariant( + flag.variants[variant], + ); + if (!variantObject) { + return; + } + const payload = variantObject.payload; + if (!payload || !Array.isArray(payload)) { + return; + } + this.handleVariantAction(key, variantObject); + } + + /** + * Set URL change listener to revert mutations and apply variants on back/forward navigation. + * @param flagKeys + */ + public setUrlChangeListener(flagKeys: string[]) { + const globalScope = getGlobalScope(); + if (!globalScope) { + return; + } + // Add URL change listener for back/forward navigation + globalScope.addEventListener('popstate', () => { + this.revertMutations(); + this.applyVariants(flagKeys); + }); + + const handleUrlChange = () => { + this.revertMutations(); + this.applyVariants(flagKeys); + this.previousUrl = globalScope.location.href; + }; + + // Create wrapper functions for pushState and replaceState + const wrapHistoryMethods = () => { + const originalPushState = history.pushState; + const originalReplaceState = history.replaceState; + + // Wrapper for pushState + history.pushState = function (...args) { + // Call the original pushState + const result = originalPushState.apply(this, args); + // Revert mutations and apply variants + handleUrlChange(); + return result; + }; + + // Wrapper for replaceState + history.replaceState = function (...args) { + // Call the original replaceState + const result = originalReplaceState.apply(this, args); + // Revert mutations and apply variants + handleUrlChange(); + return result; + }; + }; + + // Initialize the wrapper + wrapHistoryMethods(); + } + + private handleVariantAction(key: string, variant: Variant) { + for (const action of variant.payload) { + if (action.action === 'redirect') { + this.handleRedirect(action, key, variant); + } else if (action.action === 'mutate') { + this.handleMutate(action, key, variant); + } else if (action.action === 'inject') { + this.handleInject(action, key, variant); } } } @@ -300,18 +451,14 @@ export class WebExperiment { return; } const mutations = action.data?.mutations; + const mutationControllers: MutationController[] = []; mutations.forEach((m) => { - this.appliedMutations.push(mutate.declarative(m)); + mutationControllers.push(mutate.declarative(m)); }); + this.appliedMutations[key] = mutationControllers; this.exposureWithDedupe(key, variant); } - public revertMutations() { - while (this.appliedMutations.length > 0) { - this.appliedMutations.pop()?.revert(); - } - } - private handleInject(action, key: string, variant: Variant) { const globalScope = getGlobalScope(); if (!globalScope) { @@ -374,67 +521,24 @@ export class WebExperiment { ); } // Push mutation to remove CSS and any custom state cleanup set in utils. - this.appliedMutations.push({ - revert: () => { - if (utils.remove) utils.remove(); - style?.remove(); - script?.remove(); - this.appliedInjections.delete(id); + this.appliedMutations[key] = [ + { + revert: () => { + if (utils.remove) utils.remove(); + style?.remove(); + script?.remove(); + this.appliedInjections.delete(id); + }, }, - }); + ]; this.exposureWithDedupe(key, variant); } - public setUrlChangeListener(flagKeys: Set) { - const globalScope = getGlobalScope(); - if (!globalScope) { - return; - } - // Add URL change listener for back/forward navigation - globalScope.addEventListener('popstate', () => { - this.revertMutations(); - this.applyVariants(this.experimentClient?.all() || {}, flagKeys); - }); - - const handleUrlChange = () => { - this.revertMutations(); - this.applyVariants(this.experimentClient?.all() || {}, flagKeys); - this.previousUrl = globalScope.location.href; - }; - - // Create wrapper functions for pushState and replaceState - const wrapHistoryMethods = () => { - const originalPushState = history.pushState; - const originalReplaceState = history.replaceState; - - // Wrapper for pushState - history.pushState = function (...args) { - // Call the original pushState - const result = originalPushState.apply(this, args); - // Revert mutations and apply variants - handleUrlChange(); - return result; - }; - - // Wrapper for replaceState - history.replaceState = function (...args) { - // Call the original replaceState - const result = originalReplaceState.apply(this, args); - // Revert mutations and apply variants - handleUrlChange(); - return result; - }; - }; - - // Initialize the wrapper - wrapHistoryMethods(); - } - private isPageTargetingSegment(segment: EvaluationSegment) { return ( segment.metadata?.trackExposure === false && - (segment.metadata?.segmentName === 'Page not targeted' || - segment.metadata?.segmentName === 'Page is excluded') + (segment.metadata?.segmentName === PAGE_NOT_TARGETED || + segment.metadata?.segmentName === PAGE_IS_EXCLUDED) ); } diff --git a/packages/experiment-tag/src/util.ts b/packages/experiment-tag/src/util.ts index ec7ac8e4..21154fa1 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 { EvaluationVariant, getGlobalScope } from '@amplitude/experiment-core'; +import { Variant } from '@amplitude/experiment-js-client'; export const getUrlParams = (): Record => { const globalScope = getGlobalScope(); @@ -87,3 +88,27 @@ export const concatenateQueryParamsOf = ( return resultUrlObj.toString(); }; + +export const convertEvaluationVariantToVariant = ( + evaluationVariant: EvaluationVariant, +): Variant => { + if (!evaluationVariant) { + return {}; + } + let experimentKey: string | undefined = undefined; + if (evaluationVariant.metadata) { + if (typeof evaluationVariant.metadata['experimentKey'] === 'string') { + experimentKey = evaluationVariant.metadata['experimentKey']; + } else { + experimentKey = undefined; + } + } + const variant: Variant = {}; + if (evaluationVariant.key) variant.key = evaluationVariant.key; + if (evaluationVariant.value) + variant.value = evaluationVariant.value as string; + if (evaluationVariant.payload) variant.payload = evaluationVariant.payload; + if (experimentKey) variant.expKey = experimentKey; + if (evaluationVariant.metadata) variant.metadata = evaluationVariant.metadata; + return variant; +}; From 718f02afa01a4fb87d77637fc2cb362355d17d2e Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Mon, 30 Dec 2024 14:18:12 -0800 Subject: [PATCH 24/33] Refactor and add additional methods --- .../experiment-browser/src/util/convert.ts | 10 ++-- packages/experiment-tag/src/experiment.ts | 59 +++++++++++++++---- 2 files changed, 54 insertions(+), 15 deletions(-) diff --git a/packages/experiment-browser/src/util/convert.ts b/packages/experiment-browser/src/util/convert.ts index b3b5c5cf..76f633d2 100644 --- a/packages/experiment-browser/src/util/convert.ts +++ b/packages/experiment-browser/src/util/convert.ts @@ -12,11 +12,11 @@ export const convertUserToContext = ( const context: Record = { user: user }; // add page context const globalScope = getGlobalScope(); - if (globalScope) { - context.page = { - url: globalScope.location.href, - }; - } + context.page = { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + url: user.currentUrl ?? globalScope?.location.href, + }; const groups: Record> = {}; if (!user.groups) { return context; diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index ec5bdd99..6259bddb 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -11,6 +11,8 @@ import { AmplitudeIntegrationPlugin, ExperimentConfig, ExperimentClient, + ExperimentUser, + Variants, } from '@amplitude/experiment-js-client'; import mutate, { MutationController } from 'dom-mutator'; @@ -232,7 +234,7 @@ export class WebExperiment { } /** - * Apply variants to the page. + * Apply evaluated variants to the page. * @param flagKeys */ public applyVariants(flagKeys: string[] | undefined = undefined) { @@ -332,18 +334,11 @@ export class WebExperiment { if (this.appliedMutations[key]) { this.revertMutations([key]); } - let flag: EvaluationFlag | undefined; - this.initialFlags.forEach((f: EvaluationFlag) => { - if (f.key === key) { - flag = f; - } - }); + const flag = this.flagVariantMap[key]; if (!flag) { return; } - const variantObject = convertEvaluationVariantToVariant( - flag.variants[variant], - ); + const variantObject = flag[variant]; if (!variantObject) { return; } @@ -354,6 +349,50 @@ export class WebExperiment { this.handleVariantAction(key, variantObject); } + /** + * Get all variants for a user. If user is not provided, the current user is used. + * If currentUrl is not provided, the current URL is used. + * If flagKeys is not provided, all variants are returned. + * @param user + * @param currentUrl + * @param flagKeys + */ + public getVariants( + user: ExperimentUser | undefined = undefined, + currentUrl: string | undefined = undefined, + flagKeys: string[] | undefined = undefined, + ): Variants { + if (!this.experimentClient) { + return {}; + } + const existingUser = this.experimentClient?.getUser(); + if (user) { + if (currentUrl) { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + user.currentUrl = currentUrl; + } + this.experimentClient.setUser(user); + } else { + this.experimentClient.setUser({ + ...existingUser, + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + currentUrl: currentUrl, + }); + } + const variants = this.experimentClient.all(); + if (flagKeys) { + const filteredVariants = {}; + for (const key of flagKeys) { + filteredVariants[key] = variants[key]; + } + return filteredVariants; + } + this.experimentClient.setUser(existingUser); + return variants; + } + /** * Set URL change listener to revert mutations and apply variants on back/forward navigation. * @param flagKeys From 73cd67f32ef49308499ebbabb64f837ba8e912b1 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Thu, 2 Jan 2025 11:51:13 -0800 Subject: [PATCH 25/33] Additional tests and config --- packages/experiment-tag/src/config.ts | 17 ++ packages/experiment-tag/src/experiment.ts | 152 ++++++++++----- packages/experiment-tag/src/script.ts | 8 +- .../experiment-tag/test/experiment.test.ts | 180 ++++++++++++++++-- 4 files changed, 287 insertions(+), 70 deletions(-) create mode 100644 packages/experiment-tag/src/config.ts diff --git a/packages/experiment-tag/src/config.ts b/packages/experiment-tag/src/config.ts new file mode 100644 index 00000000..062b5acf --- /dev/null +++ b/packages/experiment-tag/src/config.ts @@ -0,0 +1,17 @@ +import { ExperimentConfig } from '@amplitude/experiment-js-client'; + +export interface WebExperimentConfig extends ExperimentConfig { + /** + * Determines whether variants actions should be reverted and reapplied on navigation events. + */ + reapplyVariantsOnNavigation?: boolean; + /** + * Determines whether anti-flicker CSS should be applied for remote blocking flags. + */ + applyAntiFlickerForRemoteBlocking?: boolean; +} + +export const Defaults: WebExperimentConfig = { + reapplyVariantsOnNavigation: true, + applyAntiFlickerForRemoteBlocking: true, +}; diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 6259bddb..b76d7655 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -16,6 +16,7 @@ import { } from '@amplitude/experiment-js-client'; import mutate, { MutationController } from 'dom-mutator'; +import { WebExperimentConfig } from './config'; import { getInjectUtils } from './inject-utils'; import { WindowMessenger } from './messenger'; import { @@ -33,21 +34,24 @@ const PAGE_IS_EXCLUDED = 'Page is excluded'; export class WebExperiment { private readonly apiKey: string; private readonly initialFlags: []; - private readonly config: ExperimentConfig; + private readonly config: WebExperimentConfig; private readonly globalScope = getGlobalScope(); - private experimentClient: ExperimentClient | undefined; + private readonly experimentClient: ExperimentClient | undefined; private appliedInjections: Set = new Set(); private appliedMutations: Record = {}; - private previousUrl: string | undefined; + private previousUrl: string | undefined = undefined; // Cache to track exposure for the current URL, should be cleared on URL change private urlExposureCache: Record> = {}; private flagVariantMap: Record> = {}; + private localFlagKeys: string[] = []; + private remoteFlagKeys: string[] = []; + private isRemoteBlocking = false; constructor( apiKey: string, initialFlags: string, - config: ExperimentConfig = {}, + config: WebExperimentConfig = {}, ) { this.apiKey = apiKey; this.config = config; @@ -60,9 +64,6 @@ export class WebExperiment { convertEvaluationVariantToVariant(flag.variants[variantKey]); }); }); - } - - public async initializeExperiment() { if (this.globalScope?.webExperiment) { return; } @@ -73,8 +74,6 @@ export class WebExperiment { } this.globalScope.webExperiment = this; - this.previousUrl = undefined; - this.urlExposureCache = {}; const experimentStorageName = `EXP_${this.apiKey.slice(0, 10)}`; let user; try { @@ -114,10 +113,6 @@ export class WebExperiment { return; } - let isRemoteBlocking = false; - const remoteFlagKeys: string[] = []; - const localFlagKeys: string[] = []; - this.initialFlags.forEach((flag: EvaluationFlag) => { const { key, variants, segments, metadata = {} } = flag; @@ -153,21 +148,21 @@ export class WebExperiment { } if (metadata.evaluationMode !== 'local') { - remoteFlagKeys.push(key); + this.remoteFlagKeys.push(key); // allow local evaluation for remote flags metadata.evaluationMode = 'local'; // Check if any remote flags are blocking - if (!isRemoteBlocking && metadata.blockingEvaluation) { - isRemoteBlocking = true; + if (!this.isRemoteBlocking && metadata.blockingEvaluation) { + this.isRemoteBlocking = true; // Apply anti-flicker CSS to prevent UI flicker this.applyAntiFlickerCss(); } } else { // Add locally evaluable flags to the local flag set - localFlagKeys.push(key); + this.localFlagKeys.push(key); } flag.metadata = metadata; @@ -200,30 +195,40 @@ export class WebExperiment { this.globalScope.experimentIntegration.type = 'integration'; this.experimentClient.addPlugin(this.globalScope.experimentIntegration); this.experimentClient.setUser(user); + } - this.setUrlChangeListener([...localFlagKeys, ...remoteFlagKeys]); + /** + * Start the experiment. + */ + public async start() { + if (!this.experimentClient) { + return; + } + + this.config.reapplyVariantsOnNavigation && + this.setUrlChangeListener([ + ...this.localFlagKeys, + ...this.remoteFlagKeys, + ]); // apply local variants - this.applyVariants(localFlagKeys); + this.applyVariants(this.localFlagKeys); - if (!isRemoteBlocking) { + if ( + !this.isRemoteBlocking || + !this.config.applyAntiFlickerForRemoteBlocking + ) { // Remove anti-flicker css if remote flags are not blocking - this.globalScope.document.getElementById?.('amp-exp-css')?.remove(); + this.globalScope?.document.getElementById?.('amp-exp-css')?.remove(); } - if (remoteFlagKeys.length === 0) { + if (this.remoteFlagKeys.length === 0) { return; } - try { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - await this.experimentClient.doFlags(); - } catch (error) { - console.warn('Error fetching remote flags:', error); - } + await this.fetchRemoteFlags(); // apply remote variants - if fetch is unsuccessful, fallback order: 1. localStorage flags, 2. initial flags - this.applyVariants(remoteFlagKeys); + this.applyVariants(this.remoteFlagKeys); } /** @@ -233,6 +238,37 @@ export class WebExperiment { return this.experimentClient; } + /** + * Set the context for evaluating experiments. + * If user is not provided, the current user is used. + * If currentUrl is not provided, the current URL is used. + * @param user + * @param currentUrl + */ + public setContext( + user: ExperimentUser | undefined = undefined, + currentUrl: string | undefined = undefined, + ) { + if (this.experimentClient) { + const existingUser = this.experimentClient?.getUser(); + if (user) { + if (currentUrl) { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + user.currentUrl = currentUrl; + } + this.experimentClient.setUser(user); + } else { + this.experimentClient.setUser({ + ...existingUser, + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + currentUrl: currentUrl, + }); + } + } + } + /** * Apply evaluated variants to the page. * @param flagKeys @@ -298,9 +334,12 @@ export class WebExperiment { * @param flagKeys */ public getRedirectUrls( - flagKeys: string[], + flagKeys: string[] | undefined = undefined, ): Record> { const redirectUrlMap: Record> = {}; + if (!flagKeys) { + flagKeys = Object.keys(this.flagVariantMap); + } for (const key of flagKeys) { if (this.flagVariantMap[key] === undefined) { continue; @@ -315,12 +354,15 @@ export class WebExperiment { if (action.action === 'redirect') { const url = action.data?.url; if (url) { - redirectUrls[key] = action.data.url; + redirectUrls[variantKey] = action.data.url; } } } } }); + if (Object.keys(redirectUrls).length > 0) { + redirectUrlMap[key] = redirectUrls; + } } return redirectUrlMap; } @@ -366,21 +408,7 @@ export class WebExperiment { return {}; } const existingUser = this.experimentClient?.getUser(); - if (user) { - if (currentUrl) { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - user.currentUrl = currentUrl; - } - this.experimentClient.setUser(user); - } else { - this.experimentClient.setUser({ - ...existingUser, - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - currentUrl: currentUrl, - }); - } + this.setContext(user, currentUrl); const variants = this.experimentClient.all(); if (flagKeys) { const filteredVariants = {}; @@ -389,10 +417,38 @@ export class WebExperiment { } return filteredVariants; } - this.experimentClient.setUser(existingUser); + this.setContext(existingUser, undefined); return variants; } + /** + * Fetch remote flags based on the current user. + */ + public async fetchRemoteFlags() { + if (!this.experimentClient) { + return; + } + try { + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + await this.experimentClient.doFlags(); + } catch (error) { + console.warn('Error fetching remote flags:', error); + } + } + + public getActiveExperimentsOnPage( + currentUrl: string | undefined = undefined, + ): string[] { + const variants = this.getVariants(undefined, currentUrl); + return Object.keys(variants).filter((key) => { + return ( + variants[key].metadata?.segmentName !== PAGE_NOT_TARGETED && + variants[key].metadata?.segmentName !== PAGE_IS_EXCLUDED + ); + }); + } + /** * Set URL change listener to revert mutations and apply variants on back/forward navigation. * @param flagKeys diff --git a/packages/experiment-tag/src/script.ts b/packages/experiment-tag/src/script.ts index e65f30d3..639f894f 100644 --- a/packages/experiment-tag/src/script.ts +++ b/packages/experiment-tag/src/script.ts @@ -1,9 +1,13 @@ +import { Defaults } from './config'; import { WebExperiment } from './experiment'; const API_KEY = '{{DEPLOYMENT_KEY}}'; const initialFlags = '{{INITIAL_FLAGS}}'; -const webExperimentClient = new WebExperiment(API_KEY, initialFlags); -webExperimentClient.initializeExperiment().then(() => { +const webExperimentClient = new WebExperiment(API_KEY, initialFlags, { + reapplyVariantsOnNavigation: Defaults.reapplyVariantsOnNavigation, + applyAntiFlickerForRemoteBlocking: Defaults.applyAntiFlickerForRemoteBlocking, +}); +webExperimentClient.start().then(() => { // Remove anti-flicker css if it exists document.getElementById('amp-exp-css')?.remove(); }); diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index c780a96d..312f6ea9 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -56,7 +56,7 @@ describe('initializeExperiment', () => { stringify(apiKey), JSON.stringify([]), ); - webExperiment.initializeExperiment(); + webExperiment.start(); expect(ExperimentClient.prototype.setUser).toHaveBeenCalledWith({ device_id: 'mock', web_exp_id: 'mock', @@ -68,14 +68,14 @@ describe('initializeExperiment', () => { }); test('experiment should not run without localStorage', () => { + jest + .spyOn(experimentCore, 'isLocalStorageAvailable') + .mockReturnValue(false); const webExperiment = new WebExperiment( stringify(apiKey), JSON.stringify([]), ); - jest - .spyOn(experimentCore, 'isLocalStorageAvailable') - .mockReturnValue(false); - webExperiment.initializeExperiment(); + webExperiment.start(); expect(mockGlobal.localStorage.getItem).toHaveBeenCalledTimes(0); }); @@ -86,7 +86,7 @@ describe('initializeExperiment', () => { createRedirectFlag('test', 'treatment', 'http://test.com/2'), ]), ); - webExperiment.initializeExperiment(); + webExperiment.start(); expect(mockGlobal.location.replace).toHaveBeenCalledWith( 'http://test.com/2', @@ -101,7 +101,7 @@ describe('initializeExperiment', () => { createRedirectFlag('test', 'control', 'http://test.com/2'), ]), ); - webExperiment.initializeExperiment(); + webExperiment.start(); expect(mockGlobal.location.replace).toBeCalledTimes(0); expect(mockExposure).toHaveBeenCalledWith('test'); expect(mockGlobal.history.replaceState).toBeCalledTimes(0); @@ -132,7 +132,7 @@ describe('initializeExperiment', () => { createRedirectFlag('test', 'treatment', 'http://test.com/2'), ]), ); - webExperiment.initializeExperiment(); + webExperiment.start(); expect(mockGlobal.location.replace).toHaveBeenCalledTimes(0); expect(mockGlobal.history.replaceState).toHaveBeenCalledWith( {}, @@ -166,7 +166,7 @@ describe('initializeExperiment', () => { createRedirectFlag('test', 'treatment', 'http://test.com/2'), ]), ); - webExperiment.initializeExperiment(); + webExperiment.start(); expect(mockGlobal.location.replace).toHaveBeenCalledWith( 'http://test.com/2', @@ -223,7 +223,7 @@ describe('initializeExperiment', () => { ), ]), ); - webExperiment.initializeExperiment(); + webExperiment.start(); expect(mockGlobal.location.replace).toHaveBeenCalledTimes(0); expect(mockExposure).toHaveBeenCalledTimes(0); @@ -263,7 +263,7 @@ describe('initializeExperiment', () => { ), ]), ); - webExperiment.initializeExperiment(); + webExperiment.start(); expect(mockGlobal.location.replace).toHaveBeenCalledWith( 'http://test.com/2?param3=c¶m1=a¶m2=b', @@ -278,7 +278,7 @@ describe('initializeExperiment', () => { createRedirectFlag('test', 'control', 'http://test.com/2?param3=c'), ]), ); - webExperiment.initializeExperiment(); + webExperiment.start(); expect(mockGlobal.location.replace).not.toHaveBeenCalled(); expect(mockExposure).toHaveBeenCalledWith('test'); @@ -322,7 +322,7 @@ describe('initializeExperiment', () => { ), ]), ); - webExperiment.initializeExperiment(); + webExperiment.start(); expect(mockExposure).toHaveBeenCalledWith('test'); }); @@ -363,7 +363,7 @@ describe('initializeExperiment', () => { ), ]), ); - webExperiment.initializeExperiment(); + webExperiment.start(); expect(mockExposure).not.toHaveBeenCalled(); }); @@ -385,7 +385,7 @@ describe('initializeExperiment', () => { httpClient: mockHttpClient, }, ); - webExperiment.initializeExperiment().then(() => { + webExperiment.start().then(() => { expect(mockHttpClient.requestUrl).toBe( 'https://flag.lab.amplitude.com/sdk/v2/flags?delivery_method=web', ); @@ -414,7 +414,7 @@ describe('initializeExperiment', () => { httpClient: mockHttpClient, }, ); - webExperiment.initializeExperiment().then(() => { + webExperiment.start().then(() => { // check remote flag variant actions called after successful fetch expect(mockExposure).toHaveBeenCalledTimes(2); expect(mockExposure).toHaveBeenCalledWith('test-2'); @@ -442,7 +442,7 @@ describe('initializeExperiment', () => { httpClient: mockHttpClient, }, ); - webExperiment.initializeExperiment().then(() => { + webExperiment.start().then(() => { // check remote fetch failed safely expect(mockExposure).toHaveBeenCalledTimes(2); }); @@ -466,7 +466,7 @@ describe('initializeExperiment', () => { httpClient: mockHttpClient, }, ); - webExperiment.initializeExperiment().then(() => { + webExperiment.start().then(() => { // check remote variant actions applied expect(mockExposure).toHaveBeenCalledTimes(1); expect(mockExposure).toHaveBeenCalledWith('test'); @@ -509,7 +509,7 @@ describe('initializeExperiment', () => { httpClient: mockHttpClient, }, ); - webExperiment.initializeExperiment().then(() => { + webExperiment.start().then(() => { // check remote fetch not called expect(doFlagsMock).toHaveBeenCalledTimes(0); }); @@ -539,7 +539,7 @@ describe('initializeExperiment', () => { httpClient: mockHttpClient, }, ); - await webExperiment.initializeExperiment(); + await webExperiment.start(); // check treatment variant called expect(mockExposure).toHaveBeenCalledTimes(1); expect(mockExposure).toHaveBeenCalledWith('test'); @@ -548,3 +548,143 @@ describe('initializeExperiment', () => { ); }); }); + +describe('helper methods', () => { + beforeEach(() => { + const mockGetGlobalScope = jest.spyOn(experimentCore, 'getGlobalScope'); + const mockGlobal = { + localStorage: { + getItem: jest.fn().mockReturnValue(undefined), + setItem: jest.fn(), + }, + location: { + href: 'http://test.com', + replace: jest.fn(), + search: '', + }, + document: { referrer: '' }, + history: { replaceState: jest.fn() }, + }; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + mockGetGlobalScope.mockReturnValue(mockGlobal); + apiKey++; + jest.spyOn(util, 'UUID').mockReturnValue('mock'); + jest.clearAllMocks(); + }); + + const originalLocation = global.location; // Save the original global location + + afterEach(() => { + // Restore the original global location after each test + Object.defineProperty(global, 'location', { + value: originalLocation, + writable: true, + }); + jest.restoreAllMocks(); // Restore any mocked functions + }); + test('get redirect urls', () => { + const initialFlags = [ + // remote flag + createRedirectFlag('test', 'treatment', 'http://test.com/2'), + ]; + const webExperiment = new WebExperiment( + stringify(apiKey), + JSON.stringify(initialFlags), + ); + const redirectUrls = webExperiment.getRedirectUrls(); + expect(redirectUrls).toEqual({ test: { treatment: 'http://test.com/2' } }); + }); + + test('get active experiments on current page', () => { + Object.defineProperty(global, 'location', { + value: { + href: 'http://test.com', + }, + writable: true, + }); + jest.spyOn(experimentCore, 'getGlobalScope'); + const targetedSegment = [ + { + conditions: [ + [ + { + op: 'regex does not match', + selector: ['context', 'page', 'url'], + values: ['^http:\\/\\/test.*'], + }, + ], + ], + metadata: { + segmentName: 'Page not targeted', + trackExposure: false, + }, + variant: 'off', + }, + ]; + const nonTargetedSegment = [ + { + conditions: [ + [ + { + op: 'regex match', + selector: ['context', 'page', 'url'], + values: ['.*test\\.com$'], + }, + ], + ], + metadata: { + segmentName: 'Page is excluded', + trackExposure: false, + }, + variant: 'off', + }, + ]; + const webExperiment = new WebExperiment( + stringify(apiKey), + JSON.stringify([ + createRedirectFlag( + 'targeted', + 'treatment', + '', + undefined, + targetedSegment, + ), + createRedirectFlag( + 'non-targeted', + 'treatment', + '', + undefined, + nonTargetedSegment, + ), + ]), + ); + let activeExperiments = webExperiment.getActiveExperimentsOnPage(); + expect(activeExperiments).toEqual(['targeted']); + activeExperiments = webExperiment.getActiveExperimentsOnPage( + 'http://override.com', + ); + expect(activeExperiments).toEqual(['non-targeted']); + }); + + test('get variants', () => { + const targetedSegment = [ + { + metadata: { + segmentName: 'match segment', + }, + variant: 'treatment', + }, + ]; + const webExperiment = new WebExperiment( + stringify(apiKey), + JSON.stringify([ + createRedirectFlag('flag-1', 'control', '', undefined, targetedSegment), + createRedirectFlag('flag-2', 'control', '', undefined), + ]), + ); + const variants = webExperiment.getVariants(); + expect(variants['flag-1'].key).toEqual('treatment'); + expect(variants['flag-2'].key).toEqual('control'); + }); +}); From 99ad580d07f5a1d5df5bd55156582452236e9108 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Thu, 2 Jan 2025 13:26:12 -0800 Subject: [PATCH 26/33] update tests --- packages/experiment-tag/test/experiment.test.ts | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index 312f6ea9..b98fdcfe 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -573,15 +573,14 @@ describe('helper methods', () => { jest.clearAllMocks(); }); - const originalLocation = global.location; // Save the original global location + const originalLocation = global.location; afterEach(() => { - // Restore the original global location after each test Object.defineProperty(global, 'location', { value: originalLocation, writable: true, }); - jest.restoreAllMocks(); // Restore any mocked functions + jest.restoreAllMocks(); }); test('get redirect urls', () => { const initialFlags = [ From 4cdabc7a1be54373d51331e0a1042770655b78de Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Thu, 2 Jan 2025 14:55:51 -0800 Subject: [PATCH 27/33] update with index --- packages/experiment-tag/src/experiment.ts | 13 +++++++++++-- packages/experiment-tag/src/index.ts | 2 ++ packages/experiment-tag/test/experiment.test.ts | 11 +++++++++-- 3 files changed, 22 insertions(+), 4 deletions(-) create mode 100644 packages/experiment-tag/src/index.ts diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index b76d7655..44429edd 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -9,7 +9,6 @@ import { Experiment, Variant, AmplitudeIntegrationPlugin, - ExperimentConfig, ExperimentClient, ExperimentUser, Variants, @@ -205,11 +204,12 @@ export class WebExperiment { return; } - this.config.reapplyVariantsOnNavigation && + if (this.config.reapplyVariantsOnNavigation) { this.setUrlChangeListener([ ...this.localFlagKeys, ...this.remoteFlagKeys, ]); + } // apply local variants this.applyVariants(this.localFlagKeys); @@ -269,6 +269,15 @@ export class WebExperiment { } } + /** + * Set the previous URL for tracking back/forward navigation. Set previous URL to prevent infinite redirection loop + * in single-page applications. + * @param url + */ + public setPreviousUrl(url: string) { + this.previousUrl = url; + } + /** * Apply evaluated variants to the page. * @param flagKeys diff --git a/packages/experiment-tag/src/index.ts b/packages/experiment-tag/src/index.ts new file mode 100644 index 00000000..1eee51a8 --- /dev/null +++ b/packages/experiment-tag/src/index.ts @@ -0,0 +1,2 @@ +export { WebExperiment } from './experiment'; +export { WebExperimentConfig } from './config'; diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index b98fdcfe..b7647c8b 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -585,14 +585,21 @@ describe('helper methods', () => { test('get redirect urls', () => { const initialFlags = [ // remote flag - createRedirectFlag('test', 'treatment', 'http://test.com/2'), + createRedirectFlag( + 'test', + 'treatment', + 'http://test.com/2', + 'http://test.com', + ), ]; const webExperiment = new WebExperiment( stringify(apiKey), JSON.stringify(initialFlags), ); const redirectUrls = webExperiment.getRedirectUrls(); - expect(redirectUrls).toEqual({ test: { treatment: 'http://test.com/2' } }); + expect(redirectUrls).toEqual({ + test: { control: 'http://test.com', treatment: 'http://test.com/2' }, + }); }); test('get active experiments on current page', () => { From b46f8bd8f560497f0b08eb7d518f0dfb73fa9784 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Fri, 3 Jan 2025 11:24:47 -0800 Subject: [PATCH 28/33] add server zone config --- packages/experiment-tag/src/script.ts | 3 +++ packages/experiment-tag/test/live.test.ts | 19 +++++++++++++++++++ 2 files changed, 22 insertions(+) create mode 100644 packages/experiment-tag/test/live.test.ts diff --git a/packages/experiment-tag/src/script.ts b/packages/experiment-tag/src/script.ts index 639f894f..498a325e 100644 --- a/packages/experiment-tag/src/script.ts +++ b/packages/experiment-tag/src/script.ts @@ -3,9 +3,12 @@ import { WebExperiment } from './experiment'; const API_KEY = '{{DEPLOYMENT_KEY}}'; const initialFlags = '{{INITIAL_FLAGS}}'; +const serverZone = '{{SERVER_ZONE}}'; + const webExperimentClient = new WebExperiment(API_KEY, initialFlags, { reapplyVariantsOnNavigation: Defaults.reapplyVariantsOnNavigation, applyAntiFlickerForRemoteBlocking: Defaults.applyAntiFlickerForRemoteBlocking, + serverZone: serverZone, }); webExperimentClient.start().then(() => { // Remove anti-flicker css if it exists diff --git a/packages/experiment-tag/test/live.test.ts b/packages/experiment-tag/test/live.test.ts new file mode 100644 index 00000000..7cb76bbb --- /dev/null +++ b/packages/experiment-tag/test/live.test.ts @@ -0,0 +1,19 @@ +import { initializeExperiment } from 'src/experiment'; + +import { createMutateFlag } from './util/create-flag'; + +describe('live', () => { + test('live remote', async () => { + const initialRemote = JSON.stringify([ + createMutateFlag('tim-test-web-remote', 'control', [], [], [], 'remote'), + ]); + // Test that the flag is live and the treatment is returned + await initializeExperiment( + 'client-DvWljIjiiuqLbyjqdvBaLFfEBrAvGuA3', + initialRemote, + { + flagsServerUrl: 'http://localhost:3034', + }, + ); + }); +}); From 3fcf8c70156b3dab05264c45622f01a279f49c40 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Mon, 6 Jan 2025 16:08:55 -0800 Subject: [PATCH 29/33] add types, clean up functions, fix lint --- packages/experiment-tag/src/experiment.ts | 80 ++++++++++--------- packages/experiment-tag/src/types.ts | 14 ++++ .../experiment-tag/test/experiment.test.ts | 2 +- 3 files changed, 56 insertions(+), 40 deletions(-) create mode 100644 packages/experiment-tag/src/types.ts diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 71b2550b..c5296cf8 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -20,6 +20,11 @@ import mutate, { MutationController } from 'dom-mutator'; import { WebExperimentConfig } from './config'; import { getInjectUtils } from './inject-utils'; import { WindowMessenger } from './messenger'; +import { + ApplyVariantsOption, + RevertVariantsOptions, + WebExperimentContext, +} from './types'; import { convertEvaluationVariantToVariant, getUrlParams, @@ -216,7 +221,7 @@ export class WebExperiment { } // apply local variants - this.applyVariants(this.localFlagKeys); + this.applyVariants({ flagKeys: this.localFlagKeys }); if ( !this.isRemoteBlocking || @@ -232,7 +237,7 @@ export class WebExperiment { await this.fetchRemoteFlags(); // apply remote variants - if fetch is unsuccessful, fallback order: 1. localStorage flags, 2. initial flags - this.applyVariants(this.remoteFlagKeys); + this.applyVariants({ flagKeys: this.remoteFlagKeys }); } /** @@ -244,30 +249,26 @@ export class WebExperiment { /** * Set the context for evaluating experiments. - * If user is not provided, the current user is used. - * If currentUrl is not provided, the current URL is used. - * @param user - * @param currentUrl + * If user is undefined, the current user is used. + * If currentUrl is undefined, the current URL is used. + * @param webExperimentContext */ - public setContext( - user: ExperimentUser | undefined = undefined, - currentUrl: string | undefined = undefined, - ) { + public setContext(webExperimentContext: WebExperimentContext) { if (this.experimentClient) { const existingUser = this.experimentClient?.getUser(); - if (user) { - if (currentUrl) { + if (webExperimentContext.user) { + if (webExperimentContext.currentUrl) { // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore user.currentUrl = currentUrl; } - this.experimentClient.setUser(user); + this.experimentClient.setUser(webExperimentContext.user); } else { this.experimentClient.setUser({ ...existingUser, // eslint-disable-next-line @typescript-eslint/ban-ts-comment // @ts-ignore - currentUrl: currentUrl, + currentUrl: webExperimentContext.currentUrl, }); } } @@ -284,9 +285,10 @@ export class WebExperiment { /** * Apply evaluated variants to the page. - * @param flagKeys + * @param applyVariantsOption */ - public applyVariants(flagKeys: string[] | undefined = undefined) { + public applyVariants(applyVariantsOption?: ApplyVariantsOption) { + const { flagKeys } = applyVariantsOption || {}; const variants = this.experimentClient?.all() || {}; if (Object.keys(variants).length === 0) { return; @@ -328,9 +330,10 @@ export class WebExperiment { /** * Revert mutations applied by the experiment. - * @param flagKeys + * @param revertVariantsOptions */ - public revertMutations(flagKeys: string[] | undefined = undefined) { + public revertVariants(revertVariantsOptions?: RevertVariantsOptions) { + let { flagKeys } = revertVariantsOptions || {}; if (!flagKeys) { flagKeys = Object.keys(this.appliedMutations); } @@ -347,7 +350,7 @@ export class WebExperiment { * @param flagKeys */ public getRedirectUrls( - flagKeys: string[] | undefined = undefined, + flagKeys?: string[], ): Record> { const redirectUrlMap: Record> = {}; if (!flagKeys) { @@ -387,7 +390,7 @@ export class WebExperiment { */ public previewVariant(key: string, variant: string) { if (this.appliedMutations[key]) { - this.revertMutations([key]); + this.revertVariants({ flagKeys: [key] }); } const flag = this.flagVariantMap[key]; if (!flag) { @@ -405,23 +408,24 @@ export class WebExperiment { } /** - * Get all variants for a user. If user is not provided, the current user is used. - * If currentUrl is not provided, the current URL is used. - * If flagKeys is not provided, all variants are returned. - * @param user - * @param currentUrl + * Get all variants for a given WebExperimentContext. + * If user is undefined, the current user is used. + * If currentUrl is undefined, the current URL is used. + * If flagKeys is undefined, all variants are returned. + * @param webExperimentContext * @param flagKeys */ public getVariants( - user: ExperimentUser | undefined = undefined, - currentUrl: string | undefined = undefined, - flagKeys: string[] | undefined = undefined, + webExperimentContext?: WebExperimentContext, + flagKeys?: string[], ): Variants { if (!this.experimentClient) { return {}; } - const existingUser = this.experimentClient?.getUser(); - this.setContext(user, currentUrl); + const existingContext: WebExperimentContext = { + user: this.experimentClient?.getUser(), + }; + webExperimentContext && this.setContext(webExperimentContext); const variants = this.experimentClient.all(); if (flagKeys) { const filteredVariants = {}; @@ -430,7 +434,7 @@ export class WebExperiment { } return filteredVariants; } - this.setContext(existingUser, undefined); + this.setContext(existingContext); return variants; } @@ -450,10 +454,8 @@ export class WebExperiment { } } - public getActiveExperimentsOnPage( - currentUrl: string | undefined = undefined, - ): string[] { - const variants = this.getVariants(undefined, currentUrl); + public getActiveExperimentsOnPage(currentUrl?: string): string[] { + const variants = this.getVariants({ currentUrl: currentUrl }); return Object.keys(variants).filter((key) => { return ( variants[key].metadata?.segmentName !== PAGE_NOT_TARGETED && @@ -473,13 +475,13 @@ export class WebExperiment { } // Add URL change listener for back/forward navigation globalScope.addEventListener('popstate', () => { - this.revertMutations(); - this.applyVariants(flagKeys); + this.revertVariants(); + this.applyVariants({ flagKeys: flagKeys }); }); const handleUrlChange = () => { - this.revertMutations(); - this.applyVariants(flagKeys); + this.revertVariants(); + this.applyVariants({ flagKeys: flagKeys }); this.previousUrl = globalScope.location.href; }; diff --git a/packages/experiment-tag/src/types.ts b/packages/experiment-tag/src/types.ts new file mode 100644 index 00000000..766bd26f --- /dev/null +++ b/packages/experiment-tag/src/types.ts @@ -0,0 +1,14 @@ +import { ExperimentUser } from '@amplitude/experiment-js-client'; + +export type WebExperimentContext = { + user?: ExperimentUser; + currentUrl?: string; +}; + +export type ApplyVariantsOption = { + flagKeys?: string[]; +}; + +export type RevertVariantsOptions = { + flagKeys?: string[]; +}; diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index f6af41dd..62815ecb 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -1,4 +1,5 @@ import * as experimentCore from '@amplitude/experiment-core'; +import { safeGlobal } from '@amplitude/experiment-core'; import { ExperimentClient } from '@amplitude/experiment-js-client'; import { Base64 } from 'js-base64'; import { WebExperiment } from 'src/experiment'; @@ -7,7 +8,6 @@ import { stringify } from 'ts-jest'; import { createMutateFlag, createRedirectFlag } from './util/create-flag'; import { MockHttpClient } from './util/mock-http-client'; -import { safeGlobal } from '@amplitude/experiment-core'; let apiKey = 0; From ee29ce99692a9962858eccc18c18ee3e3e886490 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Mon, 6 Jan 2025 16:15:02 -0800 Subject: [PATCH 30/33] fix convertuserToContext util --- packages/experiment-browser/src/util/convert.ts | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/packages/experiment-browser/src/util/convert.ts b/packages/experiment-browser/src/util/convert.ts index 76f633d2..561b9f99 100644 --- a/packages/experiment-browser/src/util/convert.ts +++ b/packages/experiment-browser/src/util/convert.ts @@ -12,11 +12,14 @@ export const convertUserToContext = ( const context: Record = { user: user }; // add page context const globalScope = getGlobalScope(); - context.page = { - // eslint-disable-next-line @typescript-eslint/ban-ts-comment - // @ts-ignore - url: user.currentUrl ?? globalScope?.location.href, - }; + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + const currentUrl = user.currentUrl || globalScope?.location.href; + if (currentUrl) { + context.page = { + url: currentUrl, + }; + } const groups: Record> = {}; if (!user.groups) { return context; From 46cb1ced045f725d0cacd9415e66e6c5edd8084f Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Mon, 6 Jan 2025 16:21:15 -0800 Subject: [PATCH 31/33] remove test --- packages/experiment-tag/test/live.test.ts | 19 ------------------- 1 file changed, 19 deletions(-) delete mode 100644 packages/experiment-tag/test/live.test.ts diff --git a/packages/experiment-tag/test/live.test.ts b/packages/experiment-tag/test/live.test.ts deleted file mode 100644 index 7cb76bbb..00000000 --- a/packages/experiment-tag/test/live.test.ts +++ /dev/null @@ -1,19 +0,0 @@ -import { initializeExperiment } from 'src/experiment'; - -import { createMutateFlag } from './util/create-flag'; - -describe('live', () => { - test('live remote', async () => { - const initialRemote = JSON.stringify([ - createMutateFlag('tim-test-web-remote', 'control', [], [], [], 'remote'), - ]); - // Test that the flag is live and the treatment is returned - await initializeExperiment( - 'client-DvWljIjiiuqLbyjqdvBaLFfEBrAvGuA3', - initialRemote, - { - flagsServerUrl: 'http://localhost:3034', - }, - ); - }); -}); From d6415e4c8e33a3957222f189f5249904d21f5d29 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Tue, 7 Jan 2025 10:34:12 -0800 Subject: [PATCH 32/33] refactor initial flag parsing --- packages/experiment-tag/src/experiment.ts | 14 ++++++-------- 1 file changed, 6 insertions(+), 8 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index c5296cf8..aee8d6e9 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -64,14 +64,6 @@ export class WebExperiment { this.apiKey = apiKey; this.config = config; this.initialFlags = JSON.parse(initialFlags); - this.initialFlags.forEach((flag: EvaluationFlag) => { - const variants = flag.variants; - this.flagVariantMap[flag.key] = {}; - Object.keys(variants).forEach((variantKey) => { - this.flagVariantMap[flag.key][variantKey] = - convertEvaluationVariantToVariant(flag.variants[variantKey]); - }); - }); if (this.globalScope?.webExperiment) { return; } @@ -124,6 +116,12 @@ export class WebExperiment { this.initialFlags.forEach((flag: EvaluationFlag) => { const { key, variants, segments, metadata = {} } = flag; + this.flagVariantMap[key] = {}; + Object.keys(variants).forEach((variantKey) => { + this.flagVariantMap[key][variantKey] = + convertEvaluationVariantToVariant(variants[variantKey]); + }); + // Force variant if in preview mode if ( urlParams['PREVIEW'] && From fabea174694aa27ef55a8792ce195e35483f3090 Mon Sep 17 00:00:00 2001 From: tyiuhc Date: Thu, 16 Jan 2025 16:29:10 -0800 Subject: [PATCH 33/33] fix lint --- packages/experiment-tag/test/experiment.test.ts | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index 2d5d5f11..8607efb8 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -2,7 +2,11 @@ import * as experimentCore from '@amplitude/experiment-core'; import { safeGlobal } from '@amplitude/experiment-core'; import { ExperimentClient } from '@amplitude/experiment-js-client'; import { Base64 } from 'js-base64'; -import {PAGE_IS_EXCLUDED_SEGMENT_NAME, PAGE_NOT_TARGETED_SEGMENT_NAME, WebExperiment} from 'src/experiment'; +import { + PAGE_IS_EXCLUDED_SEGMENT_NAME, + PAGE_NOT_TARGETED_SEGMENT_NAME, + WebExperiment, +} from 'src/experiment'; import * as util from 'src/util'; import { stringify } from 'ts-jest';