From 7a352b05020e2b55d3e19644303f4a6f7ff8fdcd Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isaac=20Rold=C3=A1n?= Date: Thu, 9 Apr 2026 19:49:22 +0200 Subject: [PATCH] fix: stop base64-encoding dist/index.wasm during function builds MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CLI 3.93.0 introduced a regression where dist/index.wasm gets base64-encoded text instead of raw binary after `shopify app build`. This breaks vitest for Rust function extensions. Root cause: extension.outputPath was mutated at runtime by buildFunctionExtension, buildForBundle, and copyIntoBundle. The value you got depended on when you read it. Fix: - Make outputPath effectively readonly (only set in the constructor) - Add bundleOutputPath to ExtensionBuildOptions as an explicit override - buildForBundle/copyIntoBundle pass bundleOutputPath through options instead of mutating this.outputPath - buildFunctionExtension uses local buildOutputPath variable, copies raw binary to extension.outputPath, and only base64-encodes when bundleOutputPath is set - Thread explicit outputPath through the JS build chain (buildJSFunction → JavyBuilder.compile → runJavy) so runJavy no longer reads fun.outputPath Co-Authored-By: Isaac Roldán Co-Authored-By: Claude Opus 4.6 (1M context) --- .../extensions/extension-instance.test.ts | 5 +- .../models/extensions/extension-instance.ts | 31 ++++----- .../extensions/specifications/function.ts | 6 +- .../src/cli/services/build/extension.test.ts | 67 ++++++++++++++----- .../app/src/cli/services/build/extension.ts | 58 +++++++++------- .../cli/services/extensions/bundle.test.ts | 8 +-- .../app/src/cli/services/extensions/bundle.ts | 6 +- .../app/src/cli/services/function/build.ts | 26 ++++--- 8 files changed, 130 insertions(+), 77 deletions(-) diff --git a/packages/app/src/cli/models/extensions/extension-instance.test.ts b/packages/app/src/cli/models/extensions/extension-instance.test.ts index c765e6e6f6c..c6a182377ba 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.test.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.test.ts @@ -224,10 +224,11 @@ describe('build', async () => { await extension.copyIntoBundle(options, bundleDirectory, 'uuid') // Then - const outputTomlPath = joinPath(extension.outputPath, 'shopify.extension.toml') + const bundleOutputPath = extension.getOutputPathForDirectory(bundleDirectory, 'uuid') + const outputTomlPath = joinPath(bundleOutputPath, 'shopify.extension.toml') expect(fileExistsSync(outputTomlPath)).toBe(false) - const outputProductPath = joinPath(extension.outputPath, 'blocks', 'product.liquid') + const outputProductPath = joinPath(bundleOutputPath, 'blocks', 'product.liquid') expect(fileExistsSync(outputProductPath)).toBe(true) }) }) diff --git a/packages/app/src/cli/models/extensions/extension-instance.ts b/packages/app/src/cli/models/extensions/extension-instance.ts index 318617ab2b7..048a9e9a20b 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.ts @@ -14,7 +14,7 @@ import {constantize, slugify} from '@shopify/cli-kit/common/string' import {hashString, nonRandomUUID} from '@shopify/cli-kit/node/crypto' import {partnersFqdn} from '@shopify/cli-kit/node/context/fqdn' import {joinPath, normalizePath, resolvePath, relativePath, basename} from '@shopify/cli-kit/node/path' -import {fileExists, moveFile, glob, copyFile, globSync} from '@shopify/cli-kit/node/fs' +import {fileExists, moveFile, glob, copyFile, globSync, touchFile} from '@shopify/cli-kit/node/fs' import {getPathValue} from '@shopify/cli-kit/common/object' import {outputDebug} from '@shopify/cli-kit/node/output' import { @@ -46,7 +46,7 @@ export class ExtensionInstance { - const wasmExists = await fileExists(extension.outputPath) + const outputPath = extension.configuration.build?.path ?? extension.outputRelativePath + const fullPath = joinPath(extension.directory, outputPath) + const wasmExists = await fileExists(fullPath) if (!wasmExists) { throw new AbortError( - outputContent`The function extension "${extension.handle}" hasn't compiled the wasm in the expected path: ${extension.outputPath}`, + outputContent`The function extension "${extension.handle}" hasn't compiled the wasm in the expected path: ${fullPath}`, `Make sure the build command outputs the wasm in the expected directory.`, ) } diff --git a/packages/app/src/cli/services/build/extension.test.ts b/packages/app/src/cli/services/build/extension.test.ts index 7f4be985566..84a6fa8901b 100644 --- a/packages/app/src/cli/services/build/extension.test.ts +++ b/packages/app/src/cli/services/build/extension.test.ts @@ -7,7 +7,7 @@ import {beforeEach, describe, expect, test, vi} from 'vitest' import {exec} from '@shopify/cli-kit/node/system' import lockfile from 'proper-lockfile' import {AbortError} from '@shopify/cli-kit/node/error' -import {fileExistsSync, touchFile, writeFile} from '@shopify/cli-kit/node/fs' +import {copyFile, fileExistsSync, touchFile, writeFile} from '@shopify/cli-kit/node/fs' import {joinPath} from '@shopify/cli-kit/node/path' vi.mock('@shopify/cli-kit/node/system') @@ -106,13 +106,17 @@ describe('buildFunctionExtension', () => { ).resolves.toBeUndefined() // Then - expect(buildJSFunction).toHaveBeenCalledWith(extension, { - stdout, - stderr, - signal, - app, - environment: 'production', - }) + expect(buildJSFunction).toHaveBeenCalledWith( + extension, + { + stdout, + stderr, + signal, + app, + environment: 'production', + }, + joinPath(extension.directory, 'dist/index.wasm'), + ) expect(releaseLock).toHaveBeenCalled() }) @@ -243,13 +247,17 @@ describe('buildFunctionExtension', () => { ).resolves.toBeUndefined() // Then - expect(buildJSFunction).toHaveBeenCalledWith(extension, { - stdout, - stderr, - signal, - app, - environment: 'production', - }) + expect(buildJSFunction).toHaveBeenCalledWith( + extension, + { + stdout, + stderr, + signal, + app, + environment: 'production', + }, + joinPath(extension.directory, 'dist', 'index.wasm'), + ) expect(releaseLock).toHaveBeenCalled() // wasm_opt should not be called when build config is undefined expect(runWasmOpt).not.toHaveBeenCalled() @@ -418,7 +426,7 @@ describe('buildFunctionExtension', () => { expect(runWasmOpt).toHaveBeenCalled() }) - test('does not rebundle when build.path stays in the default output directory', async () => { + test('copies raw binary when build.path differs from default output path', async () => { // Given extension.configuration.build!.path = 'dist/custom.wasm' vi.mocked(fileExistsSync).mockReturnValue(true) @@ -435,8 +443,31 @@ describe('buildFunctionExtension', () => { ).resolves.toBeUndefined() // Then - expect(fileExistsSync).toHaveBeenCalledWith(joinPath(extension.directory, 'dist/custom.wasm')) - expect(touchFile).not.toHaveBeenCalled() + const buildOutputPath = joinPath(extension.directory, 'dist/custom.wasm') + const canonicalOutputPath = joinPath(extension.directory, 'dist', 'index.wasm') + expect(fileExistsSync).toHaveBeenCalledWith(buildOutputPath) + expect(touchFile).toHaveBeenCalledWith(canonicalOutputPath) + expect(copyFile).toHaveBeenCalledWith(buildOutputPath, canonicalOutputPath) + // Must NOT base64-encode during build — only raw binary copy expect(writeFile).not.toHaveBeenCalled() }) + + test('does not mutate extension.outputPath', async () => { + // Given + extension.configuration.build!.path = 'target/wasm32-wasi/release/func.wasm' + vi.mocked(fileExistsSync).mockReturnValue(true) + const originalOutputPath = extension.outputPath + + // When + await buildFunctionExtension(extension, { + stdout, + stderr, + signal, + app, + environment: 'production', + }) + + // Then + expect(extension.outputPath).toBe(originalOutputPath) + }) }) diff --git a/packages/app/src/cli/services/build/extension.ts b/packages/app/src/cli/services/build/extension.ts index 97e8eca8f7a..4078b586310 100644 --- a/packages/app/src/cli/services/build/extension.ts +++ b/packages/app/src/cli/services/build/extension.ts @@ -10,7 +10,7 @@ import {AbortError, AbortSilentError} from '@shopify/cli-kit/node/error' import lockfile from 'proper-lockfile' import {dirname, joinPath} from '@shopify/cli-kit/node/path' import {outputDebug} from '@shopify/cli-kit/node/output' -import {readFile, touchFile, writeFile, fileExistsSync} from '@shopify/cli-kit/node/fs' +import {copyFile, readFile, touchFile, writeFile, fileExistsSync} from '@shopify/cli-kit/node/fs' import {Writable} from 'stream' export interface ExtensionBuildOptions { @@ -53,6 +53,14 @@ export interface ExtensionBuildOptions { * The URL where the app is running. */ appURL?: string + + /** + * When building for a deploy or dev bundle, this is the output path inside the + * bundle directory. When set, build functions write their final artifact here + * instead of extension.outputPath. This avoids mutating extension.outputPath at + * runtime. + */ + bundleOutputPath?: string } /** @@ -66,12 +74,13 @@ export async function buildUIExtension(extension: ExtensionInstance, options: Ex env.APP_URL = options.appURL } + const outputPath = options.bundleOutputPath ?? extension.outputPath const {main, assets} = extension.getBundleExtensionStdinContent() try { await bundleExtension({ minify: true, - outputPath: extension.outputPath, + outputPath, stdin: { contents: main, resolveDir: extension.directory, @@ -88,7 +97,7 @@ export async function buildUIExtension(extension: ExtensionInstance, options: Ex assets.map(async (asset) => { await bundleExtension({ minify: true, - outputPath: joinPath(dirname(extension.outputPath), asset.outputFileName), + outputPath: joinPath(dirname(outputPath), asset.outputFileName), stdin: { contents: asset.content, resolveDir: extension.directory, @@ -111,7 +120,7 @@ export async function buildUIExtension(extension: ExtensionInstance, options: Ex await extension.buildValidation() - const sizeInfo = await formatBundleSize(extension.outputPath) + const sizeInfo = await formatBundleSize(outputPath) options.stdout.write(`${extension.localIdentifier} successfully built${sizeInfo}`) } @@ -140,33 +149,31 @@ export async function buildFunctionExtension( } try { - const bundlePath = extension.outputPath const relativeBuildPath = (extension as ExtensionInstance).configuration.build?.path ?? extension.outputRelativePath - - extension.outputPath = joinPath(extension.directory, relativeBuildPath) + const buildOutputPath = joinPath(extension.directory, relativeBuildPath) if (extension.isJavaScript) { - await runCommandOrBuildJSFunction(extension, options) + await runCommandOrBuildJSFunction(extension, options, buildOutputPath) } else { await buildOtherFunction(extension, options) } const wasmOpt = (extension as ExtensionInstance).configuration.build?.wasm_opt - if (fileExistsSync(extension.outputPath) && wasmOpt) { - await runWasmOpt(extension.outputPath) + if (fileExistsSync(buildOutputPath) && wasmOpt) { + await runWasmOpt(buildOutputPath) } - if (fileExistsSync(extension.outputPath)) { - await runTrampoline(extension.outputPath) + if (fileExistsSync(buildOutputPath)) { + await runTrampoline(buildOutputPath) } - if ( - fileExistsSync(extension.outputPath) && - bundlePath !== extension.outputPath && - dirname(bundlePath) !== dirname(extension.outputPath) - ) { - await bundleFunctionExtension(extension.outputPath, bundlePath) + // When building for a bundle, copy + base64-encode into the bundle directory. + // This mirrors how buildUIExtension writes directly to bundleOutputPath via esbuild. + if (options.bundleOutputPath && fileExistsSync(buildOutputPath)) { + await touchFile(options.bundleOutputPath) + await copyFile(buildOutputPath, options.bundleOutputPath) + await bundleFunctionExtension(options.bundleOutputPath) } // eslint-disable-next-line @typescript-eslint/no-explicit-any } catch (error: any) { @@ -188,21 +195,24 @@ export async function buildFunctionExtension( } } -export async function bundleFunctionExtension(wasmPath: string, bundlePath: string) { - outputDebug(`Converting WASM from ${wasmPath} to base64 in ${bundlePath}`) +export async function bundleFunctionExtension(wasmPath: string) { + outputDebug(`Converting WASM to base64 in ${wasmPath}`) const base64Contents = await readFile(wasmPath, {encoding: 'base64'}) - await touchFile(bundlePath) - await writeFile(bundlePath, base64Contents) + await writeFile(wasmPath, base64Contents) } -async function runCommandOrBuildJSFunction(extension: ExtensionInstance, options: BuildFunctionExtensionOptions) { +async function runCommandOrBuildJSFunction( + extension: ExtensionInstance, + options: BuildFunctionExtensionOptions, + buildOutputPath: string, +) { if (extension.buildCommand) { if (extension.typegenCommand) { await buildGraphqlTypes(extension, options) } return runCommand(extension.buildCommand, extension, options) } else { - return buildJSFunction(extension as ExtensionInstance, options) + return buildJSFunction(extension as ExtensionInstance, options, buildOutputPath) } } diff --git a/packages/app/src/cli/services/extensions/bundle.test.ts b/packages/app/src/cli/services/extensions/bundle.test.ts index 515872041a6..1f61225a82b 100644 --- a/packages/app/src/cli/services/extensions/bundle.test.ts +++ b/packages/app/src/cli/services/extensions/bundle.test.ts @@ -322,9 +322,8 @@ describe('bundleExtension()', () => { specification, }) - const outputPath = joinPath(tmpDir, 'dist') - await mkdir(outputPath) - themeExtension.outputPath = outputPath + const bundleOutputPath = joinPath(tmpDir, 'dist') + await mkdir(bundleOutputPath) const app = testApp({ directory: '/project', @@ -361,10 +360,11 @@ describe('bundleExtension()', () => { stdout, stderr, environment: 'production', + bundleOutputPath, }) // Then - const filePaths = await glob(joinPath(themeExtension.outputPath, '/**/*')) + const filePaths = await glob(joinPath(bundleOutputPath, '/**/*')) const hasFiles = filePaths .map((filePath) => basename(filePath)) .some((filename) => ignoredFiles.includes(filename)) diff --git a/packages/app/src/cli/services/extensions/bundle.ts b/packages/app/src/cli/services/extensions/bundle.ts index 67a4ad0cc2b..8ae228370fd 100644 --- a/packages/app/src/cli/services/extensions/bundle.ts +++ b/packages/app/src/cli/services/extensions/bundle.ts @@ -71,12 +71,13 @@ export async function bundleThemeExtension( options: ExtensionBuildOptions, ): Promise { options.stdout.write(`Bundling theme extension ${extension.localIdentifier}...`) + const outputDir = options.bundleOutputPath ?? extension.outputPath const files = await themeExtensionFiles(extension) await Promise.all( files.map(function (filepath) { const relativePathName = relativePath(extension.directory, filepath) - const outputFile = joinPath(extension.outputPath, relativePathName) + const outputFile = joinPath(outputDir, relativePathName) if (filepath === outputFile) return return copyFile(filepath, outputFile) }), @@ -90,6 +91,7 @@ export async function copyFilesForExtension( ignoredPatterns: string[] = [], ): Promise { options.stdout.write(`Copying files for extension ${extension.localIdentifier}...`) + const outputDir = options.bundleOutputPath ?? extension.outputPath const include = includePatterns.map((pattern) => joinPath('**', pattern)) const ignored = ignoredPatterns.map((pattern) => joinPath('**', pattern)) const files = await glob(include, { @@ -101,7 +103,7 @@ export async function copyFilesForExtension( await Promise.all( files.map(function (filepath) { const relativePathName = relativePath(extension.directory, filepath) - const outputFile = joinPath(extension.outputPath, relativePathName) + const outputFile = joinPath(outputDir, relativePathName) if (filepath === outputFile) return return copyFile(filepath, outputFile) }), diff --git a/packages/app/src/cli/services/function/build.ts b/packages/app/src/cli/services/function/build.ts index 7e8caf4f6fa..5996d791039 100644 --- a/packages/app/src/cli/services/function/build.ts +++ b/packages/app/src/cli/services/function/build.ts @@ -56,16 +56,20 @@ interface JSFunctionBuildOptions { useTasks?: boolean } -export async function buildJSFunction(fun: ExtensionInstance, options: JSFunctionBuildOptions) { +export async function buildJSFunction( + fun: ExtensionInstance, + options: JSFunctionBuildOptions, + outputPath?: string, +) { const exports = jsExports(fun) const javyBuilder: JavyBuilder = exports.length === 0 ? DefaultJavyBuilder : new ExportJavyBuilder(exports) const deps = await validateShopifyFunctionPackageVersion(fun) if (options.useTasks) { - return buildJSFunctionWithTasks(fun, options, javyBuilder, deps) + return buildJSFunctionWithTasks(fun, options, javyBuilder, deps, outputPath) } else { - return buildJSFunctionWithoutTasks(fun, options, javyBuilder, deps) + return buildJSFunctionWithoutTasks(fun, options, javyBuilder, deps, outputPath) } } @@ -74,6 +78,7 @@ async function buildJSFunctionWithoutTasks( options: JSFunctionBuildOptions, builder: JavyBuilder, deps: BinaryDependencies, + outputPath?: string, ) { if (!options.signal?.aborted) { options.stdout.write(`Building function ${fun.localIdentifier}...`) @@ -86,7 +91,7 @@ async function buildJSFunctionWithoutTasks( } if (!options.signal?.aborted) { options.stdout.write(`Running javy...\n`) - await builder.compile(fun, options, deps) + await builder.compile(fun, options, deps, outputPath) } if (!options.signal?.aborted) { options.stdout.write(`Done!\n`) @@ -98,6 +103,7 @@ async function buildJSFunctionWithTasks( options: JSFunctionBuildOptions, builder: JavyBuilder, deps: BinaryDependencies, + outputPath?: string, ) { await renderTasks([ { @@ -115,7 +121,7 @@ async function buildJSFunctionWithTasks( { title: 'Running javy', task: async () => { - await builder.compile(fun, options, deps) + await builder.compile(fun, options, deps, outputPath) }, }, ]) @@ -313,6 +319,7 @@ export async function runJavy( options: JSFunctionBuildOptions, binaryDeps: BinaryDependencies, extra: string[] = [], + outputPath?: string, ) { const javy = javyBinary(binaryDeps.javy) const plugin = javyPluginBinary(binaryDeps.javyPlugin) @@ -329,7 +336,7 @@ export async function runJavy( `plugin=${plugin.path}`, ...extra, '-o', - fun.outputPath, + outputPath ?? fun.outputPath, 'dist/function.js', ] @@ -377,6 +384,7 @@ interface JavyBuilder { fun: ExtensionInstance, options: JSFunctionBuildOptions, binaryDeps: BinaryDependencies, + outputPath?: string, ): Promise } @@ -389,8 +397,9 @@ const DefaultJavyBuilder: JavyBuilder = { fun: ExtensionInstance, options: JSFunctionBuildOptions, binaryDeps: BinaryDependencies, + outputPath?: string, ) { - return runJavy(fun, options, binaryDeps) + return runJavy(fun, options, binaryDeps, [], outputPath) }, } @@ -424,6 +433,7 @@ export class ExportJavyBuilder implements JavyBuilder { fun: ExtensionInstance, options: JSFunctionBuildOptions, binaryDeps: BinaryDependencies, + outputPath?: string, ) { const witContent = this.wit outputDebug('Generating world to use with Javy:') @@ -433,7 +443,7 @@ export class ExportJavyBuilder implements JavyBuilder { const witPath = joinPath(dir, 'javy-world.wit') await writeFile(witPath, witContent) - return runJavy(fun, options, binaryDeps, ['-C', `wit=${witPath}`, '-C', `wit-world=${JAVY_WORLD}`]) + return runJavy(fun, options, binaryDeps, ['-C', `wit=${witPath}`, '-C', `wit-world=${JAVY_WORLD}`], outputPath) }) }