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
5 changes: 5 additions & 0 deletions .changeset/sixty-eyes-juggle.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@shopify/app': patch
---

Fix "shopify app build" for some ui extensions
Original file line number Diff line number Diff line change
Expand Up @@ -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> {
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/models/extensions/specification.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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[]>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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, _) => {
Expand All @@ -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(
Expand Down
2 changes: 1 addition & 1 deletion packages/app/src/cli/services/build/extension.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
13 changes: 13 additions & 0 deletions packages/cli-kit/src/public/node/fs.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -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', () => {
Expand Down
8 changes: 7 additions & 1 deletion packages/cli-kit/src/public/node/fs.ts
Original file line number Diff line number Diff line change
@@ -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'
Expand Down Expand Up @@ -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)
}
Expand Down
Loading