Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/perfkit-contract-tests.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
Comment thread
andguy95 marked this conversation as resolved.
"@shopify/hydrogen": patch
---

We now export the`PERF_KIT_URL` constant; the PerfKit script `data-*` attributes are now memoized. No change to runtime behavior.
225 changes: 225 additions & 0 deletions packages/hydrogen/src/analytics-manager/PerfKit.test.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,225 @@
import {afterEach, beforeEach, describe, expect, it, vi} from 'vitest';
import {cleanup, render} from '@testing-library/react';
import type {ShopAnalytics} from './AnalyticsProvider';
import {AnalyticsEvent} from './events';

// Control the simulated script load state per test without downloading the
// real PerfKit script. `parseGid` stays the real implementation.
let mockScriptStatus: 'loading' | 'done' | 'error' = 'loading';
const useLoadScriptMock = vi.fn(
(_url: string, _options?: unknown) => mockScriptStatus,
);

vi.mock('@shopify/hydrogen-react', async (importOriginal) => {
const actual =
await importOriginal<typeof import('@shopify/hydrogen-react')>();
return {
...actual,
useLoadScript: (url: string, options?: unknown) =>
useLoadScriptMock(url, options),
};
});

// Mock the analytics context so we can directly inspect registration,
// readiness, and subscriptions.
const subscribeMock = vi.fn();
const readyMock = vi.fn();
const registerMock = vi.fn(() => ({ready: readyMock}));

vi.mock('./AnalyticsProvider', () => ({
useAnalytics: () => ({
subscribe: subscribeMock,
register: registerMock,
}),
}));

// Imported after the mocks above are declared.
import {PerfKit, PERF_KIT_URL} from './PerfKit';

const SHOP: ShopAnalytics = {
shopId: 'gid://shopify/Shop/12345',
acceptedLanguage: 'EN' as ShopAnalytics['acceptedLanguage'],
currency: 'USD' as ShopAnalytics['currency'],
hydrogenSubchannelId: 'storefront-67890',
};

function getSubscribedCallback(event: string): (() => void) | undefined {
const call = (subscribeMock.mock.calls as Array<[string, () => void]>).find(
([subscribedEvent]) => subscribedEvent === event,
);
return call?.[1];
}

function getLoadScriptAttributes(): Record<string, string> {
const lastCall =
useLoadScriptMock.mock.calls[useLoadScriptMock.mock.calls.length - 1];
return (lastCall?.[1] as {attributes: Record<string, string>}).attributes;
}

describe('<PerfKit />', () => {
beforeEach(() => {
mockScriptStatus = 'loading';
subscribeMock.mockClear();
readyMock.mockClear();
registerMock.mockClear();
useLoadScriptMock.mockClear();
// @ts-expect-error - reset injected global between tests
delete window.PerfKit;
});

afterEach(() => {
cleanup();
});

describe('script contract', () => {
it('requests the pinned PerfKit SPA script URL', () => {
mockScriptStatus = 'done';
render(<PerfKit shop={SHOP} />);

expect(useLoadScriptMock).toHaveBeenCalled();
expect(useLoadScriptMock.mock.calls[0][0]).toBe(PERF_KIT_URL);
// Pinned exactly — bumping PerfKit's URL/version must be a deliberate,
// reviewed change that updates this assertion.
expect(PERF_KIT_URL).toBe(
'https://cdn.shopify.com/shopifycloud/perf-kit/shopify-perf-kit-spa.min.js',
);
});

it('passes the required data-* attributes exactly', () => {
render(<PerfKit shop={SHOP} />);

expect(getLoadScriptAttributes()).toEqual({

Check failure on line 91 in packages/hydrogen/src/analytics-manager/PerfKit.test.tsx

View workflow job for this annotation

GitHub Actions / ⬣ Unit tests

src/analytics-manager/PerfKit.test.tsx > <PerfKit /> > script contract > passes the required data-* attributes exactly

AssertionError: expected { id: 'perfkit', …(6) } to deeply equal { id: 'perfkit', …(6) } - Expected + Received { "data-application": "hydrogen", "data-monorail-region": "global", - "data-resource-timing-sampling-rate": "100", + "data-resource-timing-sampling-rate": "10", "data-shop-id": "12345", "data-spa-mode": "true", "data-storefront-id": "storefront-67890", "id": "perfkit", } ❯ src/analytics-manager/PerfKit.test.tsx:91:41
id: 'perfkit',
'data-application': 'hydrogen',
'data-shop-id': '12345',
'data-storefront-id': 'storefront-67890',
'data-monorail-region': 'global',
'data-spa-mode': 'true',
'data-resource-timing-sampling-rate': '100',
});
});

it('parses the shop id from the gid', () => {
render(<PerfKit shop={SHOP} />);
expect(getLoadScriptAttributes()['data-shop-id']).toBe('12345');
});

it('uses shop.hydrogenSubchannelId for the storefront id', () => {
render(<PerfKit shop={SHOP} />);
expect(getLoadScriptAttributes()['data-storefront-id']).toBe(
'storefront-67890',
);
});
});

describe('subscription wiring', () => {
it('registers Internal_Shopify_Perf_Kit', () => {
render(<PerfKit shop={SHOP} />);
expect(registerMock).toHaveBeenCalledWith('Internal_Shopify_Perf_Kit');
});

it('does not wire subscriptions while the script status is loading', () => {
mockScriptStatus = 'loading';
render(<PerfKit shop={SHOP} />);

expect(subscribeMock).not.toHaveBeenCalled();
expect(readyMock).not.toHaveBeenCalled();
});

it('does not wire subscriptions when the script status is error', () => {
mockScriptStatus = 'error';
render(<PerfKit shop={SHOP} />);

expect(subscribeMock).not.toHaveBeenCalled();
expect(readyMock).not.toHaveBeenCalled();
});

it('wires all five view subscriptions only after the script is done', () => {
mockScriptStatus = 'done';
render(<PerfKit shop={SHOP} />);

const subscribedEvents = (
subscribeMock.mock.calls as Array<[string, () => void]>
).map(([event]) => event);

expect(subscribedEvents).toEqual(
expect.arrayContaining([
AnalyticsEvent.PAGE_VIEWED,
AnalyticsEvent.PRODUCT_VIEWED,
AnalyticsEvent.COLLECTION_VIEWED,
AnalyticsEvent.SEARCH_VIEWED,
AnalyticsEvent.CART_VIEWED,
]),
);
expect(subscribedEvents).toHaveLength(5);
});

it('calls ready() once, after subscriptions are wired', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking: this test says ready() happens after subscriptions are wired, but it only checks that ready() was called once.

If the implementation moved ready() before the subscribe(...) calls, this test would still pass. Since this is new contract coverage, let's assert the ordering too, using Vitest's mock invocation order.

mockScriptStatus = 'done';
render(<PerfKit shop={SHOP} />);

expect(readyMock).toHaveBeenCalledTimes(1);
});

it('wires once across a loading->done transition and does not re-wire', () => {
// Start at loading: nothing wired yet.
mockScriptStatus = 'loading';
const {rerender} = render(<PerfKit shop={SHOP} />);
expect(subscribeMock).not.toHaveBeenCalled();

// Transition to done: the effect re-runs (scriptStatus dep changed) and
// wires exactly once, setting the loadedEvent guard.
mockScriptStatus = 'done';
rerender(<PerfKit shop={SHOP} />);
expect(subscribeMock).toHaveBeenCalledTimes(5);
expect(readyMock).toHaveBeenCalledTimes(1);

// A subsequent re-render must not re-wire — the loadedEvent.current guard
// is what prevents it once deps stop changing.
rerender(<PerfKit shop={SHOP} />);

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking: this doesn't actually exercise the loadedEvent.current guard.

On the final rerender, the effect deps are unchanged (subscribe, ready, and scriptStatus are the same values), so React doesn't rerun the effect at all. If someone removed the guard, this test would still pass.

Let's force a dependency change after the first successful wiring - e.g. done -> loading -> done - and assert the subscription count stays at 5 on the second done.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

blocking: this does not actually exercise the loadedEvent.current guard.

On the final rerender, the effect deps are unchanged (subscribe, ready, and scriptStatus are the same values), so React does not rerun the effect at all. If someone removed the guard, this test would still pass.

We should force a dependency change after the first successful wiring - e.g. done -> loading -> done - and assert the subscription count stays at 5 on the second done.

expect(subscribeMock).toHaveBeenCalledTimes(5);
expect(readyMock).toHaveBeenCalledTimes(1);
});
});

describe('event -> PerfKit calls', () => {
it('calls window.PerfKit.navigate() on page_viewed', () => {
mockScriptStatus = 'done';
const navigate = vi.fn();
const setPageType = vi.fn();
window.PerfKit = {navigate, setPageType};

render(<PerfKit shop={SHOP} />);
getSubscribedCallback(AnalyticsEvent.PAGE_VIEWED)?.();

expect(navigate).toHaveBeenCalledTimes(1);
});

it.each([
[AnalyticsEvent.PRODUCT_VIEWED, 'product'],
[AnalyticsEvent.COLLECTION_VIEWED, 'collection'],
[AnalyticsEvent.SEARCH_VIEWED, 'search'],
[AnalyticsEvent.CART_VIEWED, 'cart'],
])('calls setPageType for %s', (event, pageType) => {
mockScriptStatus = 'done';
const navigate = vi.fn();
const setPageType = vi.fn();
window.PerfKit = {navigate, setPageType};

render(<PerfKit shop={SHOP} />);
getSubscribedCallback(event)?.();

expect(setPageType).toHaveBeenCalledWith(pageType);
});

it('does not throw when window.PerfKit is absent (script-load race)', () => {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

non-blocking: this only exercises the page_viewed callback while window.PerfKit is absent.

The implementation uses optional chaining for all five callbacks, so it would be nice to iterate through every subscribed callback here and assert none of them throw. Otherwise a future unsafe setPageType(...) callback could slip through while this test still passes.

mockScriptStatus = 'done';
// Intentionally do not assign window.PerfKit.
render(<PerfKit shop={SHOP} />);

expect(() =>
getSubscribedCallback(AnalyticsEvent.PAGE_VIEWED)?.(),
).not.toThrow();
});
});
});
20 changes: 13 additions & 7 deletions packages/hydrogen/src/analytics-manager/PerfKit.tsx
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import {parseGid, useLoadScript} from '@shopify/hydrogen-react';
import {ShopAnalytics, useAnalytics} from './AnalyticsProvider';
import {AnalyticsEvent} from './events';
import {useEffect, useRef} from 'react';
import {useEffect, useMemo, useRef} from 'react';

declare global {
interface Window {
Expand All @@ -12,26 +12,32 @@ declare global {
}
}

// Pin to a version that have SPA support.
const PERF_KIT_URL =
// Pin to a version that has SPA support.
// Exported so contract tests can assert the exact URL.
export const PERF_KIT_URL =
'https://cdn.shopify.com/shopifycloud/perf-kit/shopify-perf-kit-spa.min.js';

export function PerfKit({shop}: {shop: ShopAnalytics}) {
const loadedEvent = useRef(false);
const {subscribe, register} = useAnalytics();
const {ready} = register('Internal_Shopify_Perf_Kit');

const scriptStatus = useLoadScript(PERF_KIT_URL, {
attributes: {
// Memoized so the object identity is stable across renders and so contract
// tests can assert the exact attributes passed to `useLoadScript`.
const attributes = useMemo(
() => ({
id: 'perfkit',
'data-application': 'hydrogen',
'data-shop-id': parseGid(shop.shopId).id.toString(),
'data-storefront-id': shop.hydrogenSubchannelId,
'data-monorail-region': 'global',
'data-spa-mode': 'true',
'data-resource-timing-sampling-rate': '10',
},
Comment thread
fredericoo marked this conversation as resolved.
});
}),
[shop.shopId, shop.hydrogenSubchannelId],
);

const scriptStatus = useLoadScript(PERF_KIT_URL, {attributes});

useEffect(() => {
if (scriptStatus !== 'done' || loadedEvent.current) return;
Expand Down
Loading