Bypass copyFile bug in fs-extra where it was not allowing files to be overwritten

Fix web_pixe_extension validateBundle step to look at the locally built file so it will work for all commands

Co-authored-by: Isaac Roldan <isaac.roldan@shopify.com>

Author: vividviolet

packages/app/src/cli/models/extensions/extension-instance.ts

@@ -203,9 +203,9 @@ export class ExtensionInstance<TConfiguration extends BaseConfigType = BaseConfi
     return this.specification.preDeployValidation(this)
   }
 
-  buildValidation(): Promise<void> {
+  buildValidation({outputPath}: {outputPath: string}): Promise<void> {
     if (!this.specification.buildValidation) return Promise.resolve()
-    return this.specification.buildValidation(this)
+    return this.specification.buildValidation(this, outputPath)
   }
 
   async keepBuiltSourcemapsLocally(inputPath: string): Promise<void> {

packages/app/src/cli/models/extensions/specification.ts

@@ -83,7 +83,7 @@ export interface ExtensionSpecification<TConfiguration extends BaseConfigType =
   ) => Promise<Record<string, unknown> | undefined>
   validate?: (config: TConfiguration, configPath: string, directory: string) => Promise<Result<unknown, string>>
   preDeployValidation?: (extension: ExtensionInstance<TConfiguration>) => Promise<void>
-  buildValidation?: (extension: ExtensionInstance<TConfiguration>) => Promise<void>
+  buildValidation?: (extension: ExtensionInstance<TConfiguration>, outputPath: string) => Promise<void>
   hasExtensionPointTarget?(config: TConfiguration, target: string): boolean
   appModuleFeatures: (config?: TConfiguration) => ExtensionFeature[]
   getDevSessionUpdateMessages?: (config: TConfiguration) => Promise<string[]>

packages/app/src/cli/models/extensions/specifications/web_pixel_extension.ts

@@ -33,11 +33,11 @@ const webPixelSpec = createExtensionSpecification({
   partnersWebIdentifier: 'web_pixel',
   schema: WebPixelSchema,
   appModuleFeatures: (_) => ['esbuild', 'single_js_entry_path'],
-  getOutputRelativePath: (extension: ExtensionInstance<WebPixelConfigType>) => `${extension.handle}.js`,
+  getOutputRelativePath: (extension: ExtensionInstance<WebPixelConfigType>) => `dist/${extension.handle}.js`,
   clientSteps: [
     {
       lifecycle: 'deploy',
-      steps: [{id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {bundleFolder: 'dist/'}}],
+      steps: [{id: 'bundle-ui', name: 'Bundle UI Extension', type: 'bundle_ui', config: {}}],
     },
   ],
   deployConfig: async (config, _) => {
@@ -47,8 +47,8 @@ const webPixelSpec = createExtensionSpecification({
       runtime_configuration_definition: config.settings,
     }
   },
-  buildValidation: async (extension) => {
-    const bundleSize = await fileSize(extension.outputPath)
+  buildValidation: async (_, outputPath) => {
+    const bundleSize = await fileSize(outputPath)
     if (bundleSize > BUNDLE_SIZE_LIMIT) {
       const humanReadableBundleSize = `${(bundleSize / kilobytes).toFixed(2)} kB`
       throw new AbortError(

packages/app/src/cli/services/build/extension.ts

@@ -123,7 +123,7 @@ export async function buildUIExtension(extension: ExtensionInstance, options: Ex
     throw newError
   }
 
-  await extension.buildValidation()
+  await extension.buildValidation({outputPath: localOutputPath})
 
   const duration = Math.round(performance.now() - startTime)
   const sizeInfo = await formatBundleSize(localOutputPath)

packages/app/src/cli/services/build/steps/bundle-ui-step.test.ts

@@ -35,17 +35,6 @@ describe('executeBundleUIStep', () => {
     config: {generatesAssetsManifest: false},
   }
 
-  test('skips the copy when local and bundle output directories are identical', async () => {
-    // Given
-    vi.mocked(buildExtension.buildUIExtension).mockResolvedValue('/test/extension/dist/handle.js')
-
-    // When
-    await executeBundleUIStep(step, mockContext)
-
-    // Then — fs-extra would throw "Source and destination must not be the same"
-    expect(fs.copyFile).not.toHaveBeenCalled()
-  })
-
   test('copies when local and bundle output directories differ', async () => {
     // Given
     mockContext.extension.outputPath = '/bundle/handle/handle.js'

packages/app/src/cli/services/build/steps/bundle-ui-step.ts

@@ -22,7 +22,6 @@ export async function executeBundleUIStep(step: BundleUIStep, context: BuildCont
   const config = context.extension.configuration
   context.options.buildDirectory = step.config?.bundleFolder ?? undefined
   const localOutputPath = await buildUIExtension(context.extension, context.options)
-  // When invoked outside a bundle directory (e.g. `shopify app build`), localOutputPath and outputPath collapse onto the same directory; fs-extra rejects same-path copies.
   const localOutputDir = dirname(localOutputPath)
   const bundleOutputDir = step.config?.bundleFolder
     ? joinPath(dirname(context.extension.outputPath), step.config.bundleFolder)

packages/cli-kit/src/public/node/fs.test.ts

@@ -92,6 +92,19 @@ describe('copy', () => {
       await expect(readFile(joinPath(to, 'child', '.dotfile'))).resolves.toEqual(content)
     })
   })
+
+  test('skips the copy when source and destination resolve to the same path', async () => {
+    await inTemporaryDirectory(async (tmpDir) => {
+      // Given
+      const content = 'test'
+      const path = joinPath(tmpDir, 'file')
+      await writeFile(path, content)
+
+      // When / Then — fs-extra would otherwise throw "Source and destination must not be the same"
+      await expect(copyFile(path, joinPath(path, '..', 'file'))).resolves.not.toThrow()
+      await expect(readFile(path)).resolves.toEqual(content)
+    })
+  })
 })
 
 describe('move', () => {

packages/cli-kit/src/public/node/fs.ts

@@ -1,5 +1,5 @@
 import {outputContent, outputToken, outputDebug} from './output.js'
-import {joinPath, normalizePath} from './path.js'
+import {joinPath, normalizePath, resolvePath} from './path.js'
 import {OverloadParameters} from '../../private/common/ts/overloaded-parameters.js'
 import {getRandomName, RandomNameFamily} from '../common/string.js'
 import {systemTempDir} from '../../private/node/temp-dir.js'
@@ -153,6 +153,12 @@ export async function fileRealPath(path: string): Promise<string> {
  * @param to - Destination path.
  */
 export async function copyFile(from: string, to: string): Promise<void> {
+  if (resolvePath(from) === resolvePath(to)) {
+    outputDebug(
+      outputContent`Skipping copy file step because source and destination is the same: ${outputToken.path(from)}`,
+    )
+    return
+  }
   outputDebug(outputContent`Copying file from ${outputToken.path(from)} to ${outputToken.path(to)}...`)
   await fsCopy(from, to)
 }