refactor: remove redundant options param from copyConfigKeyEntry

craigmichaelmartin · claude · Craig Martin · commit b7797ab66b39 · 2026-04-01T14:29:34.000-04:00
context.options already carries stdout, so the separate second
parameter was duplicating it — every caller was passing context.options
anyway. Pull stdout directly from context.options inside the function
and update the call site and tests accordingly.

Co-Authored-By: Claude Sonnet 4.6 &lt;noreply@anthropic.com&gt;
diff --git a/packages/app/src/cli/services/build/steps/include-assets-step.ts b/packages/app/src/cli/services/build/steps/include-assets-step.ts
@@ -140,16 +140,13 @@ export async function executeIncludeAssetsStep(
     const rawDest = entry.destination ? sanitizeRelativePath(entry.destination, warn) : undefined
     const sanitizedDest = rawDest === '' ? undefined : rawDest
     // eslint-disable-next-line no-await-in-loop
-    const result = await copyConfigKeyEntry(
-      {
-        key: entry.key,
-        baseDir: extension.directory,
-        outputDir,
-        context,
-        destination: sanitizedDest,
-      },
-      options,
-    )
+    const result = await copyConfigKeyEntry({
+      key: entry.key,
+      baseDir: extension.directory,
+      outputDir,
+      context,
+      destination: sanitizedDest,
+    })
     result.pathMap.forEach((val, key) => aggregatedPathMap.set(key, val))
     configKeyCount += result.filesCopied
   }
diff --git a/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.test.ts b/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.test.ts
@@ -3,9 +3,9 @@ import {inTemporaryDirectory, writeFile, fileExists, mkdir, readFile} from '@sho
 import {joinPath} from '@shopify/cli-kit/node/path'
 import {describe, expect, test, vi, beforeEach} from 'vitest'
 
-const makeContext = (configuration: Record<string, unknown>) => ({
+const makeContext = (configuration: Record<string, unknown>, stdout: any = {write: vi.fn()}) => ({
   extension: {configuration} as any,
-  options: {} as any,
+  options: {stdout} as any,
   stepResults: new Map(),
 })
 
@@ -26,11 +26,8 @@ describe('copyConfigKeyEntry', () => {
       const outDir = joinPath(tmpDir, 'out')
       await mkdir(outDir)
 
-      const context = makeContext({static_root: 'public'})
-      const result = await copyConfigKeyEntry(
-        {key: 'static_root', baseDir: tmpDir, outputDir: outDir, context},
-        {stdout: mockStdout},
-      )
+      const context = makeContext({static_root: 'public'}, mockStdout)
+      const result = await copyConfigKeyEntry({key: 'static_root', baseDir: tmpDir, outputDir: outDir, context})
 
       expect(result.filesCopied).toBe(2)
       await expect(fileExists(joinPath(outDir, 'index.html'))).resolves.toBe(true)
@@ -49,11 +46,8 @@ describe('copyConfigKeyEntry', () => {
       const outDir = joinPath(tmpDir, 'out')
       await mkdir(outDir)
 
-      const context = makeContext({schema_path: 'src/schema.json'})
-      const result = await copyConfigKeyEntry(
-        {key: 'schema_path', baseDir: tmpDir, outputDir: outDir, context},
-        {stdout: mockStdout},
-      )
+      const context = makeContext({schema_path: 'src/schema.json'}, mockStdout)
+      const result = await copyConfigKeyEntry({key: 'schema_path', baseDir: tmpDir, outputDir: outDir, context})
 
       expect(result.filesCopied).toBe(1)
       await expect(fileExists(joinPath(outDir, 'schema.json'))).resolves.toBe(true)
@@ -74,11 +68,8 @@ describe('copyConfigKeyEntry', () => {
       // Pre-create the first candidate to force a rename
       await writeFile(joinPath(outDir, 'tools-a.json'), 'existing')
 
-      const context = makeContext({files: ['tools-a.json', 'tools-b.json']})
-      const result = await copyConfigKeyEntry(
-        {key: 'files', baseDir: tmpDir, outputDir: outDir, context},
-        {stdout: mockStdout},
-      )
+      const context = makeContext({files: ['tools-a.json', 'tools-b.json']}, mockStdout)
+      const result = await copyConfigKeyEntry({key: 'files', baseDir: tmpDir, outputDir: outDir, context})
 
       expect(result.filesCopied).toBe(2)
       // tools-a.json was taken, so the copy lands as tools-a-1.json
@@ -92,11 +83,8 @@ describe('copyConfigKeyEntry', () => {
       const outDir = joinPath(tmpDir, 'out')
       await mkdir(outDir)
 
-      const context = makeContext({})
-      const result = await copyConfigKeyEntry(
-        {key: 'static_root', baseDir: tmpDir, outputDir: outDir, context},
-        {stdout: mockStdout},
-      )
+      const context = makeContext({}, mockStdout)
+      const result = await copyConfigKeyEntry({key: 'static_root', baseDir: tmpDir, outputDir: outDir, context})
 
       expect(result.filesCopied).toBe(0)
       expect(result.pathMap.size).toBe(0)
@@ -109,11 +97,8 @@ describe('copyConfigKeyEntry', () => {
       await mkdir(outDir)
 
       // 'nonexistent' directory is NOT created, so fileExists returns false naturally
-      const context = makeContext({assets_dir: 'nonexistent'})
-      const result = await copyConfigKeyEntry(
-        {key: 'assets_dir', baseDir: tmpDir, outputDir: outDir, context},
-        {stdout: mockStdout},
-      )
+      const context = makeContext({assets_dir: 'nonexistent'}, mockStdout)
+      const result = await copyConfigKeyEntry({key: 'assets_dir', baseDir: tmpDir, outputDir: outDir, context})
 
       expect(result.filesCopied).toBe(0)
       expect(result.pathMap.size).toBe(0)
@@ -137,11 +122,8 @@ describe('copyConfigKeyEntry', () => {
       const outDir = joinPath(tmpDir, 'out')
       await mkdir(outDir)
 
-      const context = makeContext({roots: ['public', 'assets']})
-      const result = await copyConfigKeyEntry(
-        {key: 'roots', baseDir: tmpDir, outputDir: outDir, context},
-        {stdout: mockStdout},
-      )
+      const context = makeContext({roots: ['public', 'assets']}, mockStdout)
+      const result = await copyConfigKeyEntry({key: 'roots', baseDir: tmpDir, outputDir: outDir, context})
 
       // Promise.all runs copies sequentially; glob on the shared outDir may see files
       // from the other copy, so the total count is at least 3 (one per real file).
@@ -161,11 +143,14 @@ describe('copyConfigKeyEntry', () => {
       const outDir = joinPath(tmpDir, 'out')
       await mkdir(outDir)
 
-      const context = makeContext({icons_dir: 'icons'})
-      await copyConfigKeyEntry(
-        {key: 'icons_dir', baseDir: tmpDir, outputDir: outDir, context, destination: 'static/icons'},
-        {stdout: mockStdout},
-      )
+      const context = makeContext({icons_dir: 'icons'}, mockStdout)
+      await copyConfigKeyEntry({
+        key: 'icons_dir',
+        baseDir: tmpDir,
+        outputDir: outDir,
+        context,
+        destination: 'static/icons',
+      })
 
       await expect(fileExists(joinPath(outDir, 'static', 'icons', 'icon.svg'))).resolves.toBe(true)
     })
@@ -186,10 +171,12 @@ describe('copyConfigKeyEntry', () => {
           {targeting: [{schema: 'schema-c.json'}]},
         ],
       })
-      const result = await copyConfigKeyEntry(
-        {key: 'extensions[].targeting[].schema', baseDir: tmpDir, outputDir: outDir, context},
-        {stdout: mockStdout},
-      )
+      const result = await copyConfigKeyEntry({
+        key: 'extensions[].targeting[].schema',
+        baseDir: tmpDir,
+        outputDir: outDir,
+        context,
+      })
 
       expect(result.filesCopied).toBe(3)
       await expect(fileExists(joinPath(outDir, 'schema-a.json'))).resolves.toBe(true)
@@ -203,15 +190,20 @@ describe('copyConfigKeyEntry', () => {
       const outDir = joinPath(tmpDir, 'out')
       await mkdir(outDir)
 
-      const context = makeContext({
-        extensions: {targeting: {schema: 'schema.json'}},
-      })
-
-      const result = await copyConfigKeyEntry(
-        {key: 'extensions[].targeting[].schema', baseDir: tmpDir, outputDir: outDir, context},
-        {stdout: mockStdout},
+      const context = makeContext(
+        {
+          extensions: {targeting: {schema: 'schema.json'}},
+        },
+        mockStdout,
       )
 
+      const result = await copyConfigKeyEntry({
+        key: 'extensions[].targeting[].schema',
+        baseDir: tmpDir,
+        outputDir: outDir,
+        context,
+      })
+
       expect(result.filesCopied).toBe(0)
       expect(result.pathMap.size).toBe(0)
     })
@@ -228,10 +220,12 @@ describe('copyConfigKeyEntry', () => {
       const context = makeContext({
         extensions: [{targeting: [{tools: 'tools.json'}, {tools: 'tools.json'}]}],
       })
-      const result = await copyConfigKeyEntry(
-        {key: 'extensions[].targeting[].tools', baseDir: tmpDir, outputDir: outDir, context},
-        {stdout: mockStdout},
-      )
+      const result = await copyConfigKeyEntry({
+        key: 'extensions[].targeting[].tools',
+        baseDir: tmpDir,
+        outputDir: outDir,
+        context,
+      })
 
       expect(result.filesCopied).toBe(1)
       await expect(fileExists(joinPath(outDir, 'tools.json'))).resolves.toBe(true)
diff --git a/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.ts b/packages/app/src/cli/services/build/steps/include-assets/copy-config-key-entry.ts
@@ -20,17 +20,15 @@ import type {BuildContext} from '../../client-steps.js'
  * value to its output-relative location. File sources map to a single string.
  * Directory sources map to a `string[]` of every output-relative file path.
  */
-export async function copyConfigKeyEntry(
-  config: {
-    key: string
-    baseDir: string
-    outputDir: string
-    context: BuildContext
-    destination?: string
-  },
-  options: {stdout: NodeJS.WritableStream},
-): Promise<{filesCopied: number; pathMap: Map<string, string | string[]>}> {
+export async function copyConfigKeyEntry(config: {
+  key: string
+  baseDir: string
+  outputDir: string
+  context: BuildContext
+  destination?: string
+}): Promise<{filesCopied: number; pathMap: Map<string, string | string[]>}> {
   const {key, baseDir, outputDir, context, destination} = config
+  const {stdout} = context.options
   const value = getNestedValue(context.extension.configuration, key)
   let paths: string[]
   if (typeof value === 'string') {
@@ -42,7 +40,7 @@ export async function copyConfigKeyEntry(
   }
 
   if (paths.length === 0) {
-    outputDebug(`No value for configKey '${key}', skipping\n`, context.options.stdout)
+    outputDebug(`No value for configKey '${key}', skipping\n`, stdout)
     return {filesCopied: 0, pathMap: new Map()}
   }
 
@@ -62,7 +60,7 @@ export async function copyConfigKeyEntry(
     const fullPath = joinPath(baseDir, sourcePath)
     const exists = await fileExists(fullPath)
     if (!exists) {
-      options.stdout.write(`Warning: path '${sourcePath}' does not exist, skipping\n`)
+      stdout.write(`Warning: path '${sourcePath}' does not exist, skipping\n`)
       continue
     }
 
@@ -73,7 +71,7 @@ export async function copyConfigKeyEntry(
     if (sourceIsDir) {
       await copyDirectoryContents(fullPath, destDir)
       const copied = await glob(['**/*'], {cwd: destDir, absolute: false})
-      options.stdout.write(`Included '${sourcePath}'\n`)
+      stdout.write(`Included '${sourcePath}'\n`)
       const relFiles = copied.map((file) => relativePath(outputDir, joinPath(destDir, file)))
       pathMap.set(sourcePath, relFiles)
       filesCopied += copied.length
@@ -82,7 +80,7 @@ export async function copyConfigKeyEntry(
       const uniqueDestPath = await findUniqueDestPath(destDir, basename(fullPath))
       await copyFile(fullPath, uniqueDestPath)
       const outputRelative = relativePath(outputDir, uniqueDestPath)
-      options.stdout.write(`Included '${sourcePath}'\n`)
+      stdout.write(`Included '${sourcePath}'\n`)
       pathMap.set(sourcePath, outputRelative)
       filesCopied += 1
     }
