From 00515cb1721c66a5f5fb6dcd5025355afc8b0291 Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Thu, 17 Oct 2024 17:23:48 +0900 Subject: [PATCH 1/7] Use hot reload for CSS liquid assets --- .../cli/utilities/theme-environment/hot-reload/server.test.ts | 3 ++- .../src/cli/utilities/theme-environment/hot-reload/server.ts | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/packages/theme/src/cli/utilities/theme-environment/hot-reload/server.test.ts b/packages/theme/src/cli/utilities/theme-environment/hot-reload/server.test.ts index 52e7cc1662..0f70019b37 100644 --- a/packages/theme/src/cli/utilities/theme-environment/hot-reload/server.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/hot-reload/server.test.ts @@ -165,7 +165,8 @@ describe('hot-reload server', () => { // Emits a CSS HotReload event after syncing: expect(cssLiquidSyncSpy).toHaveBeenCalled() await nextTick() - expect(hotReloadEvents.at(-1)).toMatch(`data: {"type":"full","key":"${cssLiquidFileKey}"}`) + // Removes the `.liquid` extension before sending it to the browser: + expect(hotReloadEvents.at(-1)).toMatch(`data: {"type":"css","key":"${cssLiquidFileKey.replace('.liquid', '')}"}`) // -- Updates other files: const jsFileKey = 'assets/something.js' diff --git a/packages/theme/src/cli/utilities/theme-environment/hot-reload/server.ts b/packages/theme/src/cli/utilities/theme-environment/hot-reload/server.ts index 9f9b647f34..28ed15f032 100644 --- a/packages/theme/src/cli/utilities/theme-environment/hot-reload/server.ts +++ b/packages/theme/src/cli/utilities/theme-environment/hot-reload/server.ts @@ -98,7 +98,7 @@ export function setupInMemoryTemplateWatcher(ctx: DevServerContext) { if (isAsset) { if (extension === '.liquid') { // If the asset is a .css.liquid or similar, we wait until it's been synced: - onSync(() => triggerHotReload(fileKey, ctx)) + onSync(() => triggerHotReload(fileKey.replace(extension, ''), ctx)) } else { // Otherwise, just full refresh directly: triggerHotReload(fileKey, ctx) From 6dd1c5715bb8689317bf5093242a2934b7f58dfd Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Thu, 17 Oct 2024 17:24:52 +0900 Subject: [PATCH 2/7] Handle reloading when new liquid CSS assets are added --- .../cli/utilities/theme-environment/hot-reload/client.ts | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/packages/theme/src/cli/utilities/theme-environment/hot-reload/client.ts b/packages/theme/src/cli/utilities/theme-environment/hot-reload/client.ts index 9bc9f07594..01af94ffcb 100644 --- a/packages/theme/src/cli/utilities/theme-environment/hot-reload/client.ts +++ b/packages/theme/src/cli/utilities/theme-environment/hot-reload/client.ts @@ -64,8 +64,10 @@ function hotReloadScript() { const refreshHTMLLinkElements = (elements: HTMLLinkElement[]) => { for (const element of elements) { - // The `href` property prepends the host to the pathname. Use attributes instead: - element.setAttribute('href', element.getAttribute('href')!.replace(/v=\d+$/, `v=${Date.now()}`)) + // The `href` property prepends the host to the pathname. Use attributes instead. + // Note: when a .liquid asset is requested but not found in SFR, it will be rendered as + // `.../assets/file.css?1234` instead of `.../assets/file.css?v=1234`. Ensure we target both. + element.setAttribute('href', element.getAttribute('href')!.replace(/(\?|&)(?:v=)?\d+$/, `$1v=${Date.now()}`)) } } From a7f7dc3002a66463a914a554f70c338b729f96e0 Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Thu, 17 Oct 2024 17:25:38 +0900 Subject: [PATCH 3/7] Changesets --- .changeset/empty-hats-own.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/empty-hats-own.md diff --git a/.changeset/empty-hats-own.md b/.changeset/empty-hats-own.md new file mode 100644 index 0000000000..9671a4bc6d --- /dev/null +++ b/.changeset/empty-hats-own.md @@ -0,0 +1,5 @@ +--- +'@shopify/theme': patch +--- + +Fix hot reloading of `.css.liquid` assets. From 8190e8e467ec0c6232022e97aff8995c004e1d05 Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Thu, 17 Oct 2024 17:48:35 +0900 Subject: [PATCH 4/7] Handle reloading when new liquid JS assets are added --- .../theme/src/cli/utilities/theme-environment/proxy.ts | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/packages/theme/src/cli/utilities/theme-environment/proxy.ts b/packages/theme/src/cli/utilities/theme-environment/proxy.ts index 6b7c9401f0..9aa06da07a 100644 --- a/packages/theme/src/cli/utilities/theme-environment/proxy.ts +++ b/packages/theme/src/cli/utilities/theme-environment/proxy.ts @@ -233,6 +233,15 @@ function proxyStorefrontRequest(event: H3Event, ctx: DevServerContext) { const path = event.path.replaceAll(EXTENSION_CDN_PREFIX, '/') const host = event.path.startsWith(EXTENSION_CDN_PREFIX) ? 'cdn.shopify.com' : ctx.session.storeFqdn const url = new URL(path, `https://${host}`) + + // When a .css.liquid or .js.liquid file is requested but it doesn't exist in SFR, + // it will be rendered with a query string like `assets/file.css?1234`. + // For some reason, after refreshing, this rendered URL keeps the wrong `?1234` + // query string for a while. We replace it with a proper timestamp here to fix it. + if (/\/assets\/[^/]+\.(css|js)$/.test(url.pathname) && /\?\d+$/.test(url.search)) { + url.search = `?v=${Date.now()}` + } + url.searchParams.set('_fd', '0') url.searchParams.set('pb', '0') const headers = getProxyStorefrontHeaders(event) From a9cf87e8c9c4c2cca8a1ba69923fe78651d5aadd Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Thu, 17 Oct 2024 17:48:44 +0900 Subject: [PATCH 5/7] Changesets --- .changeset/empty-hats-own.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/empty-hats-own.md b/.changeset/empty-hats-own.md index 9671a4bc6d..21d80658bc 100644 --- a/.changeset/empty-hats-own.md +++ b/.changeset/empty-hats-own.md @@ -2,4 +2,4 @@ '@shopify/theme': patch --- -Fix hot reloading of `.css.liquid` assets. +Fix hot reloading of `.css.liquid` and `.js.liquid` assets. From 47ba1300a4f3e1edc56a3b937c2ed0225bda6dd9 Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Thu, 17 Oct 2024 17:54:21 +0900 Subject: [PATCH 6/7] Add test --- .../theme-environment/theme-environment.test.ts | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) diff --git a/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts b/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts index d81565d0f1..d8155d5962 100644 --- a/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts +++ b/packages/theme/src/cli/utilities/theme-environment/theme-environment.test.ts @@ -347,5 +347,22 @@ describe('setupDevServer', () => { const {body} = await eventPromise expect(body).toMatch(`src: url(/cdn/shop/t/img/assets/font.woff2)`) }) + + test('proxies .js.liquid assets replacing the error query string', async () => { + const fetchStub = vi.fn(() => new Response()) + vi.stubGlobal('fetch', fetchStub) + vi.useFakeTimers() + const now = Date.now() + + const pathname = '/cdn/shop/t/img/assets/file4.js' + const eventPromise = dispatchEvent(`${pathname}?1234`) + await expect(eventPromise).resolves.not.toThrow() + expect(vi.mocked(render)).not.toHaveBeenCalled() + + expect(fetchStub).toHaveBeenCalledWith( + `https://${defaultServerContext.session.storeFqdn}${pathname}?v=${now}&_fd=0&pb=0`, + expect.any(Object), + ) + }) }) }) From 1ceb3ee5a70c69442461ed264e2322c80fc974ae Mon Sep 17 00:00:00 2001 From: Fran Dios Date: Fri, 18 Oct 2024 14:55:14 +0900 Subject: [PATCH 7/7] Remove null assertion to comply with new ESLint rules --- .../src/cli/utilities/theme-environment/hot-reload/client.ts | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/packages/theme/src/cli/utilities/theme-environment/hot-reload/client.ts b/packages/theme/src/cli/utilities/theme-environment/hot-reload/client.ts index 01af94ffcb..4f6e23f5b5 100644 --- a/packages/theme/src/cli/utilities/theme-environment/hot-reload/client.ts +++ b/packages/theme/src/cli/utilities/theme-environment/hot-reload/client.ts @@ -67,7 +67,10 @@ function hotReloadScript() { // The `href` property prepends the host to the pathname. Use attributes instead. // Note: when a .liquid asset is requested but not found in SFR, it will be rendered as // `.../assets/file.css?1234` instead of `.../assets/file.css?v=1234`. Ensure we target both. - element.setAttribute('href', element.getAttribute('href')!.replace(/(\?|&)(?:v=)?\d+$/, `$1v=${Date.now()}`)) + element.setAttribute( + 'href', + (element.getAttribute('href') ?? '').replace(/(\?|&)(?:v=)?\d+$/, `$1v=${Date.now()}`), + ) } }