Skip to content

Commit 932be3d

Browse files
committed
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 #7344
1 parent 13e143e commit 932be3d

5 files changed

Lines changed: 124 additions & 4 deletions

File tree

.changeset/tidy-bats-prove.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@shopify/theme': patch
3+
---
4+
5+
Fix `theme dev` preview occasionally rendering the live theme by preserving shopify cookie in redirects

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

Lines changed: 61 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -250,6 +250,67 @@ describe('dev proxy', () => {
250250
expect(ctx.session.sessionCookies).toHaveProperty('_shopify_essential', ':AZFbAlZ..yAAH:')
251251
})
252252

253+
test('captures _shopify_essential from Set-Cookie into session on 3xx responses', async () => {
254+
const localCtx = {
255+
...ctx,
256+
session: {storeFqdn: 'my-store.myshopify.com', sessionCookies: {}},
257+
} as unknown as DevServerContext
258+
259+
const redirectResponse = new Response('should-not-be-read', {
260+
status: 302,
261+
headers: {
262+
Location: 'https://my-store.myshopify.com/foo?bar=1',
263+
'Set-Cookie':
264+
'_shopify_essential=ABC123; Domain=my-store.myshopify.com; Path=/; Max-Age=31536000; secure; HttpOnly; SameSite=Lax',
265+
},
266+
})
267+
268+
const patchedResponse = await patchRenderingResponse(localCtx, redirectResponse)
269+
270+
expect(patchedResponse.status).toBe(302)
271+
expect(localCtx.session.sessionCookies).toHaveProperty('_shopify_essential', 'ABC123')
272+
})
273+
274+
test('rewrites Location header from store domain to local path on 3xx responses', async () => {
275+
const localCtx = {
276+
...ctx,
277+
session: {storeFqdn: 'my-store.myshopify.com', sessionCookies: {}},
278+
} as unknown as DevServerContext
279+
280+
const redirectResponse = new Response('should-not-be-read', {
281+
status: 302,
282+
headers: {
283+
Location: 'https://my-store.myshopify.com/foo?bar=1',
284+
},
285+
})
286+
287+
const patchedResponse = await patchRenderingResponse(localCtx, redirectResponse)
288+
289+
expect(patchedResponse.status).toBe(302)
290+
expect(patchedResponse.headers.get('Location')).toBe('/foo?bar=1')
291+
})
292+
293+
test('returns 3xx responses without reading or patching the body', async () => {
294+
const localCtx = {
295+
...ctx,
296+
session: {storeFqdn: 'my-store.myshopify.com', sessionCookies: {}},
297+
} as unknown as DevServerContext
298+
299+
const body = '<a href="https://my-store.myshopify.com/cdn/path/to/assets/file1">link</a>'
300+
const redirectResponse = new Response(body, {
301+
status: 301,
302+
headers: {
303+
Location: 'https://my-store.myshopify.com/foo',
304+
},
305+
})
306+
307+
const patchedResponse = await patchRenderingResponse(localCtx, redirectResponse)
308+
309+
expect(patchedResponse.status).toBe(301)
310+
// CDN injection would rewrite the href; body should be passed through unchanged.
311+
await expect(patchedResponse.text()).resolves.toBe(body)
312+
})
313+
253314
test('handles 304 Not Modified responses without crashing', async () => {
254315
// Create 304 response with no body as per HTTP spec
255316
const notModifiedResponse = new Response(null, {

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

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -194,13 +194,13 @@ export async function patchRenderingResponse(
194194
rawResponse: Response,
195195
patchCallback?: (html: string) => string | undefined,
196196
): Promise<Response> {
197+
const response = patchProxiedResponseHeaders(ctx, rawResponse)
198+
197199
// 3xx responses should be returned
198-
if (rawResponse.status >= 300 && rawResponse.status < 400) {
199-
return rawResponse
200+
if (response.status >= 300 && response.status < 400) {
201+
return response
200202
}
201203

202-
const response = patchProxiedResponseHeaders(ctx, rawResponse)
203-
204204
// Only set HTML content-type for actual HTML responses, preserve JSON content-type:
205205
const originalContentType = rawResponse.headers.get('content-type')
206206
const isJsonResponse = originalContentType?.includes('application/json')

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

Lines changed: 52 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,6 +205,58 @@ describe('render', () => {
205205
)
206206
})
207207

208+
test('does not auto-follow 3xx responses on GET (preserves intermediate Set-Cookie)', async () => {
209+
// Given
210+
vi.mocked(fetch).mockResolvedValue(
211+
new Response('redirected-body-should-not-be-returned', {
212+
status: 302,
213+
headers: {Location: 'https://store.myshopify.com/products/1'},
214+
}),
215+
)
216+
217+
// When
218+
const response = await render(session, context)
219+
220+
// Then
221+
expect(response.status).toEqual(302)
222+
expect(response.headers.get('Location')).toEqual('https://store.myshopify.com/products/1')
223+
expect(fetch).toHaveBeenCalledWith(
224+
expect.any(String),
225+
expect.objectContaining({
226+
method: 'GET',
227+
redirect: 'manual',
228+
}),
229+
)
230+
})
231+
232+
test('does not auto-follow 3xx responses on POST (replaceTemplates branch)', async () => {
233+
// Given
234+
vi.mocked(fetch).mockResolvedValue(
235+
new Response('redirected-body-should-not-be-returned', {
236+
status: 302,
237+
headers: {Location: 'https://store.myshopify.com/products/1'},
238+
}),
239+
)
240+
241+
// When
242+
const response = await render(session, {
243+
...context,
244+
method: 'POST',
245+
replaceTemplates: {'sections/header.liquid': '<div>hello</div>'},
246+
})
247+
248+
// Then
249+
expect(response.status).toEqual(302)
250+
expect(response.headers.get('Location')).toEqual('https://store.myshopify.com/products/1')
251+
expect(fetch).toHaveBeenCalledWith(
252+
expect.any(String),
253+
expect.objectContaining({
254+
method: 'POST',
255+
redirect: 'manual',
256+
}),
257+
)
258+
})
259+
208260
test('renders using query parameters', async () => {
209261
// Given
210262
vi.mocked(fetch).mockResolvedValue(new Response())

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ export async function render(session: DevServerSession, context: DevServerRender
2323
response = await fetch(url, {
2424
method: 'POST',
2525
body: bodyParams,
26+
redirect: 'manual',
2627
headers: {
2728
...headers,
2829
...defaultHeaders(),
@@ -36,6 +37,7 @@ export async function render(session: DevServerSession, context: DevServerRender
3637
// eslint-disable-next-line no-restricted-globals
3738
response = await fetch(url, {
3839
method: context.method,
40+
redirect: 'manual',
3941
headers: {
4042
...headers,
4143
...defaultHeaders(),

0 commit comments

Comments
 (0)