diff --git a/packages/app/src/cli/models/extensions/extension-instance.ts b/packages/app/src/cli/models/extensions/extension-instance.ts index 120e5b1fe47..e266cb54c0c 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.ts @@ -203,9 +203,9 @@ export class ExtensionInstance { + buildValidation({outputPath}: {outputPath: string}): Promise { if (!this.specification.buildValidation) return Promise.resolve() - return this.specification.buildValidation(this) + return this.specification.buildValidation(this, outputPath) } async keepBuiltSourcemapsLocally(inputPath: string): Promise { diff --git a/packages/app/src/cli/models/extensions/specification.ts b/packages/app/src/cli/models/extensions/specification.ts index 457f4408aa8..bfbabb2af51 100644 --- a/packages/app/src/cli/models/extensions/specification.ts +++ b/packages/app/src/cli/models/extensions/specification.ts @@ -83,7 +83,7 @@ export interface ExtensionSpecification Promise | undefined> validate?: (config: TConfiguration, configPath: string, directory: string) => Promise> preDeployValidation?: (extension: ExtensionInstance) => Promise - buildValidation?: (extension: ExtensionInstance) => Promise + buildValidation?: (extension: ExtensionInstance, outputPath: string) => Promise hasExtensionPointTarget?(config: TConfiguration, target: string): boolean appModuleFeatures: (config?: TConfiguration) => ExtensionFeature[] getDevSessionUpdateMessages?: (config: TConfiguration) => Promise diff --git a/packages/app/src/cli/models/extensions/specifications/web_pixel_extension.ts b/packages/app/src/cli/models/extensions/specifications/web_pixel_extension.ts index 89ff8a60cfb..500db617485 100644 --- a/packages/app/src/cli/models/extensions/specifications/web_pixel_extension.ts +++ b/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) => `${extension.handle}.js`, + getOutputRelativePath: (extension: ExtensionInstance) => `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( diff --git a/packages/app/src/cli/services/build/extension.ts b/packages/app/src/cli/services/build/extension.ts index cdaf60bf489..6006a9f4b57 100644 --- a/packages/app/src/cli/services/build/extension.ts +++ b/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) diff --git a/packages/app/src/cli/services/build/steps/bundle-ui-step.test.ts b/packages/app/src/cli/services/build/steps/bundle-ui-step.test.ts index 1f3d905db63..5f73580949d 100644 --- a/packages/app/src/cli/services/build/steps/bundle-ui-step.test.ts +++ b/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' diff --git a/packages/app/src/cli/services/build/steps/bundle-ui-step.ts b/packages/app/src/cli/services/build/steps/bundle-ui-step.ts index b77eb1fa1e3..dd65b01ce7c 100644 --- a/packages/app/src/cli/services/build/steps/bundle-ui-step.ts +++ b/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) diff --git a/packages/cli-kit/src/public/node/fs.test.ts b/packages/cli-kit/src/public/node/fs.test.ts index c65237fc338..a54b4d83f55 100644 --- a/packages/cli-kit/src/public/node/fs.test.ts +++ b/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', () => { diff --git a/packages/cli-kit/src/public/node/fs.ts b/packages/cli-kit/src/public/node/fs.ts index 4163b73b2d9..046c0f49e18 100644 --- a/packages/cli-kit/src/public/node/fs.ts +++ b/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 { * @param to - Destination path. */ export async function copyFile(from: string, to: string): Promise { + 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) }