Fix theme dev serving theme asset for /cdn/extensions/ requests

.changeset/river-theme-dev-cdn-extensions-collision.md

@@ -0,0 +1,5 @@
+---
+'@shopify/theme': patch
+---
+
+Fix `theme dev` serving a same-named theme asset in response to a `/cdn/extensions/...` request. When a theme and an installed app extension shared an asset filename (e.g. `app.js`), the local dev server's theme matcher swallowed the extension URL prefix and returned the theme file. Extension asset requests now fall through to the CDN proxy (or to a locally-developed extension's filesystem) as intended.

packages/theme/src/cli/utilities/theme-environment/local-assets.test.ts

@@ -0,0 +1,99 @@
+import {findLocalFile} from './local-assets.js'
+import {emptyThemeFileSystem, emptyThemeExtFileSystem} from '../theme-fs-empty.js'
+import {describe, expect, test} from 'vitest'
+import {createEvent} from 'h3'
+import {IncomingMessage, ServerResponse} from 'node:http'
+import {Socket} from 'node:net'
+import type {DevServerContext} from './types.js'
+
+function createH3Event(path: string) {
+  const req = new IncomingMessage(new Socket())
+  const res = new ServerResponse(req)
+  req.method = 'GET'
+  req.url = path
+  req.headers = {}
+  return createEvent(req, res)
+}
+
+function buildCtx({
+  themeFiles = [] as [string, unknown][],
+  extFiles = [] as [string, unknown][],
+}): DevServerContext {
+  const themeFs = emptyThemeFileSystem()
+  for (const [key, value] of themeFiles) themeFs.files.set(key, value as never)
+  const extFs = emptyThemeExtFileSystem()
+  for (const [key, value] of extFiles) extFs.files.set(key, value as never)
+  return {localThemeFileSystem: themeFs, localThemeExtensionFileSystem: extFs} as unknown as DevServerContext
+}
+
+describe('findLocalFile', () => {
+  const themeAsset = {checksum: 't', key: 'assets/app.js', value: '// theme app.js'}
+  const extAsset = {checksum: 'e', key: 'assets/app.js', value: '// extension app.js'}
+
+  test('serves a local theme asset for /cdn/<vanity>/assets/<name>', () => {
+    const ctx = buildCtx({themeFiles: [['assets/app.js', themeAsset]]})
+    const result = findLocalFile(createH3Event('/cdn/shop/t/12/assets/app.js?v=1'), ctx)
+
+    expect(result.fileKey).toBe('assets/app.js')
+    expect(result.file).toBe(themeAsset)
+  })
+
+  test('serves a local theme asset for a bare /assets/<name> request', () => {
+    const ctx = buildCtx({themeFiles: [['assets/app.js', themeAsset]]})
+    const result = findLocalFile(createH3Event('/assets/app.js'), ctx)
+
+    expect(result.fileKey).toBe('assets/app.js')
+    expect(result.file).toBe(themeAsset)
+  })
+
+  test('serves a local extension asset for /ext/cdn/extensions/<uuid>/<app>/assets/<name>', () => {
+    const ctx = buildCtx({extFiles: [['assets/app.js', extAsset]]})
+    const result = findLocalFile(
+      createH3Event('/ext/cdn/extensions/019e1813-804f-7f97-ad2d-278904fdd92f/my-app/assets/app.js'),
+      ctx,
+    )
+
+    expect(result.fileKey).toBe('assets/app.js')
+    expect(result.file).toBe(extAsset)
+  })
+
+  test('does not serve a theme asset in response to /cdn/extensions/<uuid>/<app>/assets/<name> (regression)', () => {
+    // A request for an installed-app extension asset must not be answered with a
+    // same-named theme asset. With no local extension file present the request
+    // must fall through (fileKey === undefined) so getProxyHandler can forward
+    // it to cdn.shopify.com.
+    const ctx = buildCtx({themeFiles: [['assets/app.js', themeAsset]]})
+    const result = findLocalFile(
+      createH3Event('/cdn/extensions/019e1813-804f-7f97-ad2d-278904fdd92f/klaviyo-email-marketing-54/assets/app.js'),
+      ctx,
+    )
+
+    expect(result.fileKey).toBeUndefined()
+    expect(result.file).toBeUndefined()
+  })
+
+  test('prefers the local extension asset for /cdn/extensions/... when one exists locally', () => {
+    // Both filesystems have assets/app.js. The extension matcher must win
+    // (and the theme matcher must not match this path at all), so the
+    // extension content is served instead of the theme content.
+    const ctx = buildCtx({
+      themeFiles: [['assets/app.js', themeAsset]],
+      extFiles: [['assets/app.js', extAsset]],
+    })
+    const result = findLocalFile(
+      createH3Event('/cdn/extensions/019e1813-804f-7f97-ad2d-278904fdd92f/my-app/assets/app.js'),
+      ctx,
+    )
+
+    expect(result.fileKey).toBe('assets/app.js')
+    expect(result.file).toBe(extAsset)
+  })
+
+  test('returns no match when neither filesystem has the file', () => {
+    const ctx = buildCtx({})
+    const result = findLocalFile(createH3Event('/cdn/shop/t/12/assets/missing.js'), ctx)
+
+    expect(result.fileKey).toBeUndefined()
+    expect(result.file).toBeUndefined()
+  })
+})

packages/theme/src/cli/utilities/theme-environment/local-assets.ts

@@ -66,7 +66,17 @@ export function getAssetsHandler(_theme: Theme, ctx: DevServerContext) {
   })
 }
 
-function findLocalFile(event: H3Event, ctx: DevServerContext) {
+// Theme assets are served under either a bare `/assets/...` path or the vanity CDN
+// prefix `/cdn/.../assets/...`. The negative lookahead prevents this matcher from
+// claiming `/cdn/extensions/...` paths, which belong to theme app extensions.
+const THEME_ASSET_PATTERN = /^(?:\/cdn\/(?!extensions\/).*?)?\/assets\/([^?]+)/
+
+// Theme app extension assets are served under either `/ext/cdn/extensions/...`
+// (locally-developed extensions, rewritten by `injectCdnProxy` via EXTENSION_CDN_PREFIX)
+// or `/cdn/extensions/...` (storefront-emitted URLs for installed extensions).
+const THEME_EXTENSION_ASSET_PATTERN = /^(?:\/(?:ext\/)?cdn\/extensions\/.*?)?\/assets\/([^?]+)/
+
+export function findLocalFile(event: H3Event, ctx: DevServerContext) {
   const tryGetFile = (pattern: RegExp, fileSystem: VirtualFileSystem) => {
     const matchedFileName = event.path.match(pattern)?.[1]
 
@@ -81,10 +91,13 @@ function findLocalFile(event: H3Event, ctx: DevServerContext) {
     }
   }
 
-  // Try to match theme asset files first and fallback to theme extension asset files
+  // Try to match theme asset files first and fallback to theme extension asset files.
+  // When neither matches (e.g. `/cdn/extensions/<uuid>/<app>/assets/<name>` for an
+  // installed third-party app), the request falls through to `getProxyHandler` which
+  // forwards to cdn.shopify.com.
   return (
-    tryGetFile(/^(?:\/cdn\/.*?)?\/assets\/([^?]+)/, ctx.localThemeFileSystem) ??
-    tryGetFile(/^(?:\/ext\/cdn\/extensions\/.*?)?\/assets\/([^?]+)/, ctx.localThemeExtensionFileSystem) ?? {
+    tryGetFile(THEME_ASSET_PATTERN, ctx.localThemeFileSystem) ??
+    tryGetFile(THEME_EXTENSION_ASSET_PATTERN, ctx.localThemeExtensionFileSystem) ?? {
       isUnsynced: false,
       fileKey: undefined,
       file: undefined,