Skip to content

Commit 828bf47

Browse files
committed
Fix an issue where json files were returning as text/html instead of application/json
1 parent 52f999f commit 828bf47

File tree

5 files changed

+64
-6
lines changed

5 files changed

+64
-6
lines changed

.changeset/five-points-vanish.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+
Fixed issue with json files being returned as html/text content type in theme dev

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -236,6 +236,17 @@ describe('dev proxy', () => {
236236
expect(canProxyRequest(event)).toBeTruthy()
237237
})
238238

239+
test('should proxy cart.json and other cart-related JSON endpoints', () => {
240+
const cartJsonEvent = createH3Event('GET', '/cart.json')
241+
expect(canProxyRequest(cartJsonEvent)).toBeTruthy()
242+
243+
const collectionsJsonEvent = createH3Event('GET', '/collections.json')
244+
expect(canProxyRequest(collectionsJsonEvent)).toBeTruthy()
245+
246+
const productsJsonEvent = createH3Event('GET', '/products/handle.json')
247+
expect(canProxyRequest(productsJsonEvent)).toBeTruthy()
248+
})
249+
239250
test('should proxy Checkout requests as they are not supported by the SFR client', () => {
240251
const event = createH3Event('GET', '/checkouts/xyz')
241252
expect(canProxyRequest(event)).toBeTruthy()

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,8 +173,14 @@ export async function patchRenderingResponse(
173173
): Promise<Response> {
174174
const response = patchProxiedResponseHeaders(ctx, rawResponse)
175175

176-
// Ensure the content type indicates UTF-8 charset:
177-
response.headers.set('content-type', 'text/html; charset=utf-8')
176+
// Only set HTML content-type for actual HTML responses, preserve JSON content-type:
177+
const originalContentType = rawResponse.headers.get('content-type')
178+
const isJsonResponse = originalContentType?.includes('application/json')
179+
180+
if (!isJsonResponse) {
181+
// Ensure the content type indicates UTF-8 charset for HTML responses:
182+
response.headers.set('content-type', 'text/html; charset=utf-8')
183+
}
178184

179185
let html = await response.text()
180186
html = injectCdnProxy(html, ctx)

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

Lines changed: 32 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,7 @@ describe('render', () => {
4343

4444
// Then
4545
expect(response.status).toEqual(200)
46-
expect(response.headers.get('Content-Type')).toBeNull()
46+
expect(response.headers.get('Content-Type')).toEqual('application/json')
4747
expect(response.headers.get('something')).toEqual('else')
4848
expect(fetch).toHaveBeenCalledWith(
4949
'https://store.myshopify.com/products/1?_fd=0&pb=0',
@@ -59,6 +59,36 @@ describe('render', () => {
5959
)
6060
})
6161

62+
test('preserves Content-Type header for JSON responses', async () => {
63+
// Given
64+
vi.mocked(fetch).mockResolvedValue(
65+
new Response('{"test": "data"}', {headers: {'Content-Type': 'application/json', something: 'else'}}),
66+
)
67+
68+
// When
69+
const response = await render(session, context)
70+
71+
// Then
72+
expect(response.status).toEqual(200)
73+
expect(response.headers.get('Content-Type')).toEqual('application/json')
74+
expect(response.headers.get('something')).toEqual('else')
75+
})
76+
77+
test('removes Content-Type header for non-JSON responses', async () => {
78+
// Given
79+
vi.mocked(fetch).mockResolvedValue(
80+
new Response('<html></html>', {headers: {'Content-Type': 'text/html', something: 'else'}}),
81+
)
82+
83+
// When
84+
const response = await render(session, context)
85+
86+
// Then
87+
expect(response.status).toEqual(200)
88+
expect(response.headers.get('Content-Type')).toBeNull()
89+
expect(response.headers.get('something')).toEqual('else')
90+
})
91+
6292
test('renders using theme access API', async () => {
6393
// Given
6494
vi.mocked(fetch).mockResolvedValue(
@@ -71,7 +101,7 @@ describe('render', () => {
71101

72102
// Then
73103
expect(response.status).toEqual(200)
74-
expect(response.headers.get('Content-Type')).toBeNull()
104+
expect(response.headers.get('Content-Type')).toEqual('application/json')
75105
expect(response.headers.get('something')).toEqual('else')
76106
expect(fetch).toHaveBeenCalledWith(
77107
'https://theme-kit-access.shopifyapps.com/cli/sfr/products/1?_fd=0&pb=0',

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

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,10 +50,16 @@ export async function render(session: DevServerSession, context: DevServerRender
5050
/**
5151
* Theme Access app requests return the 'application/json' content type.
5252
* However, patched renderings will never patch JSON requests; so we're
53-
* consistently discarding the content type.
53+
* only discarding the content type for non-JSON responses that will be patched.
5454
*/
55+
const contentType = response.headers.get('Content-Type')
56+
const isJsonResponse = contentType?.includes('application/json')
57+
5558
response = new Response(response.body, response)
56-
response.headers.delete('Content-Type')
59+
60+
if (!isJsonResponse) {
61+
response.headers.delete('Content-Type')
62+
}
5763

5864
return response
5965
}

0 commit comments

Comments
 (0)