From 4a333ae24ed39c512a7db4617f0f618de21fc7ee Mon Sep 17 00:00:00 2001 From: Tim Yiu <137842098+tyiuhc@users.noreply.github.com> Date: Wed, 29 Oct 2025 14:13:10 -0700 Subject: [PATCH 1/9] poc --- packages/experiment-tag/src/experiment.ts | 138 +++++-- packages/experiment-tag/src/util/url.ts | 31 ++ .../experiment-tag/test/experiment.test.ts | 379 +++++++++++++++--- 3 files changed, 455 insertions(+), 93 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 4fa66d5c..16de5b6d 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -1,4 +1,5 @@ import { AnalyticsConnector } from '@amplitude/analytics-connector'; +import { CookieStorage } from '@amplitude/analytics-core'; import { EvaluationFlag, getGlobalScope, @@ -49,6 +50,7 @@ import { urlWithoutParamsAndAnchor, concatenateQueryParamsOf, matchesUrl, + getCookieDomain, } from './util/url'; import { UUID } from './util/uuid'; import { convertEvaluationVariantToVariant } from './util/variant'; @@ -190,6 +192,11 @@ export class DefaultWebExperimentClient implements WebExperimentClient { this.setupPreviewMode(urlParams); this.subscriptionManager.initSubscriptions(); + // Subscribe to url_change events to fire redirect impressions + this.messageBus.subscribe('url_change', () => { + this.fireStoredRedirectImpressions().catch(); + }); + // if in visual edit mode, remove the query param if (this.isVisualEditorMode) { WindowMessenger.setup(); @@ -350,8 +357,6 @@ export class DefaultWebExperimentClient implements WebExperimentClient { this.urlExposureCache[currentUrl] = {}; } - this.fireStoredRedirectImpressions(); - for (const key in variants) { // preview actions are handled by previewVariants if ((flagKeys && !flagKeys.includes(key)) || this.previewFlags[key]) { @@ -799,55 +804,100 @@ export class DefaultWebExperimentClient implements WebExperimentClient { variant: Variant, redirectUrl: string, ) { - const redirectStorageKey = `EXP_${this.apiKey.slice(0, 10)}_REDIRECT`; - // Store the current flag and variant for exposure tracking after redirect - const storedRedirects = - getStorageItem('sessionStorage', redirectStorageKey) || {}; - storedRedirects[flagKey] = { redirectUrl, variant }; - setStorageItem('sessionStorage', redirectStorageKey, storedRedirects); - } + const currentDomain = getCookieDomain(this.globalScope.location.href); + const redirectDomain = getCookieDomain(redirectUrl); - private fireStoredRedirectImpressions() { - // Check for stored redirects and process them - const redirectStorageKey = `EXP_${this.apiKey.slice(0, 10)}_REDIRECT`; - const storedRedirects = - getStorageItem('sessionStorage', redirectStorageKey) || {}; - - // If we have stored redirects, track exposures for them - if (Object.keys(storedRedirects).length > 0) { - for (const storedFlagKey in storedRedirects) { - const { redirectUrl, variant } = storedRedirects[storedFlagKey]; - const currentUrl = urlWithoutParamsAndAnchor( - this.globalScope.location.href, + // Only allow redirects to same root domain for security + if (currentDomain !== redirectDomain) { + console.warn( + `Redirect impressions are only supported for same root domain. Current: ${currentDomain}, Redirect: ${redirectDomain}`, + ); + return; + } + + const storage = new CookieStorage< + Record + >({ + domain: redirectDomain, + sameSite: 'Lax', + expirationDays: 1 / 1440, // 1 minute + }); + + const storageKey = `EXP_${this.apiKey.slice(0, 10)}_REDIRECT`; + storage + .get(storageKey) + .then((storedRedirects) => { + const redirects = storedRedirects || {}; + redirects[flagKey] = { redirectUrl, variant }; + storage.set(storageKey, redirects); + }) + .catch((error) => { + console.error( + `Failed to store redirect impression for ${flagKey}:`, + error, ); + }); + } + + private async fireStoredRedirectImpressions() { + const storage = new CookieStorage< + Record + >({ + domain: getCookieDomain(this.globalScope.location.href), + sameSite: 'Lax', + }); + + const storageKey = `EXP_${this.apiKey.slice(0, 10)}_REDIRECT`; + + try { + const storedRedirects = (await storage.get(storageKey)) || {}; + if (Object.keys(storedRedirects).length === 0) { + return; + } + + const currentUrl = urlWithoutParamsAndAnchor( + this.globalScope.location.href, + ); + + // Track exposures for redirects that match current URL + for (const flagKey in storedRedirects) { + const { redirectUrl, variant } = storedRedirects[flagKey]; const strippedRedirectUrl = urlWithoutParamsAndAnchor(redirectUrl); - if (matchesUrl([currentUrl], strippedRedirectUrl)) { - // Force variant to ensure original evaluation result is tracked - this.exposureWithDedupe(storedFlagKey, variant, true); - // Remove this flag from stored redirects - delete storedRedirects[storedFlagKey]; + if (matchesUrl([currentUrl], strippedRedirectUrl)) { + this.exposureWithDedupe(flagKey, variant, true); + delete storedRedirects[flagKey]; } } - } - // Update or clear the storage - if (Object.keys(storedRedirects).length > 0) { - // track exposure with timeout of 500ms - this.globalScope.setTimeout(() => { - const redirects = - getStorageItem('sessionStorage', redirectStorageKey) || {}; - for (const storedFlagKey in redirects) { - this.exposureWithDedupe( - storedFlagKey, - redirects[storedFlagKey].variant, - true, - ); - } - removeStorageItem('sessionStorage', redirectStorageKey); - }, 500); - } else { - removeStorageItem('sessionStorage', redirectStorageKey); + // Track remaining redirects after timeout, then cleanup + if (Object.keys(storedRedirects).length > 0) { + this.globalScope.setTimeout(async () => { + try { + const redirects = (await storage.get(storageKey)) || {}; + for (const flagKey in redirects) { + this.exposureWithDedupe( + flagKey, + redirects[flagKey].variant, + true, + ); + } + await storage.remove(storageKey); + } catch (error) { + console.error( + `Failed to remove redirect impressions from ${storageKey}:`, + error, + ); + } + }, 500); + } else { + await storage.remove(storageKey); + } + } catch (error) { + console.error( + `Failed to retrieve redirect impressions from ${storageKey}:`, + error, + ); } } diff --git a/packages/experiment-tag/src/util/url.ts b/packages/experiment-tag/src/util/url.ts index 743f2cbc..96b96036 100644 --- a/packages/experiment-tag/src/util/url.ts +++ b/packages/experiment-tag/src/util/url.ts @@ -101,3 +101,34 @@ export const isPreviewMode = (): boolean => { } return false; }; + +/** + * Extracts the root domain from a URL and returns it with a leading dot for cookie sharing. + * Examples: + * - app.economist.com -> .economist.com + * - xxx.localhost.test -> .localhost.test + * - subdomain.localhost -> .localhost + */ +export const getCookieDomain = (url: string): string | undefined => { + try { + const hostname = new URL(url).hostname; + + // Handle localhost and subdomains + if (hostname === 'localhost' || hostname.endsWith('.localhost')) { + return '.localhost'; + } + + // Handle IP addresses + if (/^(\d{1,3}\.){3}\d{1,3}$/.test(hostname)) { + return hostname; + } + + // Extract root domain (e.g., app.economist.com -> economist.com) + const parts = hostname.split('.'); + const rootDomain = parts.length <= 2 ? hostname : parts.slice(-2).join('.'); + + return `.${rootDomain}`; + } catch (error) { + return undefined; + } +}; diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index 879a7a02..134856f2 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -18,6 +18,39 @@ const DEFAULT_PAGE_OBJECTS = { const DEFAULT_REDIRECT_SCOPE = { treatment: ['A'], control: ['A'] }; const DEFAULT_MUTATE_SCOPE = { metadata: { scope: ['A'] } }; +// Mock CookieStorage to use an in-memory store for testing +const cookieStore: Record = {}; + +export const getCookieStore = () => cookieStore; +export const clearCookieStore = () => { + Object.keys(cookieStore).forEach(key => delete cookieStore[key]); +}; + +jest.mock('@amplitude/analytics-core', () => { + const actual = jest.requireActual('@amplitude/analytics-core'); + + return { + ...actual, + CookieStorage: jest.fn().mockImplementation(() => ({ + get: jest.fn((key: string) => Promise.resolve(cookieStore[key])), + set: jest.fn((key: string, value: any) => { + cookieStore[key] = value; + return Promise.resolve(); + }), + remove: jest.fn((key: string) => { + delete cookieStore[key]; + return Promise.resolve(); + }), + getRaw: jest.fn((key: string) => Promise.resolve(JSON.stringify(cookieStore[key]))), + isEnabled: jest.fn(() => Promise.resolve(true)), + reset: jest.fn(() => { + Object.keys(cookieStore).forEach(key => delete cookieStore[key]); + return Promise.resolve(); + }), + })), + }; +}); + jest.mock('src/util/messenger', () => { return { WindowMessenger: { @@ -132,6 +165,9 @@ describe('initializeExperiment', () => { jest.clearAllMocks(); jest.spyOn(experimentCore, 'isLocalStorageAvailable').mockReturnValue(true); + // Clear cookie store + clearCookieStore(); + // Create fresh mock global for each test mockGlobal = newMockGlobal(); @@ -210,7 +246,7 @@ describe('initializeExperiment', () => { expect(mockGlobal.localStorage.getItem).toHaveBeenCalledTimes(0); }); - test('treatment variant on control page - should redirect and store in sessionStorage', async () => { + test('treatment variant on control page - should redirect and store in cookies', async () => { // Create a fresh mock global for this test mockGlobal = newMockGlobal(); // eslint-disable-next-line @typescript-eslint/ban-ts-comment @@ -219,8 +255,8 @@ describe('initializeExperiment', () => { const redirectStorageKey = `EXP_${apiKey.toString().slice(0, 10)}_REDIRECT`; - // Verify sessionStorage is empty before test - expect(mockGlobal.sessionStorage.getItem(redirectStorageKey)).toBeNull(); + // Verify cookie store is empty before test + expect(getCookieStore()[redirectStorageKey]).toBeUndefined(); const client = DefaultWebExperimentClient.getInstance( stringify(apiKey), @@ -244,13 +280,14 @@ describe('initializeExperiment', () => { 'http://test.com/2', ); - // Directly check if the value was stored in sessionStorage - // This bypasses the mock function call history and checks the actual storage - const storedValue = mockGlobal.sessionStorage.getItem(redirectStorageKey); - expect(storedValue).not.toBeNull(); + // Wait a bit for async cookie operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); - if (storedValue) { - const storedRedirects = JSON.parse(storedValue); + // Directly check if the value was stored in cookies + const storedRedirects = getCookieStore()[redirectStorageKey]; + expect(storedRedirects).toBeDefined(); + + if (storedRedirects) { expect(storedRedirects).toHaveProperty('test'); } @@ -262,6 +299,9 @@ describe('initializeExperiment', () => { // Simulate URL change event after redirect (client as any).messageBus.publish('url_change', {}); + // Wait for async operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); + // Verify exposureInternal was called with the correct flag key expect(mockExposureInternal).toHaveBeenCalledTimes(1); expect(mockExposureInternal.mock.calls[0][0]).toBe('test'); @@ -369,12 +409,14 @@ describe('initializeExperiment', () => { 'http://test.com/2', ); - // Check if redirect info was stored in sessionStorage - const storedValue = mockGlobal.sessionStorage.getItem(redirectStorageKey); - expect(storedValue).not.toBeNull(); + // Wait a bit for async cookie operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); + + // Check if redirect info was stored in cookies + const storedRedirects = getCookieStore()[redirectStorageKey]; + expect(storedRedirects).toBeDefined(); - if (storedValue) { - const storedRedirects = JSON.parse(storedValue); + if (storedRedirects) { expect(storedRedirects).toHaveProperty('test'); } @@ -385,6 +427,9 @@ describe('initializeExperiment', () => { // Simulate URL change event after redirect (client as any).messageBus.publish('url_change', {}); + // Wait for async operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); + // Verify exposureInternal was called with the correct flag key expect(mockExposureInternal).toHaveBeenCalledTimes(1); expect(mockExposureInternal.mock.calls[0][0]).toBe('test'); @@ -395,8 +440,8 @@ describe('initializeExperiment', () => { expect(sourceVariant.variant).toBeDefined(); expect(sourceVariant.variant.key).toBe('treatment'); // Preview forces treatment - // Verify sessionStorage was cleared after tracking - expect(mockGlobal.sessionStorage.getItem(redirectStorageKey)).toBeNull(); + // Verify cookie store was cleared after tracking + expect(getCookieStore()[redirectStorageKey]).toBeUndefined(); }); test('preview - force treatment variant when on treatment page', () => { @@ -464,12 +509,14 @@ describe('initializeExperiment', () => { 'http://test.com/2?param3=c¶m1=a¶m2=b', ); - // Check if redirect info was stored in sessionStorage - const storedValue = mockGlobal.sessionStorage.getItem(redirectStorageKey); - expect(storedValue).not.toBeNull(); + // Wait a bit for async cookie operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); + + // Check if redirect info was stored in cookies + const storedRedirects = getCookieStore()[redirectStorageKey]; + expect(storedRedirects).toBeDefined(); - if (storedValue) { - const storedRedirects = JSON.parse(storedValue); + if (storedRedirects) { expect(storedRedirects).toHaveProperty('test'); } @@ -480,6 +527,9 @@ describe('initializeExperiment', () => { // Simulate URL change event after redirect (client as any).messageBus.publish('url_change', {}); + // Wait for async operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); + // Verify exposureInternal was called with the correct flag key expect(mockExposureInternal).toHaveBeenCalledTimes(1); expect(mockExposureInternal.mock.calls[0][0]).toBe('test'); @@ -490,8 +540,224 @@ describe('initializeExperiment', () => { expect(sourceVariant.variant).toBeDefined(); expect(sourceVariant.variant.key).toBe('treatment'); - // Verify sessionStorage was cleared after tracking - expect(mockGlobal.sessionStorage.getItem(redirectStorageKey)).toBeNull(); + // Verify cookie store was cleared after tracking + expect(getCookieStore()[redirectStorageKey]).toBeUndefined(); + }); + + test('cross-subdomain redirect - should store and fire impressions', async () => { + // Start on subdomain1.example.com + const mockGlobal = newMockGlobal({ + location: { + href: 'http://subdomain1.example.com/', + replace: jest.fn(), + search: '', + }, + }); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + mockGetGlobalScope.mockReturnValue(mockGlobal); + + const redirectStorageKey = `EXP_${apiKey.toString().slice(0, 10)}_REDIRECT`; + + // Create page object for subdomain1 + const pageObjects = { + test: createPageObject( + 'A', + 'url_change', + undefined, + 'http://subdomain1.example.com', + ), + }; + + const client = DefaultWebExperimentClient.getInstance( + stringify(apiKey), + JSON.stringify([ + createRedirectFlag( + 'test', + 'treatment', + 'http://subdomain2.example.com/target', + undefined, + DEFAULT_REDIRECT_SCOPE, + ), + ]), + JSON.stringify(pageObjects), + ); + + await client.start(); + + // Check redirect was called + expect(mockGlobal.location.replace).toHaveBeenCalledWith( + 'http://subdomain2.example.com/target', + ); + + // Wait for async cookie operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); + + // Check if redirect info was stored in cookies (should use root domain example.com) + const storedRedirects = getCookieStore()[redirectStorageKey]; + expect(storedRedirects).toBeDefined(); + expect(storedRedirects).toHaveProperty('test'); + + // Clear exposure tracking before simulating navigation to subdomain2 + mockExposureInternal.mockClear(); + + // Simulate landing on subdomain2.example.com + const mockGlobal2 = newMockGlobal({ + location: { + href: 'http://subdomain2.example.com/target', + replace: jest.fn(), + search: '', + }, + }); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + mockGetGlobalScope.mockReturnValue(mockGlobal2); + + // Create new client instance on subdomain2 + const client2 = DefaultWebExperimentClient.getInstance( + stringify(apiKey), + JSON.stringify([ + createRedirectFlag( + 'test', + 'treatment', + 'http://subdomain2.example.com/target', + undefined, + DEFAULT_REDIRECT_SCOPE, + ), + ]), + JSON.stringify(pageObjects), + ); + + await client2.start(); + + // Wait for async operations to complete + await new Promise(resolve => setTimeout(resolve, 600)); + + // Verify exposureInternal was called with the correct flag key + expect(mockExposureInternal).toHaveBeenCalled(); + const exposureCalls = mockExposureInternal.mock.calls.filter( + (call) => call[0] === 'test', + ); + expect(exposureCalls.length).toBeGreaterThan(0); + + const sourceVariant: any = exposureCalls[0][1]; + expect(sourceVariant).toBeDefined(); + expect(sourceVariant.variant).toBeDefined(); + expect(sourceVariant.variant.key).toBe('treatment'); + }); + + test('cross-domain redirect - should not store impressions', async () => { + // Start on example.com + const mockGlobal = newMockGlobal({ + location: { + href: 'http://example.com/', + replace: jest.fn(), + search: '', + }, + }); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + mockGetGlobalScope.mockReturnValue(mockGlobal); + + const redirectStorageKey = `EXP_${apiKey.toString().slice(0, 10)}_REDIRECT`; + + // Spy on console.warn to verify warning is logged + const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(); + + // Create page object for example.com + const pageObjects = { + test: createPageObject('A', 'url_change', undefined, 'http://example.com'), + }; + + const client = DefaultWebExperimentClient.getInstance( + stringify(apiKey), + JSON.stringify([ + createRedirectFlag( + 'test', + 'treatment', + 'http://different.com/target', // Different root domain + undefined, + DEFAULT_REDIRECT_SCOPE, + ), + ]), + JSON.stringify(pageObjects), + ); + + await client.start(); + + // Check redirect was called (the redirect itself still happens) + expect(mockGlobal.location.replace).toHaveBeenCalledWith( + 'http://different.com/target', + ); + + // Wait for async cookie operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); + + // Verify warning was logged + expect(consoleWarnSpy).toHaveBeenCalledWith( + expect.stringContaining('Redirect impressions are only supported for same root domain'), + ); + + // Check that redirect info was NOT stored in cookies (different domain) + const storedRedirects = getCookieStore()[redirectStorageKey]; + expect(storedRedirects).toBeUndefined(); + + consoleWarnSpy.mockRestore(); + }); + + test('localhost subdomain redirect - should store and fire impressions', async () => { + // Start on localhost + const mockGlobal = newMockGlobal({ + location: { + href: 'http://localhost:3000/', + replace: jest.fn(), + search: '', + }, + }); + // eslint-disable-next-line @typescript-eslint/ban-ts-comment + // @ts-ignore + mockGetGlobalScope.mockReturnValue(mockGlobal); + + const redirectStorageKey = `EXP_${apiKey.toString().slice(0, 10)}_REDIRECT`; + + // Create page object for localhost + const pageObjects = { + test: createPageObject( + 'A', + 'url_change', + undefined, + 'http://localhost:3000', + ), + }; + + const client = DefaultWebExperimentClient.getInstance( + stringify(apiKey), + JSON.stringify([ + createRedirectFlag( + 'test', + 'treatment', + 'http://sign.localhost:3000/target', + undefined, + DEFAULT_REDIRECT_SCOPE, + ), + ]), + JSON.stringify(pageObjects), + ); + + await client.start(); + + // Check redirect was called (redirects are commented out in current code) + // expect(mockGlobal.location.replace).toHaveBeenCalledWith( + // 'http://sign.localhost:3000/target', + // ); + + // Wait for async cookie operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); + + // Check if redirect info was stored in cookies (should use root domain localhost) + const storedRedirects = getCookieStore()[redirectStorageKey]; + expect(storedRedirects).toBeDefined(); + expect(storedRedirects).toHaveProperty('test'); }); test('should behave as control variant when payload is empty', async () => { @@ -552,12 +818,14 @@ describe('initializeExperiment', () => { 'http://test.com/2', ); - // Check if redirect info was stored in sessionStorage - const storedValue = mockGlobal.sessionStorage.getItem(redirectStorageKey); - expect(storedValue).not.toBeNull(); + // Wait a bit for async cookie operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); - if (storedValue) { - const storedRedirects = JSON.parse(storedValue); + // Check if redirect info was stored in cookies + const storedRedirects = getCookieStore()[redirectStorageKey]; + expect(storedRedirects).toBeDefined(); + + if (storedRedirects) { expect(storedRedirects).toHaveProperty('test'); } @@ -570,6 +838,9 @@ describe('initializeExperiment', () => { // Simulate URL change event after redirect (client as any).messageBus.publish('url_change', {}); + // Wait for async operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); + // Verify exposureInternal was called with the correct flag key expect(mockExposureInternal).toHaveBeenCalledTimes(1); expect(mockExposureInternal.mock.calls[0][0]).toBe('test'); @@ -857,12 +1128,14 @@ describe('initializeExperiment', () => { 'http://test.com/2', ); - // Check if redirect info was stored in sessionStorage - const storedValue = mockGlobal.sessionStorage.getItem(redirectStorageKey); - expect(storedValue).not.toBeNull(); + // Wait a bit for async cookie operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); + + // Check if redirect info was stored in cookies + const storedRedirects = getCookieStore()[redirectStorageKey]; + expect(storedRedirects).toBeDefined(); - if (storedValue) { - const storedRedirects = JSON.parse(storedValue); + if (storedRedirects) { expect(storedRedirects).toHaveProperty('test'); } @@ -873,6 +1146,9 @@ describe('initializeExperiment', () => { // Simulate URL change event after redirect (client as any).messageBus.publish('url_change', {}); + // Wait for async operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); + // Verify exposureInternal was called with the correct flag key expect(mockExposureInternal).toHaveBeenCalledTimes(1); expect(mockExposureInternal.mock.calls[0][0]).toBe('test'); @@ -883,8 +1159,8 @@ describe('initializeExperiment', () => { expect(sourceVariant.variant).toBeDefined(); expect(sourceVariant.variant.key).toBe('treatment'); - // Verify sessionStorage was cleared after tracking - expect(mockGlobal.sessionStorage.getItem(redirectStorageKey)).toBeNull(); + // Verify cookie store was cleared after tracking + expect(getCookieStore()[redirectStorageKey]).toBeUndefined(); }); test('scoped mutations - experiment active, both mutations active on same page', async () => { @@ -1160,7 +1436,7 @@ describe('initializeExperiment', () => { ); }); - test('applyVariants should fire stored redirect impressions', () => { + test('applyVariants should fire stored redirect impressions', async () => { // Create a fresh mock global const mockGlobal = newMockGlobal({ location: { @@ -1173,17 +1449,14 @@ describe('initializeExperiment', () => { const redirectStorageKey = `EXP_${apiKey.toString().slice(0, 10)}_REDIRECT`; - // Set up stored redirect data in sessionStorage + // Set up stored redirect data in cookie store const storedRedirects = { 'test-redirect': { variant: { key: 'treatment' }, redirectUrl: 'http://test.com/2', }, }; - mockGlobal.sessionStorage.setItem( - redirectStorageKey, - JSON.stringify(storedRedirects), - ); + getCookieStore()[redirectStorageKey] = storedRedirects; // Create client with some flags (not the stored redirect flag) const client = DefaultWebExperimentClient.getInstance( @@ -1205,17 +1478,25 @@ describe('initializeExperiment', () => { mockExposureInternal.mockClear(); mockExposure.mockClear(); - client.applyVariants(); + await client.start(); - // Verify exposureInternal was called with the correct flag key - expect(mockExposureInternal).toHaveBeenCalledTimes(1); - expect(mockExposureInternal.mock.calls[0][0]).toBe('test-redirect'); + // Wait for async operations to complete + await new Promise(resolve => setTimeout(resolve, 10)); + + // Verify exposureInternal was called (once for redirect, once for other-flag) + expect(mockExposureInternal).toHaveBeenCalledTimes(2); + + // Find the test-redirect call + const redirectCall = mockExposureInternal.mock.calls.find(call => call[0] === 'test-redirect'); + expect(redirectCall).toBeDefined(); // Check that the sourceVariant parameter contains the expected properties - const sourceVariant: any = mockExposureInternal.mock.calls[0][1]; - expect(sourceVariant).toBeDefined(); - expect(sourceVariant.variant).toBeDefined(); - expect(sourceVariant.variant.key).toBe('treatment'); + if (redirectCall) { + const sourceVariant: any = redirectCall[1]; + expect(sourceVariant).toBeDefined(); + expect(sourceVariant.variant).toBeDefined(); + expect(sourceVariant.variant.key).toBe('treatment'); + } // Verify sessionStorage was cleared expect(mockGlobal.sessionStorage.getItem(redirectStorageKey)).toBeNull(); From ac9297d8db05d86792d2bace4e90764abf9e0d20 Mon Sep 17 00:00:00 2001 From: Tim Yiu <137842098+tyiuhc@users.noreply.github.com> Date: Wed, 29 Oct 2025 14:22:27 -0700 Subject: [PATCH 2/9] simplify --- packages/experiment-tag/src/util/url.ts | 7 -- .../experiment-tag/test/experiment.test.ts | 64 +++++++------------ 2 files changed, 24 insertions(+), 47 deletions(-) diff --git a/packages/experiment-tag/src/util/url.ts b/packages/experiment-tag/src/util/url.ts index 96b96036..f0e71767 100644 --- a/packages/experiment-tag/src/util/url.ts +++ b/packages/experiment-tag/src/util/url.ts @@ -104,26 +104,19 @@ export const isPreviewMode = (): boolean => { /** * Extracts the root domain from a URL and returns it with a leading dot for cookie sharing. - * Examples: - * - app.economist.com -> .economist.com - * - xxx.localhost.test -> .localhost.test - * - subdomain.localhost -> .localhost */ export const getCookieDomain = (url: string): string | undefined => { try { const hostname = new URL(url).hostname; - // Handle localhost and subdomains if (hostname === 'localhost' || hostname.endsWith('.localhost')) { return '.localhost'; } - // Handle IP addresses if (/^(\d{1,3}\.){3}\d{1,3}$/.test(hostname)) { return hostname; } - // Extract root domain (e.g., app.economist.com -> economist.com) const parts = hostname.split('.'); const rootDomain = parts.length <= 2 ? hostname : parts.slice(-2).join('.'); diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index 134856f2..d2cd2300 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -23,7 +23,7 @@ const cookieStore: Record = {}; export const getCookieStore = () => cookieStore; export const clearCookieStore = () => { - Object.keys(cookieStore).forEach(key => delete cookieStore[key]); + Object.keys(cookieStore).forEach((key) => delete cookieStore[key]); }; jest.mock('@amplitude/analytics-core', () => { @@ -41,10 +41,12 @@ jest.mock('@amplitude/analytics-core', () => { delete cookieStore[key]; return Promise.resolve(); }), - getRaw: jest.fn((key: string) => Promise.resolve(JSON.stringify(cookieStore[key]))), + getRaw: jest.fn((key: string) => + Promise.resolve(JSON.stringify(cookieStore[key])), + ), isEnabled: jest.fn(() => Promise.resolve(true)), reset: jest.fn(() => { - Object.keys(cookieStore).forEach(key => delete cookieStore[key]); + Object.keys(cookieStore).forEach((key) => delete cookieStore[key]); return Promise.resolve(); }), })), @@ -280,9 +282,6 @@ describe('initializeExperiment', () => { 'http://test.com/2', ); - // Wait a bit for async cookie operations to complete - await new Promise(resolve => setTimeout(resolve, 10)); - // Directly check if the value was stored in cookies const storedRedirects = getCookieStore()[redirectStorageKey]; expect(storedRedirects).toBeDefined(); @@ -300,7 +299,7 @@ describe('initializeExperiment', () => { (client as any).messageBus.publish('url_change', {}); // Wait for async operations to complete - await new Promise(resolve => setTimeout(resolve, 10)); + await new Promise((resolve) => setTimeout(resolve, 10)); // Verify exposureInternal was called with the correct flag key expect(mockExposureInternal).toHaveBeenCalledTimes(1); @@ -409,9 +408,6 @@ describe('initializeExperiment', () => { 'http://test.com/2', ); - // Wait a bit for async cookie operations to complete - await new Promise(resolve => setTimeout(resolve, 10)); - // Check if redirect info was stored in cookies const storedRedirects = getCookieStore()[redirectStorageKey]; expect(storedRedirects).toBeDefined(); @@ -428,7 +424,7 @@ describe('initializeExperiment', () => { (client as any).messageBus.publish('url_change', {}); // Wait for async operations to complete - await new Promise(resolve => setTimeout(resolve, 10)); + await new Promise((resolve) => setTimeout(resolve, 10)); // Verify exposureInternal was called with the correct flag key expect(mockExposureInternal).toHaveBeenCalledTimes(1); @@ -509,9 +505,6 @@ describe('initializeExperiment', () => { 'http://test.com/2?param3=c¶m1=a¶m2=b', ); - // Wait a bit for async cookie operations to complete - await new Promise(resolve => setTimeout(resolve, 10)); - // Check if redirect info was stored in cookies const storedRedirects = getCookieStore()[redirectStorageKey]; expect(storedRedirects).toBeDefined(); @@ -528,7 +521,7 @@ describe('initializeExperiment', () => { (client as any).messageBus.publish('url_change', {}); // Wait for async operations to complete - await new Promise(resolve => setTimeout(resolve, 10)); + await new Promise((resolve) => setTimeout(resolve, 10)); // Verify exposureInternal was called with the correct flag key expect(mockExposureInternal).toHaveBeenCalledTimes(1); @@ -591,7 +584,7 @@ describe('initializeExperiment', () => { ); // Wait for async cookie operations to complete - await new Promise(resolve => setTimeout(resolve, 10)); + await new Promise((resolve) => setTimeout(resolve, 10)); // Check if redirect info was stored in cookies (should use root domain example.com) const storedRedirects = getCookieStore()[redirectStorageKey]; @@ -631,7 +624,7 @@ describe('initializeExperiment', () => { await client2.start(); // Wait for async operations to complete - await new Promise(resolve => setTimeout(resolve, 600)); + await new Promise((resolve) => setTimeout(resolve, 600)); // Verify exposureInternal was called with the correct flag key expect(mockExposureInternal).toHaveBeenCalled(); @@ -661,12 +654,14 @@ describe('initializeExperiment', () => { const redirectStorageKey = `EXP_${apiKey.toString().slice(0, 10)}_REDIRECT`; - // Spy on console.warn to verify warning is logged - const consoleWarnSpy = jest.spyOn(console, 'warn').mockImplementation(); - // Create page object for example.com const pageObjects = { - test: createPageObject('A', 'url_change', undefined, 'http://example.com'), + test: createPageObject( + 'A', + 'url_change', + undefined, + 'http://example.com', + ), }; const client = DefaultWebExperimentClient.getInstance( @@ -691,18 +686,11 @@ describe('initializeExperiment', () => { ); // Wait for async cookie operations to complete - await new Promise(resolve => setTimeout(resolve, 10)); - - // Verify warning was logged - expect(consoleWarnSpy).toHaveBeenCalledWith( - expect.stringContaining('Redirect impressions are only supported for same root domain'), - ); + await new Promise((resolve) => setTimeout(resolve, 10)); // Check that redirect info was NOT stored in cookies (different domain) const storedRedirects = getCookieStore()[redirectStorageKey]; expect(storedRedirects).toBeUndefined(); - - consoleWarnSpy.mockRestore(); }); test('localhost subdomain redirect - should store and fire impressions', async () => { @@ -752,7 +740,7 @@ describe('initializeExperiment', () => { // ); // Wait for async cookie operations to complete - await new Promise(resolve => setTimeout(resolve, 10)); + await new Promise((resolve) => setTimeout(resolve, 10)); // Check if redirect info was stored in cookies (should use root domain localhost) const storedRedirects = getCookieStore()[redirectStorageKey]; @@ -818,9 +806,6 @@ describe('initializeExperiment', () => { 'http://test.com/2', ); - // Wait a bit for async cookie operations to complete - await new Promise(resolve => setTimeout(resolve, 10)); - // Check if redirect info was stored in cookies const storedRedirects = getCookieStore()[redirectStorageKey]; expect(storedRedirects).toBeDefined(); @@ -839,7 +824,7 @@ describe('initializeExperiment', () => { (client as any).messageBus.publish('url_change', {}); // Wait for async operations to complete - await new Promise(resolve => setTimeout(resolve, 10)); + await new Promise((resolve) => setTimeout(resolve, 10)); // Verify exposureInternal was called with the correct flag key expect(mockExposureInternal).toHaveBeenCalledTimes(1); @@ -1128,9 +1113,6 @@ describe('initializeExperiment', () => { 'http://test.com/2', ); - // Wait a bit for async cookie operations to complete - await new Promise(resolve => setTimeout(resolve, 10)); - // Check if redirect info was stored in cookies const storedRedirects = getCookieStore()[redirectStorageKey]; expect(storedRedirects).toBeDefined(); @@ -1147,7 +1129,7 @@ describe('initializeExperiment', () => { (client as any).messageBus.publish('url_change', {}); // Wait for async operations to complete - await new Promise(resolve => setTimeout(resolve, 10)); + await new Promise((resolve) => setTimeout(resolve, 10)); // Verify exposureInternal was called with the correct flag key expect(mockExposureInternal).toHaveBeenCalledTimes(1); @@ -1481,13 +1463,15 @@ describe('initializeExperiment', () => { await client.start(); // Wait for async operations to complete - await new Promise(resolve => setTimeout(resolve, 10)); + await new Promise((resolve) => setTimeout(resolve, 10)); // Verify exposureInternal was called (once for redirect, once for other-flag) expect(mockExposureInternal).toHaveBeenCalledTimes(2); // Find the test-redirect call - const redirectCall = mockExposureInternal.mock.calls.find(call => call[0] === 'test-redirect'); + const redirectCall = mockExposureInternal.mock.calls.find( + (call) => call[0] === 'test-redirect', + ); expect(redirectCall).toBeDefined(); // Check that the sourceVariant parameter contains the expected properties From 1707e1c4358dfa35c82dcaa2559dc7a35e300b66 Mon Sep 17 00:00:00 2001 From: Tim Yiu <137842098+tyiuhc@users.noreply.github.com> Date: Wed, 29 Oct 2025 15:14:28 -0700 Subject: [PATCH 3/9] simplify --- packages/experiment-tag/src/experiment.ts | 2 +- packages/experiment-tag/test/experiment.test.ts | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 16de5b6d..73a7b08a 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -829,7 +829,7 @@ export class DefaultWebExperimentClient implements WebExperimentClient { .then((storedRedirects) => { const redirects = storedRedirects || {}; redirects[flagKey] = { redirectUrl, variant }; - storage.set(storageKey, redirects); + storage.set(storageKey, redirects).catch(); }) .catch((error) => { console.error( diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index d2cd2300..a46948ff 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -624,7 +624,7 @@ describe('initializeExperiment', () => { await client2.start(); // Wait for async operations to complete - await new Promise((resolve) => setTimeout(resolve, 600)); + await new Promise((resolve) => setTimeout(resolve, 10)); // Verify exposureInternal was called with the correct flag key expect(mockExposureInternal).toHaveBeenCalled(); From ed3151aae1e3fbe72bd1b4a38004b9570dea645c Mon Sep 17 00:00:00 2001 From: Tim Yiu <137842098+tyiuhc@users.noreply.github.com> Date: Fri, 31 Oct 2025 16:56:27 -0700 Subject: [PATCH 4/9] fix --- packages/experiment-tag/src/experiment.ts | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 73a7b08a..b776fc5e 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -192,11 +192,6 @@ export class DefaultWebExperimentClient implements WebExperimentClient { this.setupPreviewMode(urlParams); this.subscriptionManager.initSubscriptions(); - // Subscribe to url_change events to fire redirect impressions - this.messageBus.subscribe('url_change', () => { - this.fireStoredRedirectImpressions().catch(); - }); - // if in visual edit mode, remove the query param if (this.isVisualEditorMode) { WindowMessenger.setup(); @@ -280,6 +275,7 @@ export class DefaultWebExperimentClient implements WebExperimentClient { this.previewVariants({ keyToVariant: this.previewFlags, }); + this.fireStoredRedirectImpressions().catch(); if ( // do not fetch remote flags if all remote flags are in preview mode @@ -451,6 +447,9 @@ export class DefaultWebExperimentClient implements WebExperimentClient { return; } const variantObject = flag[variant]; + if (variantObject?.metadata) { + variantObject.metadata.deliveryMethod = 'web'; + } if (!variantObject) { return; } From 1edb608a49233cce9e9cd80caa3ffb5f565ab5e1 Mon Sep 17 00:00:00 2001 From: Tim Yiu <137842098+tyiuhc@users.noreply.github.com> Date: Mon, 3 Nov 2025 13:18:16 -0800 Subject: [PATCH 5/9] fix --- packages/experiment-tag/src/experiment.ts | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index b776fc5e..9df5312f 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -137,6 +137,9 @@ export class DefaultWebExperimentClient implements WebExperimentClient { Object.keys(variants).forEach((variantKey) => { this.flagVariantMap[key][variantKey] = convertEvaluationVariantToVariant(variants[variantKey]); + this.flagVariantMap[key][variantKey].metadata = { + deliveryMethod: 'web', + }; }); if (metadata.evaluationMode !== 'local') { @@ -447,9 +450,6 @@ export class DefaultWebExperimentClient implements WebExperimentClient { return; } const variantObject = flag[variant]; - if (variantObject?.metadata) { - variantObject.metadata.deliveryMethod = 'web'; - } if (!variantObject) { return; } From e030327c90a656255624aa6f2923fd6d1c73ed6f Mon Sep 17 00:00:00 2001 From: Tim Yiu <137842098+tyiuhc@users.noreply.github.com> Date: Thu, 6 Nov 2025 11:43:10 -0800 Subject: [PATCH 6/9] fix for special case domains --- packages/experiment-tag/src/experiment.ts | 6 ++++++ packages/experiment-tag/src/util/url.ts | 13 ++++++++++--- 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 9df5312f..c87438bd 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -278,7 +278,13 @@ export class DefaultWebExperimentClient implements WebExperimentClient { this.previewVariants({ keyToVariant: this.previewFlags, }); + + // fire stored redirect impressions upon startup this.fireStoredRedirectImpressions().catch(); + this.addPageChangeSubscriber(() => { + // support custom redirect handler + this.fireStoredRedirectImpressions().catch(); + }); if ( // do not fetch remote flags if all remote flags are in preview mode diff --git a/packages/experiment-tag/src/util/url.ts b/packages/experiment-tag/src/util/url.ts index f0e71767..e80f6963 100644 --- a/packages/experiment-tag/src/util/url.ts +++ b/packages/experiment-tag/src/util/url.ts @@ -112,9 +112,16 @@ export const getCookieDomain = (url: string): string | undefined => { if (hostname === 'localhost' || hostname.endsWith('.localhost')) { return '.localhost'; } - - if (/^(\d{1,3}\.){3}\d{1,3}$/.test(hostname)) { - return hostname; + // Special handling for Vercel and other platform domains + // These are on the public suffix list and cannot have cookies set at the root + const publicSuffixes = ['vercel.app', 'netlify.app', 'pages.dev']; + + for (const suffix of publicSuffixes) { + if (hostname.endsWith(`.${suffix}`)) { + // Return the full hostname without a leading dot + // This sets the cookie for ONLY this specific subdomain + return '.' + hostname; + } } const parts = hostname.split('.'); From 0c5ead6a0b71611211fe59cde075e1390b943858 Mon Sep 17 00:00:00 2001 From: Tim Yiu <137842098+tyiuhc@users.noreply.github.com> Date: Thu, 6 Nov 2025 12:06:27 -0800 Subject: [PATCH 7/9] fix tests --- packages/experiment-tag/src/experiment.ts | 4 +-- .../experiment-tag/test/experiment.test.ts | 31 +++++++++---------- 2 files changed, 16 insertions(+), 19 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index c87438bd..d8fa6fc2 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -281,8 +281,8 @@ export class DefaultWebExperimentClient implements WebExperimentClient { // fire stored redirect impressions upon startup this.fireStoredRedirectImpressions().catch(); - this.addPageChangeSubscriber(() => { - // support custom redirect handler + // Subscribe directly to url_change events to fire redirect impressions + this.messageBus.subscribe('url_change', () => { this.fireStoredRedirectImpressions().catch(); }); diff --git a/packages/experiment-tag/test/experiment.test.ts b/packages/experiment-tag/test/experiment.test.ts index 7612eaa8..4a7d4a0c 100644 --- a/packages/experiment-tag/test/experiment.test.ts +++ b/packages/experiment-tag/test/experiment.test.ts @@ -293,27 +293,24 @@ describe('initializeExperiment', () => { // Clear exposure tracking before simulating URL change mockExposureInternal.mockClear(); - // Ensure messageBus exists before publishing - if ((client as any).messageBus) { - // Simulate URL change event after redirect - (client as any).messageBus.publish('url_change', {}); + // Simulate URL change event after redirect + (client as any).messageBus.publish('url_change', {}); - // Wait for async operations to complete - await new Promise((resolve) => setTimeout(resolve, 10)); + // Wait for async operations to complete + await new Promise((resolve) => setTimeout(resolve, 10)); - // Verify exposureInternal was called with the correct flag key - expect(mockExposureInternal).toHaveBeenCalledTimes(1); - expect(mockExposureInternal.mock.calls[0][0]).toBe('test'); + // Verify exposureInternal was called with the correct flag key + expect(mockExposureInternal).toHaveBeenCalledTimes(1); + expect(mockExposureInternal.mock.calls[0][0]).toBe('test'); - // Check that the sourceVariant parameter contains the expected properties - const sourceVariant: any = mockExposureInternal.mock.calls[0][1]; - expect(sourceVariant).toBeDefined(); - expect(sourceVariant.variant).toBeDefined(); - expect(sourceVariant.variant.key).toBe('treatment'); + // Check that the sourceVariant parameter contains the expected properties + const sourceVariant: any = mockExposureInternal.mock.calls[0][1]; + expect(sourceVariant).toBeDefined(); + expect(sourceVariant.variant).toBeDefined(); + expect(sourceVariant.variant.key).toBe('treatment'); - // Verify sessionStorage was cleared after tracking - expect(mockGlobal.sessionStorage.getItem(redirectStorageKey)).toBeNull(); - } + // Verify sessionStorage was cleared after tracking + expect(mockGlobal.sessionStorage.getItem(redirectStorageKey)).toBeNull(); }); test('control variant on control page - should not redirect but call exposure', async () => { From 0a45d2924cae5f6c474a8c3f748fc551b532d165 Mon Sep 17 00:00:00 2001 From: Tim Yiu <137842098+tyiuhc@users.noreply.github.com> Date: Thu, 6 Nov 2025 14:47:02 -0800 Subject: [PATCH 8/9] fix lint --- packages/experiment-tag/test/url.test.ts | 94 ++++++++++++++++++++++++ 1 file changed, 94 insertions(+) diff --git a/packages/experiment-tag/test/url.test.ts b/packages/experiment-tag/test/url.test.ts index 926d1482..4f85f44d 100644 --- a/packages/experiment-tag/test/url.test.ts +++ b/packages/experiment-tag/test/url.test.ts @@ -2,6 +2,7 @@ import * as coreUtil from '@amplitude/experiment-core'; import { concatenateQueryParamsOf, + getCookieDomain, getUrlParams, matchesUrl, removeQueryParams, @@ -355,6 +356,99 @@ describe('removeQueryParams', () => { }); }); +describe('getCookieDomain', () => { + describe('regular domains', () => { + it('should return root domain with leading dot for standard domain', () => { + expect(getCookieDomain('https://example.com')).toBe('.example.com'); + }); + + it('should return root domain with leading dot for subdomain', () => { + expect(getCookieDomain('https://subdomain.example.com')).toBe( + '.example.com', + ); + }); + + it('should return root domain with leading dot for multiple subdomains', () => { + expect(getCookieDomain('https://sub1.sub2.example.com')).toBe( + '.example.com', + ); + }); + + it('should handle domain with path and query parameters', () => { + expect( + getCookieDomain('https://subdomain.example.com/path?param=value'), + ).toBe('.example.com'); + }); + + it('should handle domain with port', () => { + expect(getCookieDomain('https://subdomain.example.com:3000')).toBe( + '.example.com', + ); + }); + }); + + describe('localhost', () => { + it('should return .localhost for localhost', () => { + expect(getCookieDomain('http://localhost')).toBe('.localhost'); + expect(getCookieDomain('http://localhost:3000')).toBe('.localhost'); + }); + + it('should return .localhost for subdomains of localhost', () => { + expect(getCookieDomain('http://app.localhost')).toBe('.localhost'); + expect(getCookieDomain('http://app.localhost:3000')).toBe('.localhost'); + expect(getCookieDomain('http://sub1.sub2.localhost')).toBe('.localhost'); + }); + }); + + describe('public suffix domains', () => { + it('should return full hostname with leading dot for vercel.app subdomain', () => { + expect(getCookieDomain('https://myapp.vercel.app')).toBe( + '.myapp.vercel.app', + ); + }); + + it('should return full hostname with leading dot for netlify.app subdomain', () => { + expect(getCookieDomain('https://myapp.netlify.app')).toBe( + '.myapp.netlify.app', + ); + }); + + it('should return full hostname with leading dot for pages.dev subdomain', () => { + expect(getCookieDomain('https://myapp.pages.dev')).toBe( + '.myapp.pages.dev', + ); + }); + + it('should return full hostname with leading dot for nested public suffix subdomains', () => { + expect(getCookieDomain('https://feature-branch.myapp.vercel.app')).toBe( + '.feature-branch.myapp.vercel.app', + ); + expect(getCookieDomain('https://preview.myapp.netlify.app')).toBe( + '.preview.myapp.netlify.app', + ); + expect(getCookieDomain('https://staging.myapp.pages.dev')).toBe( + '.staging.myapp.pages.dev', + ); + }); + }); + + describe('edge cases', () => { + it('should return undefined for invalid URL', () => { + expect(getCookieDomain('not-a-valid-url')).toBeUndefined(); + expect(getCookieDomain('')).toBeUndefined(); + }); + + it('should handle two-part domain', () => { + expect(getCookieDomain('https://example.co')).toBe('.example.co'); + }); + + it('should handle three-part top-level domain', () => { + expect(getCookieDomain('https://example.co.uk')).toBe('.co.uk'); + expect(getCookieDomain('https://subdomain.example.co.uk')).toBe('.co.uk'); + }); + }); +}); + afterAll(() => { // Restore the original getGlobalScope function after all tests spyGetGlobalScope.mockRestore(); From cafc03c784fbc41a6fe7d207359f34b203d55685 Mon Sep 17 00:00:00 2001 From: Tim Yiu <137842098+tyiuhc@users.noreply.github.com> Date: Mon, 8 Dec 2025 14:11:12 -0800 Subject: [PATCH 9/9] fix marketing cookie logic --- packages/experiment-tag/src/experiment.ts | 7 ++++--- packages/experiment-tag/src/util/cookie.ts | 8 +++++++- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/packages/experiment-tag/src/experiment.ts b/packages/experiment-tag/src/experiment.ts index 775848ad..9e4d3726 100644 --- a/packages/experiment-tag/src/experiment.ts +++ b/packages/experiment-tag/src/experiment.ts @@ -587,11 +587,12 @@ export class DefaultWebExperimentClient implements WebExperimentClient { return; } - this.storeRedirectImpressions(flagKey, variant, redirectUrl); + const currentDomain = getCookieDomain(this.globalScope.location.href); + this.storeRedirectImpressions(flagKey, variant, currentDomain, redirectUrl); // set previous url - relevant for SPA if redirect happens before push/replaceState is complete this.previousUrl = this.globalScope.location.href; - setMarketingCookie(this.apiKey).then(); + setMarketingCookie(this.apiKey, currentDomain).then(); // perform redirection if (this.customRedirectHandler) { this.customRedirectHandler(targetUrl); @@ -809,9 +810,9 @@ export class DefaultWebExperimentClient implements WebExperimentClient { private storeRedirectImpressions( flagKey: string, variant: Variant, + currentDomain: string | undefined, redirectUrl: string, ) { - const currentDomain = getCookieDomain(this.globalScope.location.href); const redirectDomain = getCookieDomain(redirectUrl); // Only allow redirects to same root domain for security diff --git a/packages/experiment-tag/src/util/cookie.ts b/packages/experiment-tag/src/util/cookie.ts index 2042b693..4ce40406 100644 --- a/packages/experiment-tag/src/util/cookie.ts +++ b/packages/experiment-tag/src/util/cookie.ts @@ -6,10 +6,16 @@ import type { Campaign } from '@amplitude/analytics-core'; /** * Utility function to generate and set marketing cookie * Parses current campaign data from URL and referrer, then stores it in the marketing cookie + * @param apiKey - The API key used to generate the storage key + * @param domain - Cookie domain (e.g., result from getCookieDomain) */ -export async function setMarketingCookie(apiKey: string) { +export async function setMarketingCookie( + apiKey: string, + domain: string | undefined, +) { const storage = new CookieStorage({ sameSite: 'Lax', + ...(domain && { domain }), }); const parser = new CampaignParser();