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/tidy-bats-prove.md
Original file line number Diff line number Diff line change
@@ -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
Original file line number Diff line number Diff line change
Expand Up @@ -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 = '<a href="https://my-store.myshopify.com/cdn/path/to/assets/file1">link</a>'
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, {
Expand Down
8 changes: 4 additions & 4 deletions packages/theme/src/cli/utilities/theme-environment/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -194,13 +194,13 @@ export async function patchRenderingResponse(
rawResponse: Response,
patchCallback?: (html: string) => string | undefined,
): Promise<Response> {
const response = patchProxiedResponseHeaders(ctx, rawResponse)
Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Flipping this to happen earlier makes sure the cookie is available to our session for any following requests.


// 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')
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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': '<div>hello</div>'},
})

// 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())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@ export async function render(session: DevServerSession, context: DevServerRender
response = await fetch(url, {
method: 'POST',
body: bodyParams,
redirect: 'manual',
Copy link
Copy Markdown
Contributor Author

@EvilGenius13 EvilGenius13 May 26, 2026

Choose a reason for hiding this comment

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

This makes the cookie visible by not having node drop it on a redirect.

headers: {
...headers,
...defaultHeaders(),
Expand All @@ -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(),
Expand Down
Loading