Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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'), '{}')
Expand Down
Original file line number Diff line number Diff line change
@@ -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'

/**
Expand All @@ -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[]
Expand Down Expand Up @@ -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
Expand All @@ -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
Expand Down
Loading