From 39533e0a50587a4d3d0cb20171b0de9bc5c333f4 Mon Sep 17 00:00:00 2001 From: Josh Faigan Date: Mon, 25 May 2026 16:58:13 -0400 Subject: [PATCH] fix(theme): preserve dev preview theme across SFR redirects The storefront renderer fetch call was using fetch's default redirect behavior ('follow'), which silently dropped the intermediate Set-Cookie headers that bind _shopify_essential to the dev theme. After SFR migrated preview_theme_id from server-side storage into the cookie (shop/world 2026-03-27), this latent issue surfaced as the dev preview occasionally rendering the live theme. Set redirect: 'manual' on storefront-renderer fetches and patch response headers (Set-Cookie capture + Location rewrite) on 3xx so the browser can follow the redirect cleanly while we keep the dev-bound session cookie. Fixes Shopify/cli#7344 --- .changeset/tidy-bats-prove.md | 5 ++ .../utilities/theme-environment/proxy.test.ts | 61 +++++++++++++++++++ .../cli/utilities/theme-environment/proxy.ts | 8 +-- .../storefront-renderer.test.ts | 52 ++++++++++++++++ .../theme-environment/storefront-renderer.ts | 2 + 5 files changed, 124 insertions(+), 4 deletions(-) create mode 100644 .changeset/tidy-bats-prove.md diff --git a/.changeset/tidy-bats-prove.md b/.changeset/tidy-bats-prove.md new file mode 100644 index 00000000000..8c7f5d51eda --- /dev/null +++ b/.changeset/tidy-bats-prove.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': patch +--- + +Fix `theme dev` preview occasionally rendering the live theme by preserving the Shopify `_shopify_essential` cookie in redirects diff --git a/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts b/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts index 4329aa35ccc..efacdc852ea 100644 --- a/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/proxy.test.ts @@ -250,6 +250,67 @@ describe('dev proxy', () => { expect(ctx.session.sessionCookies).toHaveProperty('_shopify_essential', ':AZFbAlZ..yAAH:') }) + test('captures _shopify_essential from Set-Cookie into session on 3xx responses', async () => { + const localCtx = { + ...ctx, + session: {storeFqdn: 'my-store.myshopify.com', sessionCookies: {}}, + } as unknown as DevServerContext + + const redirectResponse = new Response('should-not-be-read', { + status: 302, + headers: { + Location: 'https://my-store.myshopify.com/foo?bar=1', + 'Set-Cookie': + '_shopify_essential=ABC123; Domain=my-store.myshopify.com; Path=/; Max-Age=31536000; secure; HttpOnly; SameSite=Lax', + }, + }) + + const patchedResponse = await patchRenderingResponse(localCtx, redirectResponse) + + expect(patchedResponse.status).toBe(302) + expect(localCtx.session.sessionCookies).toHaveProperty('_shopify_essential', 'ABC123') + }) + + test('rewrites Location header from store domain to local path on 3xx responses', async () => { + const localCtx = { + ...ctx, + session: {storeFqdn: 'my-store.myshopify.com', sessionCookies: {}}, + } as unknown as DevServerContext + + const redirectResponse = new Response('should-not-be-read', { + status: 302, + headers: { + Location: 'https://my-store.myshopify.com/foo?bar=1', + }, + }) + + const patchedResponse = await patchRenderingResponse(localCtx, redirectResponse) + + expect(patchedResponse.status).toBe(302) + expect(patchedResponse.headers.get('Location')).toBe('/foo?bar=1') + }) + + test('returns 3xx responses without reading or patching the body', async () => { + const localCtx = { + ...ctx, + session: {storeFqdn: 'my-store.myshopify.com', sessionCookies: {}}, + } as unknown as DevServerContext + + const body = 'link' + const redirectResponse = new Response(body, { + status: 301, + headers: { + Location: 'https://my-store.myshopify.com/foo', + }, + }) + + const patchedResponse = await patchRenderingResponse(localCtx, redirectResponse) + + expect(patchedResponse.status).toBe(301) + // CDN injection would rewrite the href; body should be passed through unchanged. + await expect(patchedResponse.text()).resolves.toBe(body) + }) + test('handles 304 Not Modified responses without crashing', async () => { // Create 304 response with no body as per HTTP spec const notModifiedResponse = new Response(null, { diff --git a/packages/theme/src/cli/utilities/theme-environment/proxy.ts b/packages/theme/src/cli/utilities/theme-environment/proxy.ts index 6f4b765da5f..2dfa7f95527 100644 --- a/packages/theme/src/cli/utilities/theme-environment/proxy.ts +++ b/packages/theme/src/cli/utilities/theme-environment/proxy.ts @@ -194,13 +194,13 @@ export async function patchRenderingResponse( rawResponse: Response, patchCallback?: (html: string) => string | undefined, ): Promise { + const response = patchProxiedResponseHeaders(ctx, rawResponse) + // 3xx responses should be returned - if (rawResponse.status >= 300 && rawResponse.status < 400) { - return rawResponse + if (response.status >= 300 && response.status < 400) { + return response } - const response = patchProxiedResponseHeaders(ctx, rawResponse) - // Only set HTML content-type for actual HTML responses, preserve JSON content-type: const originalContentType = rawResponse.headers.get('content-type') const isJsonResponse = originalContentType?.includes('application/json') diff --git a/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.test.ts b/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.test.ts index 87e32234bea..3b75a96391c 100644 --- a/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.test.ts @@ -205,6 +205,58 @@ describe('render', () => { ) }) + test('does not auto-follow 3xx responses on GET (preserves intermediate Set-Cookie)', async () => { + // Given + vi.mocked(fetch).mockResolvedValue( + new Response('redirected-body-should-not-be-returned', { + status: 302, + headers: {Location: 'https://store.myshopify.com/products/1'}, + }), + ) + + // When + const response = await render(session, context) + + // Then + expect(response.status).toEqual(302) + expect(response.headers.get('Location')).toEqual('https://store.myshopify.com/products/1') + expect(fetch).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + method: 'GET', + redirect: 'manual', + }), + ) + }) + + test('does not auto-follow 3xx responses on POST (replaceTemplates branch)', async () => { + // Given + vi.mocked(fetch).mockResolvedValue( + new Response('redirected-body-should-not-be-returned', { + status: 302, + headers: {Location: 'https://store.myshopify.com/products/1'}, + }), + ) + + // When + const response = await render(session, { + ...context, + method: 'POST', + replaceTemplates: {'sections/header.liquid': '
hello
'}, + }) + + // Then + expect(response.status).toEqual(302) + expect(response.headers.get('Location')).toEqual('https://store.myshopify.com/products/1') + expect(fetch).toHaveBeenCalledWith( + expect.any(String), + expect.objectContaining({ + method: 'POST', + redirect: 'manual', + }), + ) + }) + test('renders using query parameters', async () => { // Given vi.mocked(fetch).mockResolvedValue(new Response()) diff --git a/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.ts b/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.ts index 4a61e423558..efd4d654402 100644 --- a/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.ts +++ b/packages/theme/src/cli/utilities/theme-environment/storefront-renderer.ts @@ -23,6 +23,7 @@ export async function render(session: DevServerSession, context: DevServerRender response = await fetch(url, { method: 'POST', body: bodyParams, + redirect: 'manual', headers: { ...headers, ...defaultHeaders(), @@ -36,6 +37,7 @@ export async function render(session: DevServerSession, context: DevServerRender // eslint-disable-next-line no-restricted-globals response = await fetch(url, { method: context.method, + redirect: 'manual', headers: { ...headers, ...defaultHeaders(),