Skip to content

fix: stop base64-encoding dist/index.wasm during function builds#7232

Open
isaacroldan wants to merge 1 commit intomainfrom
04-09-fix_stop_base64-encoding_dist_index.wasm_during_function_builds
Open

fix: stop base64-encoding dist/index.wasm during function builds#7232
isaacroldan wants to merge 1 commit intomainfrom
04-09-fix_stop_base64-encoding_dist_index.wasm_during_function_builds

Conversation

@isaacroldan
Copy link
Copy Markdown
Contributor

WHY are these changes introduced?

Fixes #0000

WHAT is this pull request doing?

How to test your changes?

Post-release steps

Checklist

  • I've considered possible cross-platform impacts (Mac, Linux, Windows)
  • I've considered possible documentation changes
  • I've considered analytics changes to measure impact
  • The change is user-facing, so I've added a changelog entry with pnpm changeset add

Copilot AI review requested due to automatic review settings April 9, 2026 17:49
@isaacroldan isaacroldan requested a review from a team as a code owner April 9, 2026 17:49
Copy link
Copy Markdown
Contributor Author

This stack of pull requests is managed by Graphite. Learn more about stacking.

@isaacroldan isaacroldan force-pushed the 04-09-fix_stop_base64-encoding_dist_index.wasm_during_function_builds branch from ba0dc62 to cc4479d Compare April 9, 2026 17:54
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to prevent dist/index.wasm from being base64-encoded during normal function builds by separating “raw build output” from “bundle output” and introducing an explicit bundleOutputPath override for bundle builds.

Changes:

  • Add an optional outputPath override to the JS function build pipeline (Javy compile/run) so builds can write artifacts without mutating extension.outputPath.
  • Update extension build/bundle logic to support bundleOutputPath, and adjust function build flow to copy raw WASM to the canonical output and only base64-encode for bundle builds.
  • Update tests to reflect the new build output/copy semantics and the non-mutation of extension.outputPath.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file
File Description
packages/app/src/cli/services/function/build.ts Allows JS function compilation to write WASM to an optional override output path.
packages/app/src/cli/services/extensions/bundle.ts Uses bundleOutputPath (when present) as the destination for theme/file copy bundling.
packages/app/src/cli/services/build/extension.ts Introduces bundleOutputPath; updates UI/function build output routing and function bundle/base64 behavior.
packages/app/src/cli/services/build/extension.test.ts Updates expectations for JS function build output path and raw-binary copy behavior.
packages/app/src/cli/models/extensions/extension-instance.ts Stops mutating outputPath during bundle builds; passes bundleOutputPath through options and adjusts bundle copying logic.
packages/app/src/cli/models/extensions/extension-instance.test.ts Updates bundle copy assertions to reflect bundle output paths without relying on mutated outputPath.
Comments suppressed due to low confidence (1)

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

  • When bundleOutputPath is set, this block copies extension.outputPath to bundleOutputPath and then calls bundleFunctionExtension with the same path as both input and output. This extra touch/copy is redundant and does unnecessary I/O; you can base64-encode directly from extension.outputPath into bundleOutputPath (and avoid the self-read/self-write pattern).
  } catch (error: any) {
    // To avoid random user-code errors being reported as CLI bugs, we capture and rethrow them as AbortError.
    // In this case, we need to keep the ESBuild details for the logs. (the `errors` array).
    // If the error is already an AbortError, we can just rethrow it.
    if (error instanceof AbortError) {
      throw error

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +339 to +345
async buildForBundle(options: ExtensionBuildOptions, bundleDirectory: string, outputId?: string) {
this.outputPath = this.getOutputPathForDirectory(bundleDirectory, outputId)
await this.build(options)
const bundleOutputPath = this.getOutputPathForDirectory(bundleDirectory, outputId)
await this.build({...options, bundleOutputPath})

if (this.isFunctionExtension && (await fileExists(this.outputPath))) {
await touchFile(bundleOutputPath)
await copyFile(this.outputPath, bundleOutputPath)
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

buildForBundle no longer overrides extension.outputPath and instead passes bundleOutputPath via options, but several deploy build steps write to extension.outputPath directly (e.g., bundle_theme, include_assets, create_tax_stub). As a result, buildForBundle will likely emit artifacts into the extension’s local output path instead of the bundle directory, producing incomplete/incorrect bundles. Either restore a scoped outputPath override for the duration of buildForBundle (and restore afterward), or update all step executors/helpers that use extension.outputPath to prefer options.bundleOutputPath when present.

Copilot uses AI. Check for mistakes.
Comment on lines +59 to +61
* bundle directory. When set, build functions write their final artifact here
* instead of extension.outputPath. This avoids mutating extension.outputPath at
* runtime.
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The new bundleOutputPath docstring says “build functions write their final artifact here instead of extension.outputPath”, but buildFunctionExtension still writes to extension.outputPath (and only uses bundleOutputPath for bundling/base64 output). Please adjust the comment to match the actual behavior (or update the implementation to truly redirect build output when bundleOutputPath is set).

Suggested change
* bundle directory. When set, build functions write their final artifact here
* instead of extension.outputPath. This avoids mutating extension.outputPath at
* runtime.
* bundle directory used for bundled output artifacts.
*
* Some build flows may still write their final artifact to extension.outputPath,
* so this option should not be treated as a universal replacement for that path.

Copilot uses AI. Check for mistakes.
Comment on lines 352 to +357

async copyIntoBundle(options: ExtensionBuildOptions, bundleDirectory: string, extensionUuid?: string) {
const defaultOutputPath = this.outputPath

this.outputPath = this.getOutputPathForDirectory(bundleDirectory, extensionUuid)

const buildMode = this.specification.buildConfig.mode
const bundleOutputPath = this.getOutputPathForDirectory(bundleDirectory, extensionUuid)

if (this.isThemeExtension) {
await bundleThemeExtension(this, options)
} else if (buildMode !== 'none') {
outputDebug(`Will copy pre-built file from ${defaultOutputPath} to ${this.outputPath}`)
if (await fileExists(defaultOutputPath)) {
await copyFile(defaultOutputPath, this.outputPath)

if (buildMode === 'function') {
await bundleFunctionExtension(this.outputPath, this.outputPath)
await bundleThemeExtension(this, {...options, bundleOutputPath})
Copy link

Copilot AI Apr 9, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In copyIntoBundle, touchFile(bundleOutputPath) assumes the bundle output is a file path. For specs where outputRelativePath is empty (common for contract-based modules), getOutputPathForDirectory returns a directory path; touching it will create a file at that path and can break the subsequent directory copy. Consider removing touchFile here (fs-extra copy creates parent dirs), or switching to ensuring a directory when bundleOutputPath is a directory.

Copilot uses AI. Check for mistakes.
@isaacroldan isaacroldan force-pushed the 04-09-fix_stop_base64-encoding_dist_index.wasm_during_function_builds branch 2 times, most recently from 7dbfe03 to 5b73b35 Compare April 9, 2026 18:06
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 <isaac.roldan@shopify.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
@isaacroldan isaacroldan force-pushed the 04-09-fix_stop_base64-encoding_dist_index.wasm_during_function_builds branch from 5b73b35 to 7a352b0 Compare April 10, 2026 09:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants