Merge pull request #4669 from Shopify/fd-fix-liquid-asset-reload

[Theme] Fix Liquid asset hot reload

.changeset/empty-hats-own.md

@@ -0,0 +1,5 @@
+---
+'@shopify/theme': patch
+---
+
+Fix hot reloading of `.css.liquid` and `.js.liquid` assets.

packages/theme/src/cli/utilities/theme-environment/hot-reload/client.ts

@@ -64,9 +64,13 @@ function hotReloadScript() {
 
  const refreshHTMLLinkElements = (elements: HTMLLinkElement[]) => {
  for (const element of elements) {
- // The `href` property prepends the host to the pathname. Use attributes instead:
- // eslint-disable-next-line @typescript-eslint/no-non-null-assertion
- 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()}`),
+ )
  }
  }
 

packages/theme/src/cli/utilities/theme-environment/hot-reload/server.test.ts

@@ -178,7 +178,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'

packages/theme/src/cli/utilities/theme-environment/hot-reload/server.ts

@@ -97,7 +97,7 @@ export function setupInMemoryTemplateWatcher(ctx: DevServerContext) {
  if (isAsset(fileKey)) {
  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)

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

@@ -235,6 +235,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)

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),
+ )
+ })
  })
 })