Merge pull request #7389 from Shopify/04-23-allow_preserving_file_paths_when_including_assets_from_config_key_entry

Allow preserving file paths when including assets from config key entry

Author: vividviolet

packages/app/src/cli/services/build/steps/include-assets-step.ts

@@ -55,6 +55,7 @@ const ConfigKeyEntrySchema = z.object({
   destination: z.string().optional(),
   anchor: z.string().optional(),
   groupBy: z.string().optional(),
+  preserveFilePaths: z.boolean().default(false),
 })
 
 const InclusionEntrySchema = z.discriminatedUnion('type', [PatternEntrySchema, StaticEntrySchema, ConfigKeyEntrySchema])
@@ -126,9 +127,10 @@ export async function executeIncludeAssetsStep(
   const outputDir = resolveOutputDir(extension.outputPath)
 
   const aggregatedPathMap = new Map<string, string | string[]>()
-  // Track basenames written across all configKey entries in this build to detect
-  // true conflicts (different sources, same basename) while allowing overwrites
-  // from previous builds.
+  // Track basenames written across all configKey entries in this build. In
+  // default mode `uniqueBasename` uses this to rename basename collisions; in
+  // `preserveFilePaths` mode it's used to detect cross-entry collisions
+  // (different sources writing the same basename to the bundle).
   const usedBasenames = new Set<string>()
 
   // configKey entries run sequentially to avoid filesystem race conditions
@@ -147,6 +149,7 @@ export async function executeIncludeAssetsStep(
       context,
       destination: sanitizedDest,
       usedBasenames,
+      preserveFilePaths: entry.preserveFilePaths,
     })
     result.pathMap.forEach((val, key) => aggregatedPathMap.set(key, val))
     configKeyCount += result.filesCopied

packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.test.ts

@@ -1,4 +1,5 @@
 import {copyConfigKeyEntry} from './copy-config-key-entry.js'
+import {AbortError} from '@shopify/cli-kit/node/error'
 import {inTemporaryDirectory, writeFile, fileExists, mkdir, readFile} from '@shopify/cli-kit/node/fs'
 import {joinPath} from '@shopify/cli-kit/node/path'
 import {describe, expect, test, vi, beforeEach} from 'vitest'
@@ -206,6 +207,93 @@ describe('copyConfigKeyEntry', () => {
     })
   })
 
+  describe('preserveFilePaths', () => {
+    test('throws when two directory sources produce the same output-relative file', async () => {
+      await inTemporaryDirectory(async (tmpDir) => {
+        const dirA = joinPath(tmpDir, 'a-assets')
+        const dirB = joinPath(tmpDir, 'b-assets')
+        await mkdir(dirA)
+        await mkdir(dirB)
+        await writeFile(joinPath(dirA, 'logo.png'), 'a')
+        await writeFile(joinPath(dirB, 'logo.png'), 'b')
+
+        const outDir = joinPath(tmpDir, 'out')
+        await mkdir(outDir)
+
+        const usedBasenames = new Set<string>()
+        const contextA = makeContext({dir: 'a-assets'}, mockStdout)
+        await copyConfigKeyEntry({
+          key: 'dir',
+          baseDir: tmpDir,
+          outputDir: outDir,
+          context: contextA,
+          usedBasenames,
+          preserveFilePaths: true,
+        })
+
+        const contextB = makeContext({dir: 'b-assets'}, mockStdout)
+        const promise = copyConfigKeyEntry({
+          key: 'dir',
+          baseDir: tmpDir,
+          outputDir: outDir,
+          context: contextB,
+          usedBasenames,
+          preserveFilePaths: true,
+        })
+        await expect(promise).rejects.toThrow(AbortError)
+        await expect(promise).rejects.toThrow(/File collision: 'logo\.png'/)
+      })
+    })
+
+    test('throws when two single-file sources share a basename (no renaming)', async () => {
+      await inTemporaryDirectory(async (tmpDir) => {
+        const dirA = joinPath(tmpDir, 'a')
+        const dirB = joinPath(tmpDir, 'b')
+        await mkdir(dirA)
+        await mkdir(dirB)
+        await writeFile(joinPath(dirA, 'schema.json'), '{"a": true}')
+        await writeFile(joinPath(dirB, 'schema.json'), '{"b": true}')
+
+        const outDir = joinPath(tmpDir, 'out')
+        await mkdir(outDir)
+
+        const context = makeContext({files: ['a/schema.json', 'b/schema.json']}, mockStdout)
+        const promise = copyConfigKeyEntry({
+          key: 'files',
+          baseDir: tmpDir,
+          outputDir: outDir,
+          context,
+          preserveFilePaths: true,
+        })
+        await expect(promise).rejects.toThrow(AbortError)
+        await expect(promise).rejects.toThrow(/File collision: 'schema\.json'/)
+      })
+    })
+
+    test('does not throw when the same directory source is referenced twice (deduped within one call)', async () => {
+      await inTemporaryDirectory(async (tmpDir) => {
+        const srcDir = joinPath(tmpDir, 'assets')
+        await mkdir(srcDir)
+        await writeFile(joinPath(srcDir, 'foo.json'), '{}')
+
+        const outDir = joinPath(tmpDir, 'out')
+        await mkdir(outDir)
+
+        const context = makeContext({extensions: [{targeting: [{assets: 'assets'}, {assets: 'assets'}]}]}, mockStdout)
+        const result = await copyConfigKeyEntry({
+          key: 'extensions[].targeting[].assets',
+          baseDir: tmpDir,
+          outputDir: outDir,
+          context,
+          preserveFilePaths: true,
+        })
+
+        expect(result.filesCopied).toBe(1)
+        await expect(fileExists(joinPath(outDir, 'foo.json'))).resolves.toBe(true)
+      })
+    })
+  })
+
   test('deduplicates repeated source paths — copies each unique path only once', async () => {
     await inTemporaryDirectory(async (tmpDir) => {
       await writeFile(joinPath(tmpDir, 'tools.json'), '{}')

packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.ts

@@ -1,6 +1,7 @@
 import {joinPath, basename, relativePath, extname} from '@shopify/cli-kit/node/path'
 import {glob, copyFile, copyDirectoryContents, fileExists, mkdir, isDirectory} from '@shopify/cli-kit/node/fs'
 import {outputContent, outputDebug, outputToken} from '@shopify/cli-kit/node/output'
+import {AbortError} from '@shopify/cli-kit/node/error'
 import type {BuildContext} from '../../client-steps.js'
 
 /**
@@ -25,8 +26,17 @@ export async function copyConfigKeyEntry(config: {
   context: BuildContext
   destination?: string
   usedBasenames?: Set<string>
+  preserveFilePaths?: boolean
 }): Promise<{filesCopied: number; pathMap: Map<string, string | string[]>}> {
-  const {key, baseDir, outputDir, context, destination, usedBasenames = new Set()} = config
+  const {
+    key,
+    baseDir,
+    outputDir,
+    context,
+    destination,
+    usedBasenames = new Set<string>(),
+    preserveFilePaths = false,
+  } = config
   const {stdout} = context.options
   const value = getNestedValue(context.extension.configuration, key)
   let paths: string[]
@@ -75,19 +85,31 @@ export async function copyConfigKeyEntry(config: {
       // that may no longer exist in the source, inflating the file count and producing
       // stale entries in the manifest's pathMap.
       const sourceFiles = await glob(['**/*'], {cwd: fullPath, absolute: false})
+      const relFiles = sourceFiles.map((file) => relativePath(outputDir, joinPath(destDir, file)))
+      if (preserveFilePaths) {
+        for (const file of sourceFiles) assertNoCollision(basename(file), sourcePath, usedBasenames)
+      }
       await copyDirectoryContents(fullPath, destDir)
       stdout.write(`Included '${sourcePath}'\n`)
-      const relFiles = sourceFiles.map((file) => relativePath(outputDir, joinPath(destDir, file)))
+      for (const file of sourceFiles) usedBasenames.add(basename(file))
       pathMap.set(sourcePath, relFiles)
       filesCopied += sourceFiles.length
     } else {
       await mkdir(destDir)
       const filename = basename(fullPath)
-      const destFilename = uniqueBasename(filename, usedBasenames)
+      let destFilename: string
+      if (preserveFilePaths) {
+        // Strict mode: any prior entry that wrote the same basename is a collision.
+        assertNoCollision(filename, sourcePath, usedBasenames)
+        destFilename = filename
+      } else {
+        // Default mode: flat basename uniqueness across all destinations.
+        destFilename = uniqueBasename(filename, usedBasenames)
+      }
       usedBasenames.add(destFilename)
+      const outputRelative = relativePath(outputDir, joinPath(destDir, destFilename))
       const destPath = joinPath(destDir, destFilename)
       await copyFile(fullPath, destPath)
-      const outputRelative = relativePath(outputDir, destPath)
       stdout.write(`Included '${sourcePath}'\n`)
       pathMap.set(sourcePath, outputRelative)
       filesCopied += 1
@@ -98,6 +120,18 @@ export async function copyConfigKeyEntry(config: {
   return {filesCopied, pathMap}
 }
 
+/**
+ * Throws if `path` is already present in `used`. Shared between directory and
+ * single-file branches so the error message and check shape stay in one place.
+ */
+function assertNoCollision(path: string, sourcePath: string, used: Set<string>): void {
+  if (used.has(path)) {
+    throw new AbortError(
+      `File collision: '${path}' from '${sourcePath}' would overwrite a file copied from a different source. Rename or relocate the conflicting file.`,
+    )
+  }
+}
+
 /**
  * Returns a unique filename given the set of basenames already used in this
  * build. If `filename` hasn't been used, returns it as-is. Otherwise appends