Send preview_theme_id on every request

.changeset/jolly-coins-search.md

@@ -0,0 +1,5 @@
+---
+'@shopify/theme': patch
+---
+
+Fix theme dev preview theme resolution

packages/theme/src/cli/utilities/theme-environment/html.test.ts

@@ -78,6 +78,29 @@ describe('getHtmlHandler', async () => {
     expect(ctx.lastRequestedPath).toStrictEqual('/previous-page')
   })
 
+  test('renders storefront requests with the development theme id', async () => {
+    const handler = getHtmlHandler(theme, ctx)
+    const event = createH3Event('GET', '/products/test?color=blue', {'sec-fetch-mode': 'navigate'})
+
+    vi.mocked(render).mockResolvedValueOnce(
+      new Response('', {
+        status: 200,
+        headers: {'x-request-id': 'test-request-id'},
+      }),
+    )
+
+    await handler(event)
+
+    expect(render).toHaveBeenCalledWith(
+      ctx.session,
+      expect.objectContaining({
+        path: '/products/test',
+        query: [['color', 'blue']],
+        themeId: '123',
+      }),
+    )
+  })
+
   test('the development server session recovers when a theme id mismatch occurs', async () => {
     // Given
     const handler = getHtmlHandler(theme, ctx)
@@ -108,6 +131,29 @@ describe('getHtmlHandler', async () => {
     expect(ctx.session.refresh).toHaveBeenCalled()
   })
 
+  test('the mismatch retry redirect drops a stale preview_theme_id from the browser', async () => {
+    const handler = getHtmlHandler(theme, ctx)
+    const event = createH3Event('GET', '/?preview_theme_id=999&color=blue')
+
+    vi.mocked(render).mockResolvedValueOnce(
+      new Response(
+        `<script>
+          var Shopify = Shopify || {};
+          Shopify.theme = {"name":"Live","id":456,"role":"main"};
+        </script>`,
+        {
+          status: 200,
+          headers: {'x-request-id': 'test-request-id'},
+        },
+      ),
+    )
+
+    const response = await handler(event)
+
+    expect(response.status).toBe(302)
+    expect(response.headers.get('Location')).toBe('/?color=blue')
+  })
+
   test('the development server aborts when max theme id mismatch retries is reached', async () => {
     // Given
     const handler = getHtmlHandler(theme, ctx)

packages/theme/src/cli/utilities/theme-environment/html.ts

@@ -101,10 +101,14 @@ export function getHtmlHandler(theme: Theme, ctx: DevServerContext): EventHandle
 
             /**
              * Filtering out __sfr params to avoid infinite redirects, when in
-             * development mode.
+             * development mode. Also drop preview_theme_id so a stale value
+             * carried over from the browser cannot pin retries to the wrong
+             * theme; the dev server will re-inject the correct id.
              */
             const searchParams = new URLSearchParams(browserSearch)
-            const filteredParams = new URLSearchParams([...searchParams].filter(([key]) => !key.startsWith('__sfr')))
+            const filteredParams = new URLSearchParams(
+              [...searchParams].filter(([key]) => !key.startsWith('__sfr') && key !== 'preview_theme_id'),
+            )
             const location = browserPathname + (filteredParams.toString() ? `?${filteredParams}` : '')
 
             return new Response(null, {

packages/theme/src/cli/utilities/theme-environment/proxy.test.ts

@@ -269,6 +269,20 @@ describe('dev proxy', () => {
       expect(result.headers.get('etag')).toBe('"abc123"')
       expect(result.body).toBeNull()
     })
+
+    test('patches redirect locations and removes internal rendering params', async () => {
+      const redirectResponse = new Response(null, {
+        status: 302,
+        headers: {
+          Location: 'https://my-store.myshopify.com/products/test?_fd=0&pb=0&preview_theme_id=123&variant=1',
+        },
+      })
+
+      const result = await patchRenderingResponse(ctx, redirectResponse)
+
+      expect(result.status).toBe(302)
+      expect(result.headers.get('Location')).toBe('/products/test?variant=1')
+    })
   })
 
   describe('getProxyStorefrontHeaders', () => {

packages/theme/src/cli/utilities/theme-environment/proxy.ts

@@ -183,13 +183,13 @@ export async function patchRenderingResponse(
   rawResponse: Response,
   patchCallback?: (html: string) => string | undefined,
 ): Promise<Response> {
-  // 3xx responses should be returned
+  const response = patchProxiedResponseHeaders(ctx, rawResponse)
+
+  // 3xx responses should be returned after header patching.
   if (rawResponse.status >= 300 && rawResponse.status < 400) {
-    return rawResponse
+    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')
@@ -255,6 +255,7 @@ function patchProxiedResponseHeaders(ctx: DevServerContext, rawResponse: Respons
     if (!CHECKOUT_PATTERN.test(url.pathname)) {
       url.searchParams.delete('_fd')
       url.searchParams.delete('pb')
+      url.searchParams.delete('preview_theme_id')
       response.headers.set('Location', url.href.replace(url.origin, ''))
     }
   }

packages/theme/src/cli/utilities/theme-environment/storefront-renderer.test.ts

@@ -46,7 +46,7 @@ describe('render', () => {
     expect(response.headers.get('Content-Type')).toEqual('application/json')
     expect(response.headers.get('something')).toEqual('else')
     expect(fetch).toHaveBeenCalledWith(
-      'https://store.myshopify.com/products/1?_fd=0&pb=0',
+      'https://store.myshopify.com/products/1?_fd=0&pb=0&preview_theme_id=123',
       expect.objectContaining({
         method: 'GET',
         headers: expect.objectContaining({
@@ -104,7 +104,7 @@ describe('render', () => {
     expect(response.headers.get('Content-Type')).toEqual('application/json')
     expect(response.headers.get('something')).toEqual('else')
     expect(fetch).toHaveBeenCalledWith(
-      'https://theme-kit-access.shopifyapps.com/cli/sfr/products/1?_fd=0&pb=0',
+      'https://theme-kit-access.shopifyapps.com/cli/sfr/products/1?_fd=0&pb=0&preview_theme_id=123',
       expect.objectContaining({
         method: 'GET',
         headers: expect.objectContaining({
@@ -116,7 +116,7 @@ describe('render', () => {
       }),
     )
     expect(fetch).toHaveBeenCalledWith(
-      'https://theme-kit-access.shopifyapps.com/cli/sfr/products/1?_fd=0&pb=0',
+      'https://theme-kit-access.shopifyapps.com/cli/sfr/products/1?_fd=0&pb=0&preview_theme_id=123',
       expect.objectContaining({
         method: 'GET',
         headers: expect.not.objectContaining({
@@ -139,7 +139,7 @@ describe('render', () => {
     // Then
     expect(response.status).toEqual(200)
     expect(fetch).toHaveBeenCalledWith(
-      'https://store.myshopify.com/products/1?_fd=0&pb=0&section_id=sections--1__announcement-bar',
+      'https://store.myshopify.com/products/1?_fd=0&pb=0&preview_theme_id=123&section_id=sections--1__announcement-bar',
       expect.objectContaining({
         method: 'GET',
         headers: expect.objectContaining({
@@ -165,7 +165,7 @@ describe('render', () => {
     // Then
     expect(response.status).toEqual(200)
     expect(fetch).toHaveBeenCalledWith(
-      'https://store.myshopify.com/products/1?_fd=0&pb=0&app_block_id=00001111222233334444',
+      'https://store.myshopify.com/products/1?_fd=0&pb=0&preview_theme_id=123&app_block_id=00001111222233334444',
       expect.objectContaining({
         method: 'GET',
         headers: expect.objectContaining({
@@ -192,7 +192,7 @@ describe('render', () => {
     // Then
     expect(response.status).toEqual(200)
     expect(fetch).toHaveBeenCalledWith(
-      'https://store.myshopify.com/products/1?_fd=0&pb=0&section_id=sections--1__announcement-bar',
+      'https://store.myshopify.com/products/1?_fd=0&pb=0&preview_theme_id=123&section_id=sections--1__announcement-bar',
       expect.objectContaining({
         method: 'GET',
         headers: expect.objectContaining({
@@ -221,7 +221,36 @@ describe('render', () => {
     // Then
     expect(response.status).toEqual(200)
     expect(fetch).toHaveBeenCalledWith(
-      'https://store.myshopify.com/products/1?_fd=0&pb=0&value=A&value=B',
+      'https://store.myshopify.com/products/1?_fd=0&pb=0&value=A&value=B&preview_theme_id=123',
+      expect.objectContaining({
+        method: 'GET',
+        headers: expect.objectContaining({
+          Authorization: 'Bearer token_111222333',
+          Cookie: 'theme_cookie=abc; storefront_digest=00001111222233334444; _shopify_essential=:00112233445566778899:',
+          'Content-Length': '100',
+          'X-Special-Header': '200',
+        }),
+      }),
+    )
+  })
+
+  test('preserves preview_theme_id from query parameters', async () => {
+    // Given
+    vi.mocked(fetch).mockResolvedValue(new Response())
+
+    // When
+    const response = await render(session, {
+      ...context,
+      query: [
+        ['preview_theme_id', '456'],
+        ['value', 'A'],
+      ],
+    })
+
+    // Then
+    expect(response.status).toEqual(200)
+    expect(fetch).toHaveBeenCalledWith(
+      'https://store.myshopify.com/products/1?_fd=0&pb=0&preview_theme_id=456&value=A',
       expect.objectContaining({
         method: 'GET',
         headers: expect.objectContaining({

packages/theme/src/cli/utilities/theme-environment/storefront-renderer.ts

@@ -114,7 +114,10 @@ export function buildCookies(session: DevServerSession, ctx: Pick<DevServerRende
   })
 }
 
-function buildStorefrontUrl(session: DevServerSession, {path, sectionId, appBlockId, query}: DevServerRenderContext) {
+function buildStorefrontUrl(
+  session: DevServerSession,
+  {path, sectionId, appBlockId, query, themeId}: DevServerRenderContext,
+) {
   const baseUrl = buildBaseStorefrontUrl(session)
   const url = `${baseUrl}${path}`
   const params = new URLSearchParams({
@@ -126,6 +129,10 @@ function buildStorefrontUrl(session: DevServerSession, {path, sectionId, appBloc
     params.append(key, value)
   }
 
+  if (!params.has('preview_theme_id')) {
+    params.append('preview_theme_id', themeId)
+  }
+
   // The Section Rendering API takes precendence over the Block Rendering API.
   if (sectionId) {
     params.append('section_id', sectionId)