From b403d970b3bbfd58a89d7df9befdaa465faa0b81 Mon Sep 17 00:00:00 2001 From: Alex Bradley Date: Mon, 9 Dec 2024 13:58:49 -0500 Subject: [PATCH 1/9] download and run wasm-opt --- .../app/src/cli/services/build/extension.ts | 7 ++++- .../app/src/cli/services/function/binaries.ts | 31 +++++++++++++++++++ .../app/src/cli/services/function/build.ts | 23 ++++++++++++-- 3 files changed, 58 insertions(+), 3 deletions(-) diff --git a/packages/app/src/cli/services/build/extension.ts b/packages/app/src/cli/services/build/extension.ts index 4266136eb6e..d73782b9423 100644 --- a/packages/app/src/cli/services/build/extension.ts +++ b/packages/app/src/cli/services/build/extension.ts @@ -1,7 +1,7 @@ import {runThemeCheck} from './theme-check.js' import {AppInterface} from '../../models/app/app.js' import {bundleExtension, bundleFlowTemplateExtension} from '../extensions/bundle.js' -import {buildJSFunction} from '../function/build.js' +import {buildJSFunction, runWasmOpt} from '../function/build.js' import {ExtensionInstance} from '../../models/extensions/extension-instance.js' import {FunctionConfigType} from '../../models/extensions/specifications/function.js' import {exec} from '@shopify/cli-kit/node/system' @@ -175,6 +175,11 @@ export async function buildFunctionExtension( } else { await buildOtherFunction(extension, options) } + + if (fileExistsSync(extension.outputPath)) { + await runWasmOpt(extension.outputPath) + } + if (fileExistsSync(extension.outputPath) && bundlePath !== extension.outputPath) { const base64Contents = await readFile(extension.outputPath, {encoding: 'base64'}) await touchFile(bundlePath) diff --git a/packages/app/src/cli/services/function/binaries.ts b/packages/app/src/cli/services/function/binaries.ts index d702c1f1a1c..97470ea588a 100644 --- a/packages/app/src/cli/services/function/binaries.ts +++ b/packages/app/src/cli/services/function/binaries.ts @@ -15,6 +15,8 @@ const JAVY_VERSION = 'v4.0.0' // match the plugin version used in the function-runner version specified above. const JAVY_PLUGIN_VERSION = 'v3.2.0' +const BINARYEN_VERSION = '120.0.0' + interface DownloadableBinary { path: string name: string @@ -107,9 +109,30 @@ class JavyPlugin implements DownloadableBinary { } } +class WasmOptExecutable implements DownloadableBinary { + readonly name: string + readonly version: string + readonly path: string + + constructor(name: string, version: string) { + this.name = name + this.version = version + this.path = joinPath(dirname(fileURLToPath(import.meta.url)), '..', 'bin', name) + } + + downloadUrl(_processPlatform: string, _processArch: string) { + return `https://cdn.jsdelivr.net/npm/binaryen@${this.version}/bin/wasm-opt` + } + + async processResponse(responseStream: PipelineSource, outputStream: fs.WriteStream): Promise { + await stream.pipeline(responseStream, outputStream) + } +} + let _javy: DownloadableBinary let _javyPlugin: DownloadableBinary let _functionRunner: DownloadableBinary +let _wasmOpt: DownloadableBinary export function javyBinary() { if (!_javy) { @@ -132,6 +155,14 @@ export function functionRunnerBinary() { return _functionRunner } +export function wasmOptBinary() { + if (!_wasmOpt) { + _wasmOpt = new WasmOptExecutable('wasm-opt.cjs', BINARYEN_VERSION) + } + + return _wasmOpt +} + export async function downloadBinary(bin: DownloadableBinary) { const isDownloaded = await fileExists(bin.path) if (isDownloaded) { diff --git a/packages/app/src/cli/services/function/build.ts b/packages/app/src/cli/services/function/build.ts index ed35eb1aa1b..09272c135ec 100644 --- a/packages/app/src/cli/services/function/build.ts +++ b/packages/app/src/cli/services/function/build.ts @@ -1,5 +1,5 @@ /* eslint-disable @typescript-eslint/no-non-null-assertion */ -import {downloadBinary, javyBinary, javyPluginBinary} from './binaries.js' +import {downloadBinary, javyBinary, javyPluginBinary, wasmOptBinary} from './binaries.js' import {ExtensionInstance} from '../../models/extensions/extension-instance.js' import {FunctionConfigType} from '../../models/extensions/specifications/function.js' import {AppInterface} from '../../models/app/app.js' @@ -7,7 +7,7 @@ import {EsbuildEnvVarRegex} from '../../constants.js' import {hyphenate, camelize} from '@shopify/cli-kit/common/string' import {outputContent, outputDebug, outputToken} from '@shopify/cli-kit/node/output' import {exec} from '@shopify/cli-kit/node/system' -import {joinPath} from '@shopify/cli-kit/node/path' +import {dirname, joinPath} from '@shopify/cli-kit/node/path' import {build as esBuild, BuildResult} from 'esbuild' import {findPathUp, inTemporaryDirectory, writeFile} from '@shopify/cli-kit/node/fs' import {AbortSignal} from '@shopify/cli-kit/node/abort' @@ -16,6 +16,7 @@ import {pickBy} from '@shopify/cli-kit/common/object' import {runWithTimer} from '@shopify/cli-kit/node/metadata' import {AbortError} from '@shopify/cli-kit/node/error' import {Writable} from 'stream' +import {execSync} from 'child_process' interface JSFunctionBuildOptions { stdout: Writable @@ -186,6 +187,24 @@ function getESBuildOptions( return esbuildOptions } +export async function downloadWasmOpt() { + const wasmOpt = wasmOptBinary() + await Promise.all([downloadBinary(wasmOpt)]) +} + +export async function runWasmOpt(modulePath: string) { + await downloadWasmOpt() + + const wasmOptDir = dirname(wasmOptBinary().path) + const command = `node wasm-opt.cjs ${modulePath}` + const args = ['-Oz', '--enable-bulk-memory', '-o', modulePath].join(' ') + + console.log('Attempting to run wasm-opt with the following command:') + console.log(`${command} ${args}`) + + execSync(`${command} ${args}`, {cwd: wasmOptDir}) +} + export async function runJavy( fun: ExtensionInstance, options: JSFunctionBuildOptions, From 30a0c08584047e19ce24b9309f2b6525c31a10fa Mon Sep 17 00:00:00 2001 From: Alex Bradley Date: Mon, 9 Dec 2024 16:14:31 -0500 Subject: [PATCH 2/9] wasm opt binary test --- .../cli/services/function/binaries.test.ts | 34 ++++++++++++++++++- 1 file changed, 33 insertions(+), 1 deletion(-) diff --git a/packages/app/src/cli/services/function/binaries.test.ts b/packages/app/src/cli/services/function/binaries.test.ts index 7656cd9e2f0..4c53387e348 100644 --- a/packages/app/src/cli/services/function/binaries.test.ts +++ b/packages/app/src/cli/services/function/binaries.test.ts @@ -1,4 +1,4 @@ -import {javyBinary, functionRunnerBinary, downloadBinary, javyPluginBinary} from './binaries.js' +import {javyBinary, functionRunnerBinary, downloadBinary, javyPluginBinary, wasmOptBinary} from './binaries.js' import {fetch, Response} from '@shopify/cli-kit/node/http' import {fileExists, removeFile} from '@shopify/cli-kit/node/fs' import {describe, expect, test, vi} from 'vitest' @@ -279,3 +279,35 @@ describe('functionRunner', () => { await expect(fileExists(functionRunner.path)).resolves.toBeTruthy() }) }) + +describe('wasm-opt', () => { + const wasmOpt = wasmOptBinary() + + test('properties are set correctly', () => { + expect(wasmOpt.name).toBe('wasm-opt.cjs') + expect(wasmOpt.version).match(/\d.\d.\d$/) + expect(wasmOpt.path).toMatch(/(\/|\\)wasm-opt.cjs$/) + }) + + test('downloadUrl returns the correct URL', () => { + // When + const url = wasmOpt.downloadUrl('', '') + + // Then + expect(url).toMatch(/https:\/\/cdn.jsdelivr.net\/npm\/binaryen@\d{3}\.\d\.\d\/bin\/wasm-opt/) + }) + + test('downloads wasm-opt', async () => { + // Given + await removeFile(wasmOpt.path) + await expect(fileExists(wasmOpt.path)).resolves.toBeFalsy() + vi.mocked(fetch).mockResolvedValue(new Response('wasm-opt')) + + // When + await downloadBinary(wasmOpt) + + // Then + expect(fetch).toHaveBeenCalledOnce() + await expect(fileExists(wasmOpt.path)).resolves.toBeTruthy() + }) +}) From 87dc62e7a74582dc2884b8ff6bc6fb1b363bdc13 Mon Sep 17 00:00:00 2001 From: Alex Bradley Date: Mon, 9 Dec 2024 16:40:50 -0500 Subject: [PATCH 3/9] use exec over execSync --- .../app/src/cli/services/function/build.ts | 19 +++++++++++++------ 1 file changed, 13 insertions(+), 6 deletions(-) diff --git a/packages/app/src/cli/services/function/build.ts b/packages/app/src/cli/services/function/build.ts index 09272c135ec..74599639e99 100644 --- a/packages/app/src/cli/services/function/build.ts +++ b/packages/app/src/cli/services/function/build.ts @@ -16,7 +16,6 @@ import {pickBy} from '@shopify/cli-kit/common/object' import {runWithTimer} from '@shopify/cli-kit/node/metadata' import {AbortError} from '@shopify/cli-kit/node/error' import {Writable} from 'stream' -import {execSync} from 'child_process' interface JSFunctionBuildOptions { stdout: Writable @@ -196,13 +195,21 @@ export async function runWasmOpt(modulePath: string) { await downloadWasmOpt() const wasmOptDir = dirname(wasmOptBinary().path) - const command = `node wasm-opt.cjs ${modulePath}` - const args = ['-Oz', '--enable-bulk-memory', '-o', modulePath].join(' ') - console.log('Attempting to run wasm-opt with the following command:') - console.log(`${command} ${args}`) + const command = `node` + const args = [ + // invoke the js-wrapped wasm-opt binary + wasmOptBinary().name, + modulePath, + // pass these options to wasm-opt + '-Oz', + '--enable-bulk-memory', + // overwrite our existing module with the optimized version + '-o', + modulePath, + ] - execSync(`${command} ${args}`, {cwd: wasmOptDir}) + await exec(command, args, {cwd: wasmOptDir}) } export async function runJavy( From b795a278fcc7e3649932855e21b9d6f3203fb1bb Mon Sep 17 00:00:00 2001 From: Alex Bradley Date: Tue, 10 Dec 2024 11:06:11 -0500 Subject: [PATCH 4/9] add wasm-opt build step test --- .../src/cli/services/function/build.test.ts | 27 ++++++++++++++++--- .../app/src/cli/services/function/build.ts | 8 ++---- 2 files changed, 26 insertions(+), 9 deletions(-) diff --git a/packages/app/src/cli/services/function/build.test.ts b/packages/app/src/cli/services/function/build.test.ts index 677900c3364..2f67bab9b2f 100644 --- a/packages/app/src/cli/services/function/build.test.ts +++ b/packages/app/src/cli/services/function/build.test.ts @@ -1,9 +1,9 @@ -import {buildGraphqlTypes, bundleExtension, runJavy, ExportJavyBuilder, jsExports} from './build.js' -import {javyBinary, javyPluginBinary} from './binaries.js' +import {buildGraphqlTypes, bundleExtension, runJavy, ExportJavyBuilder, jsExports, runWasmOpt} from './build.js' +import {javyBinary, javyPluginBinary, wasmOptBinary} from './binaries.js' import {testApp, testFunctionExtension} from '../../models/app/app.test-data.js' import {beforeEach, describe, expect, test, vi} from 'vitest' import {exec} from '@shopify/cli-kit/node/system' -import {joinPath} from '@shopify/cli-kit/node/path' +import {dirname, joinPath} from '@shopify/cli-kit/node/path' import {inTemporaryDirectory, mkdir, writeFile, removeFile} from '@shopify/cli-kit/node/fs' import {build as esBuild} from 'esbuild' @@ -183,6 +183,27 @@ describe('runJavy', () => { }) }) +describe('runWasmOpt', () => { + test('runs wasm-opt on the module', async () => { + // Given + const ourFunction = await testFunctionExtension() + const modulePath = ourFunction.outputPath + + // When + const got = runWasmOpt(modulePath) + + // Then + await expect(got).resolves.toBeUndefined() + expect(exec).toHaveBeenCalledWith( + 'node', + [wasmOptBinary().name, modulePath, '-Oz', '--enable-bulk-memory', '-o', modulePath], + { + cwd: dirname(wasmOptBinary().path), + }, + ) + }) +}) + describe('ExportJavyBuilder', () => { const exports = ['foo-bar', 'foo-baz'] const builder = new ExportJavyBuilder(exports) diff --git a/packages/app/src/cli/services/function/build.ts b/packages/app/src/cli/services/function/build.ts index 74599639e99..b568a6bb76f 100644 --- a/packages/app/src/cli/services/function/build.ts +++ b/packages/app/src/cli/services/function/build.ts @@ -186,13 +186,9 @@ function getESBuildOptions( return esbuildOptions } -export async function downloadWasmOpt() { - const wasmOpt = wasmOptBinary() - await Promise.all([downloadBinary(wasmOpt)]) -} - export async function runWasmOpt(modulePath: string) { - await downloadWasmOpt() + const wasmOpt = wasmOptBinary() + await downloadBinary(wasmOpt) const wasmOptDir = dirname(wasmOptBinary().path) From ad762ad57486c1f5677cef3212c5db61bbf613d8 Mon Sep 17 00:00:00 2001 From: Alex Bradley Date: Tue, 10 Dec 2024 14:16:53 -0500 Subject: [PATCH 5/9] add disable-wasm-opt flag --- .../app-function-build.interface.ts | 6 +++ .../generated/generated_docs_data.json | 11 ++++- .../src/cli/commands/app/function/build.ts | 6 +++ .../src/cli/services/build/extension.test.ts | 44 ++++++++++++++++++- .../app/src/cli/services/build/extension.ts | 6 ++- packages/cli/README.md | 4 +- packages/cli/oclif.manifest.json | 7 +++ 7 files changed, 79 insertions(+), 5 deletions(-) diff --git a/docs-shopify.dev/commands/interfaces/app-function-build.interface.ts b/docs-shopify.dev/commands/interfaces/app-function-build.interface.ts index c6795e1e713..8bbc72a4d6f 100644 --- a/docs-shopify.dev/commands/interfaces/app-function-build.interface.ts +++ b/docs-shopify.dev/commands/interfaces/app-function-build.interface.ts @@ -12,6 +12,12 @@ export interface appfunctionbuild { */ '-c, --config '?: string + /** + * Disables the automatic optimization by wasm-opt after a function build + * @environment SHOPIFY_FLAG_DISABLE_WASM_OPT + */ + '--disable-wasm-opt'?: '' + /** * Disable color output. * @environment SHOPIFY_FLAG_NO_COLOR diff --git a/docs-shopify.dev/generated/generated_docs_data.json b/docs-shopify.dev/generated/generated_docs_data.json index a63ead60ab6..6e1f0627d0d 100644 --- a/docs-shopify.dev/generated/generated_docs_data.json +++ b/docs-shopify.dev/generated/generated_docs_data.json @@ -816,6 +816,15 @@ "isOptional": true, "environmentValue": "SHOPIFY_FLAG_CLIENT_ID" }, + { + "filePath": "docs-shopify.dev/commands/interfaces/app-function-build.interface.ts", + "syntaxKind": "PropertySignature", + "name": "--disable-wasm-opt", + "value": "\"\"", + "description": "Disables the automatic optimization by wasm-opt after a function build", + "isOptional": true, + "environmentValue": "SHOPIFY_FLAG_DISABLE_WASM_OPT" + }, { "filePath": "docs-shopify.dev/commands/interfaces/app-function-build.interface.ts", "syntaxKind": "PropertySignature", @@ -862,7 +871,7 @@ "environmentValue": "SHOPIFY_FLAG_APP_CONFIG" } ], - "value": "export interface appfunctionbuild {\n /**\n * The Client ID of your app.\n * @environment SHOPIFY_FLAG_CLIENT_ID\n */\n '--client-id '?: string\n\n /**\n * The name of the app configuration.\n * @environment SHOPIFY_FLAG_APP_CONFIG\n */\n '-c, --config '?: string\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * The path to your function directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path '?: string\n\n /**\n * Reset all your settings.\n * @environment SHOPIFY_FLAG_RESET\n */\n '--reset'?: ''\n\n /**\n * Increase the verbosity of the output.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n}" + "value": "export interface appfunctionbuild {\n /**\n * The Client ID of your app.\n * @environment SHOPIFY_FLAG_CLIENT_ID\n */\n '--client-id '?: string\n\n /**\n * The name of the app configuration.\n * @environment SHOPIFY_FLAG_APP_CONFIG\n */\n '-c, --config '?: string\n\n /**\n * Disables the automatic optimization by wasm-opt after a function build\n * @environment SHOPIFY_FLAG_DISABLE_WASM_OPT\n */\n '--disable-wasm-opt'?: ''\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * The path to your function directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path '?: string\n\n /**\n * Reset all your settings.\n * @environment SHOPIFY_FLAG_RESET\n */\n '--reset'?: ''\n\n /**\n * Increase the verbosity of the output.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n}" } } } diff --git a/packages/app/src/cli/commands/app/function/build.ts b/packages/app/src/cli/commands/app/function/build.ts index 0ac57587ab4..9857e6985ef 100644 --- a/packages/app/src/cli/commands/app/function/build.ts +++ b/packages/app/src/cli/commands/app/function/build.ts @@ -4,6 +4,7 @@ import {appFlags} from '../../../flags.js' import AppCommand, {AppCommandOutput} from '../../../utilities/app-command.js' import {globalFlags} from '@shopify/cli-kit/node/cli' import {renderSuccess} from '@shopify/cli-kit/node/ui' +import {Flags} from '@oclif/core' export default class FunctionBuild extends AppCommand { static summary = 'Compile a function to wasm.' @@ -16,6 +17,10 @@ export default class FunctionBuild extends AppCommand { ...globalFlags, ...appFlags, ...functionFlags, + 'disable-wasm-opt': Flags.boolean({ + description: 'Disables the automatic optimization by wasm-opt after a function build', + env: 'SHOPIFY_FLAG_DISABLE_WASM_OPT', + }), } public async run(): Promise { @@ -33,6 +38,7 @@ export default class FunctionBuild extends AppCommand { stderr: process.stderr, useTasks: true, environment: 'production', + disableWasmOpt: flags['disable-wasm-opt'], }) renderSuccess({headline: 'Function built successfully.'}) return app diff --git a/packages/app/src/cli/services/build/extension.test.ts b/packages/app/src/cli/services/build/extension.test.ts index 628c3095aa6..375694e2609 100644 --- a/packages/app/src/cli/services/build/extension.test.ts +++ b/packages/app/src/cli/services/build/extension.test.ts @@ -1,16 +1,18 @@ import {buildFunctionExtension} from './extension.js' import {testFunctionExtension} from '../../models/app/app.test-data.js' -import {buildJSFunction} from '../function/build.js' +import {buildJSFunction, runWasmOpt} from '../function/build.js' import {ExtensionInstance} from '../../models/extensions/extension-instance.js' import {FunctionConfigType} from '../../models/extensions/specifications/function.js' 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} from '@shopify/cli-kit/node/fs' vi.mock('@shopify/cli-kit/node/system') vi.mock('../function/build.js') vi.mock('proper-lockfile') +vi.mock('@shopify/cli-kit/node/fs') describe('buildFunctionExtension', () => { let extension: ExtensionInstance @@ -138,6 +140,46 @@ describe('buildFunctionExtension', () => { expect(releaseLock).toHaveBeenCalled() }) + test('performs wasm-opt execution by default', async () => { + // Given + vi.mocked(fileExistsSync).mockResolvedValue(true) + + // When + await expect( + buildFunctionExtension(extension, { + stdout, + stderr, + signal, + app, + environment: 'production', + }), + ).resolves.toBeUndefined() + + // Then + expect(runWasmOpt).toHaveBeenCalled() + }) + + test('skips wasm-opt execution when the disable-wasm-opt is true', async () => { + // Given + vi.mocked(fileExistsSync).mockResolvedValue(true) + extension.configuration.build.command = './scripts/build.sh argument' + + // When + await expect( + buildFunctionExtension(extension, { + stdout, + stderr, + signal, + app, + environment: 'production', + disableWasmOpt: true, + }), + ).resolves.toBeUndefined() + + // Then + expect(runWasmOpt).not.toHaveBeenCalled() + }) + test('fails when build lock cannot be acquired', async () => { // Given vi.mocked(lockfile.lock).mockRejectedValue('failed to acquire lock') diff --git a/packages/app/src/cli/services/build/extension.ts b/packages/app/src/cli/services/build/extension.ts index d73782b9423..4d486c9f933 100644 --- a/packages/app/src/cli/services/build/extension.ts +++ b/packages/app/src/cli/services/build/extension.ts @@ -139,7 +139,9 @@ export async function buildUIExtension(extension: ExtensionInstance, options: Ex options.stdout.write(`${extension.localIdentifier} successfully built`) } -type BuildFunctionExtensionOptions = ExtensionBuildOptions +interface BuildFunctionExtensionOptions extends ExtensionBuildOptions { + disableWasmOpt?: boolean +} /** * Builds a function extension @@ -176,7 +178,7 @@ export async function buildFunctionExtension( await buildOtherFunction(extension, options) } - if (fileExistsSync(extension.outputPath)) { + if (fileExistsSync(extension.outputPath) && !options.disableWasmOpt) { await runWasmOpt(extension.outputPath) } diff --git a/packages/cli/README.md b/packages/cli/README.md index 587e511fb83..cb8ee8c069f 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -328,11 +328,13 @@ Compile a function to wasm. ``` USAGE - $ shopify app function build [--client-id | -c ] [--no-color] [--path ] [--reset | ] [--verbose] + $ shopify app function build [--client-id | -c ] [--disable-wasm-opt] [--no-color] [--path ] + [--reset | ] [--verbose] FLAGS -c, --config= The name of the app configuration. --client-id= The Client ID of your app. + --disable-wasm-opt Disables the automatic optimization by wasm-opt after a function build --no-color Disable color output. --path= The path to your function directory. --reset Reset all your settings. diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json index 78bfbc0e701..7decb536597 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -782,6 +782,13 @@ "name": "config", "type": "option" }, + "disable-wasm-opt": { + "allowNo": false, + "description": "Disables the automatic optimization by wasm-opt after a function build", + "env": "SHOPIFY_FLAG_DISABLE_WASM_OPT", + "name": "disable-wasm-opt", + "type": "boolean" + }, "no-color": { "allowNo": false, "description": "Disable color output.", From 62ad101203150b43d815d77b336e5b51a469049f Mon Sep 17 00:00:00 2001 From: Alex Bradley Date: Tue, 10 Dec 2024 14:48:35 -0500 Subject: [PATCH 6/9] add strip-debug arg for wasm-opt --- packages/app/src/cli/services/function/build.test.ts | 2 +- packages/app/src/cli/services/function/build.ts | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/packages/app/src/cli/services/function/build.test.ts b/packages/app/src/cli/services/function/build.test.ts index 2f67bab9b2f..af07517f4b2 100644 --- a/packages/app/src/cli/services/function/build.test.ts +++ b/packages/app/src/cli/services/function/build.test.ts @@ -196,7 +196,7 @@ describe('runWasmOpt', () => { await expect(got).resolves.toBeUndefined() expect(exec).toHaveBeenCalledWith( 'node', - [wasmOptBinary().name, modulePath, '-Oz', '--enable-bulk-memory', '-o', modulePath], + [wasmOptBinary().name, modulePath, '-Oz', '--enable-bulk-memory', '--strip-debug', '-o', modulePath], { cwd: dirname(wasmOptBinary().path), }, diff --git a/packages/app/src/cli/services/function/build.ts b/packages/app/src/cli/services/function/build.ts index b568a6bb76f..e30c62cec79 100644 --- a/packages/app/src/cli/services/function/build.ts +++ b/packages/app/src/cli/services/function/build.ts @@ -200,6 +200,7 @@ export async function runWasmOpt(modulePath: string) { // pass these options to wasm-opt '-Oz', '--enable-bulk-memory', + '--strip-debug', // overwrite our existing module with the optimized version '-o', modulePath, From 3139db8724b3ec5a440f2ce73b8d46a51b1871d0 Mon Sep 17 00:00:00 2001 From: Alex Bradley Date: Tue, 10 Dec 2024 15:59:36 -0500 Subject: [PATCH 7/9] Use wasm_opt from config --- packages/app/src/cli/models/app/app.test-data.ts | 1 + packages/app/src/cli/models/app/app.test.ts | 1 + .../models/extensions/extension-instance.test.ts | 14 +++++++++++--- .../extensions/specifications/function.test.ts | 1 + .../models/extensions/specifications/function.ts | 1 + .../app/src/cli/services/build/extension.test.ts | 3 ++- packages/app/src/cli/services/build/extension.ts | 3 ++- .../src/cli/services/context/id-matching.test.ts | 1 + .../context/identifiers-extensions.test.ts | 1 + .../app/src/cli/services/function/replay.test.ts | 1 + .../app/src/cli/services/generate-schema.test.ts | 2 ++ 11 files changed, 24 insertions(+), 5 deletions(-) diff --git a/packages/app/src/cli/models/app/app.test-data.ts b/packages/app/src/cli/models/app/app.test-data.ts index 4cac2ecdbd9..1b715f4ab68 100644 --- a/packages/app/src/cli/models/app/app.test-data.ts +++ b/packages/app/src/cli/models/app/app.test-data.ts @@ -501,6 +501,7 @@ function defaultFunctionConfiguration(): FunctionConfigType { build: { command: 'echo "hello world"', watch: ['src/**/*.rs'], + wasm_opt: true, }, api_version: '2022-07', configuration_ui: true, diff --git a/packages/app/src/cli/models/app/app.test.ts b/packages/app/src/cli/models/app/app.test.ts index 2d9280efa9f..da46653b30c 100644 --- a/packages/app/src/cli/models/app/app.test.ts +++ b/packages/app/src/cli/models/app/app.test.ts @@ -205,6 +205,7 @@ describe('validateFunctionExtensionsWithUiHandle', () => { description: 'description', build: { command: 'echo "hello world"', + wasm_opt: true, }, api_version: '2022-07', configuration_ui: true, 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 4df163e4cd3..2faab64263d 100644 --- a/packages/app/src/cli/models/extensions/extension-instance.test.ts +++ b/packages/app/src/cli/models/extensions/extension-instance.test.ts @@ -32,7 +32,9 @@ function functionConfiguration(): FunctionConfigType { api_version: '2023-07', configuration_ui: true, metafields: [], - build: {}, + build: { + wasm_opt: true, + }, } } @@ -41,6 +43,7 @@ describe('watchPaths', async () => { const config = functionConfiguration() config.build = { watch: 'src/single-path.foo', + wasm_opt: true, } const extensionInstance = await testFunctionExtension({ config, @@ -54,7 +57,9 @@ describe('watchPaths', async () => { test('returns default paths for javascript', async () => { const config = functionConfiguration() - config.build = {} + config.build = { + wasm_opt: true, + } const extensionInstance = await testFunctionExtension({ config, entryPath: 'src/index.js', @@ -86,6 +91,7 @@ describe('watchPaths', async () => { const config = functionConfiguration() config.build = { watch: ['src/**/*.rs', 'src/**/*.foo'], + wasm_opt: true, } const extensionInstance = await testFunctionExtension({ config, @@ -103,7 +109,9 @@ describe('watchPaths', async () => { test('returns null if not javascript and not configured', async () => { const config = functionConfiguration() - config.build = {} + config.build = { + wasm_opt: true, + } const extensionInstance = await testFunctionExtension({ config, }) diff --git a/packages/app/src/cli/models/extensions/specifications/function.test.ts b/packages/app/src/cli/models/extensions/specifications/function.test.ts index 7ab0c36a714..3dd0061228c 100644 --- a/packages/app/src/cli/models/extensions/specifications/function.test.ts +++ b/packages/app/src/cli/models/extensions/specifications/function.test.ts @@ -20,6 +20,7 @@ describe('functionConfiguration', () => { build: { command: 'make build', path: 'dist/index.wasm', + wasm_opt: true, }, ui: { paths: { diff --git a/packages/app/src/cli/models/extensions/specifications/function.ts b/packages/app/src/cli/models/extensions/specifications/function.ts index 9f900625701..3250c960d3d 100644 --- a/packages/app/src/cli/models/extensions/specifications/function.ts +++ b/packages/app/src/cli/models/extensions/specifications/function.ts @@ -25,6 +25,7 @@ const FunctionExtensionSchema = BaseSchema.extend({ .optional(), path: zod.string().optional(), watch: zod.union([zod.string(), zod.string().array()]).optional(), + wasm_opt: zod.boolean().optional().default(true), }), configuration_ui: zod.boolean().optional().default(true), ui: zod diff --git a/packages/app/src/cli/services/build/extension.test.ts b/packages/app/src/cli/services/build/extension.test.ts index 375694e2609..2328e1da496 100644 --- a/packages/app/src/cli/services/build/extension.test.ts +++ b/packages/app/src/cli/services/build/extension.test.ts @@ -28,6 +28,7 @@ describe('buildFunctionExtension', () => { build: { command: 'make build', path: 'dist/index.wasm', + wasm_opt: true, }, configuration_ui: true, api_version: '2022-07', @@ -162,7 +163,7 @@ describe('buildFunctionExtension', () => { test('skips wasm-opt execution when the disable-wasm-opt is true', async () => { // Given vi.mocked(fileExistsSync).mockResolvedValue(true) - extension.configuration.build.command = './scripts/build.sh argument' + extension.configuration.build.wasm_opt = false // When await expect( diff --git a/packages/app/src/cli/services/build/extension.ts b/packages/app/src/cli/services/build/extension.ts index 4d486c9f933..c21e79e8c34 100644 --- a/packages/app/src/cli/services/build/extension.ts +++ b/packages/app/src/cli/services/build/extension.ts @@ -178,7 +178,8 @@ export async function buildFunctionExtension( await buildOtherFunction(extension, options) } - if (fileExistsSync(extension.outputPath) && !options.disableWasmOpt) { + const wasmOpt = (extension as ExtensionInstance).configuration.build.wasm_opt + if (fileExistsSync(extension.outputPath) && wasmOpt) { await runWasmOpt(extension.outputPath) } diff --git a/packages/app/src/cli/services/context/id-matching.test.ts b/packages/app/src/cli/services/context/id-matching.test.ts index dc5217bb1bd..82e83ef61a3 100644 --- a/packages/app/src/cli/services/context/id-matching.test.ts +++ b/packages/app/src/cli/services/context/id-matching.test.ts @@ -233,6 +233,7 @@ beforeAll(async () => { build: { command: 'make build', path: 'dist/index.wasm', + wasm_opt: true, }, configuration_ui: false, api_version: '2022-07', diff --git a/packages/app/src/cli/services/context/identifiers-extensions.test.ts b/packages/app/src/cli/services/context/identifiers-extensions.test.ts index 29fb2f9f6b0..43265d8555a 100644 --- a/packages/app/src/cli/services/context/identifiers-extensions.test.ts +++ b/packages/app/src/cli/services/context/identifiers-extensions.test.ts @@ -263,6 +263,7 @@ beforeAll(async () => { build: { command: 'make build', path: 'dist/index.wasm', + wasm_opt: true, }, metafields: [], configuration_ui: false, diff --git a/packages/app/src/cli/services/function/replay.test.ts b/packages/app/src/cli/services/function/replay.test.ts index 0da743c056c..9ffc5f2fe3a 100644 --- a/packages/app/src/cli/services/function/replay.test.ts +++ b/packages/app/src/cli/services/function/replay.test.ts @@ -29,6 +29,7 @@ describe('replay', () => { build: { command: 'make build', path: 'dist/index.wasm', + wasm_opt: true, }, configuration_ui: true, api_version: '2022-07', diff --git a/packages/app/src/cli/services/generate-schema.test.ts b/packages/app/src/cli/services/generate-schema.test.ts index 1d034e47dab..2c1aa79dced 100644 --- a/packages/app/src/cli/services/generate-schema.test.ts +++ b/packages/app/src/cli/services/generate-schema.test.ts @@ -93,6 +93,7 @@ describe('generateSchemaService', () => { type: 'api_type', build: { command: 'echo "hello world"', + wasm_opt: true, }, api_version: 'unstable', configuration_ui: true, @@ -148,6 +149,7 @@ describe('generateSchemaService', () => { ], build: { command: 'echo "hello world"', + wasm_opt: true, }, api_version: 'unstable', configuration_ui: true, From f47844ac3ad2f40826f12d53952866cc9ec318b5 Mon Sep 17 00:00:00 2001 From: Alex Bradley Date: Tue, 10 Dec 2024 16:04:10 -0500 Subject: [PATCH 8/9] remove flag, prefer config --- .../interfaces/app-function-build.interface.ts | 6 ------ docs-shopify.dev/generated/generated_docs_data.json | 11 +---------- packages/app/src/cli/commands/app/function/build.ts | 6 ------ packages/app/src/cli/services/build/extension.test.ts | 1 - packages/app/src/cli/services/build/extension.ts | 4 +--- packages/cli/README.md | 4 +--- packages/cli/oclif.manifest.json | 7 ------- 7 files changed, 3 insertions(+), 36 deletions(-) diff --git a/docs-shopify.dev/commands/interfaces/app-function-build.interface.ts b/docs-shopify.dev/commands/interfaces/app-function-build.interface.ts index 8bbc72a4d6f..c6795e1e713 100644 --- a/docs-shopify.dev/commands/interfaces/app-function-build.interface.ts +++ b/docs-shopify.dev/commands/interfaces/app-function-build.interface.ts @@ -12,12 +12,6 @@ export interface appfunctionbuild { */ '-c, --config '?: string - /** - * Disables the automatic optimization by wasm-opt after a function build - * @environment SHOPIFY_FLAG_DISABLE_WASM_OPT - */ - '--disable-wasm-opt'?: '' - /** * Disable color output. * @environment SHOPIFY_FLAG_NO_COLOR diff --git a/docs-shopify.dev/generated/generated_docs_data.json b/docs-shopify.dev/generated/generated_docs_data.json index 6e1f0627d0d..a63ead60ab6 100644 --- a/docs-shopify.dev/generated/generated_docs_data.json +++ b/docs-shopify.dev/generated/generated_docs_data.json @@ -816,15 +816,6 @@ "isOptional": true, "environmentValue": "SHOPIFY_FLAG_CLIENT_ID" }, - { - "filePath": "docs-shopify.dev/commands/interfaces/app-function-build.interface.ts", - "syntaxKind": "PropertySignature", - "name": "--disable-wasm-opt", - "value": "\"\"", - "description": "Disables the automatic optimization by wasm-opt after a function build", - "isOptional": true, - "environmentValue": "SHOPIFY_FLAG_DISABLE_WASM_OPT" - }, { "filePath": "docs-shopify.dev/commands/interfaces/app-function-build.interface.ts", "syntaxKind": "PropertySignature", @@ -871,7 +862,7 @@ "environmentValue": "SHOPIFY_FLAG_APP_CONFIG" } ], - "value": "export interface appfunctionbuild {\n /**\n * The Client ID of your app.\n * @environment SHOPIFY_FLAG_CLIENT_ID\n */\n '--client-id '?: string\n\n /**\n * The name of the app configuration.\n * @environment SHOPIFY_FLAG_APP_CONFIG\n */\n '-c, --config '?: string\n\n /**\n * Disables the automatic optimization by wasm-opt after a function build\n * @environment SHOPIFY_FLAG_DISABLE_WASM_OPT\n */\n '--disable-wasm-opt'?: ''\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * The path to your function directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path '?: string\n\n /**\n * Reset all your settings.\n * @environment SHOPIFY_FLAG_RESET\n */\n '--reset'?: ''\n\n /**\n * Increase the verbosity of the output.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n}" + "value": "export interface appfunctionbuild {\n /**\n * The Client ID of your app.\n * @environment SHOPIFY_FLAG_CLIENT_ID\n */\n '--client-id '?: string\n\n /**\n * The name of the app configuration.\n * @environment SHOPIFY_FLAG_APP_CONFIG\n */\n '-c, --config '?: string\n\n /**\n * Disable color output.\n * @environment SHOPIFY_FLAG_NO_COLOR\n */\n '--no-color'?: ''\n\n /**\n * The path to your function directory.\n * @environment SHOPIFY_FLAG_PATH\n */\n '--path '?: string\n\n /**\n * Reset all your settings.\n * @environment SHOPIFY_FLAG_RESET\n */\n '--reset'?: ''\n\n /**\n * Increase the verbosity of the output.\n * @environment SHOPIFY_FLAG_VERBOSE\n */\n '--verbose'?: ''\n}" } } } diff --git a/packages/app/src/cli/commands/app/function/build.ts b/packages/app/src/cli/commands/app/function/build.ts index 9857e6985ef..0ac57587ab4 100644 --- a/packages/app/src/cli/commands/app/function/build.ts +++ b/packages/app/src/cli/commands/app/function/build.ts @@ -4,7 +4,6 @@ import {appFlags} from '../../../flags.js' import AppCommand, {AppCommandOutput} from '../../../utilities/app-command.js' import {globalFlags} from '@shopify/cli-kit/node/cli' import {renderSuccess} from '@shopify/cli-kit/node/ui' -import {Flags} from '@oclif/core' export default class FunctionBuild extends AppCommand { static summary = 'Compile a function to wasm.' @@ -17,10 +16,6 @@ export default class FunctionBuild extends AppCommand { ...globalFlags, ...appFlags, ...functionFlags, - 'disable-wasm-opt': Flags.boolean({ - description: 'Disables the automatic optimization by wasm-opt after a function build', - env: 'SHOPIFY_FLAG_DISABLE_WASM_OPT', - }), } public async run(): Promise { @@ -38,7 +33,6 @@ export default class FunctionBuild extends AppCommand { stderr: process.stderr, useTasks: true, environment: 'production', - disableWasmOpt: flags['disable-wasm-opt'], }) renderSuccess({headline: 'Function built successfully.'}) return app diff --git a/packages/app/src/cli/services/build/extension.test.ts b/packages/app/src/cli/services/build/extension.test.ts index 2328e1da496..83df6888c73 100644 --- a/packages/app/src/cli/services/build/extension.test.ts +++ b/packages/app/src/cli/services/build/extension.test.ts @@ -173,7 +173,6 @@ describe('buildFunctionExtension', () => { signal, app, environment: 'production', - disableWasmOpt: true, }), ).resolves.toBeUndefined() diff --git a/packages/app/src/cli/services/build/extension.ts b/packages/app/src/cli/services/build/extension.ts index c21e79e8c34..2b0258b92e9 100644 --- a/packages/app/src/cli/services/build/extension.ts +++ b/packages/app/src/cli/services/build/extension.ts @@ -139,9 +139,7 @@ export async function buildUIExtension(extension: ExtensionInstance, options: Ex options.stdout.write(`${extension.localIdentifier} successfully built`) } -interface BuildFunctionExtensionOptions extends ExtensionBuildOptions { - disableWasmOpt?: boolean -} +type BuildFunctionExtensionOptions = ExtensionBuildOptions /** * Builds a function extension diff --git a/packages/cli/README.md b/packages/cli/README.md index cb8ee8c069f..587e511fb83 100644 --- a/packages/cli/README.md +++ b/packages/cli/README.md @@ -328,13 +328,11 @@ Compile a function to wasm. ``` USAGE - $ shopify app function build [--client-id | -c ] [--disable-wasm-opt] [--no-color] [--path ] - [--reset | ] [--verbose] + $ shopify app function build [--client-id | -c ] [--no-color] [--path ] [--reset | ] [--verbose] FLAGS -c, --config= The name of the app configuration. --client-id= The Client ID of your app. - --disable-wasm-opt Disables the automatic optimization by wasm-opt after a function build --no-color Disable color output. --path= The path to your function directory. --reset Reset all your settings. diff --git a/packages/cli/oclif.manifest.json b/packages/cli/oclif.manifest.json index 7decb536597..78bfbc0e701 100644 --- a/packages/cli/oclif.manifest.json +++ b/packages/cli/oclif.manifest.json @@ -782,13 +782,6 @@ "name": "config", "type": "option" }, - "disable-wasm-opt": { - "allowNo": false, - "description": "Disables the automatic optimization by wasm-opt after a function build", - "env": "SHOPIFY_FLAG_DISABLE_WASM_OPT", - "name": "disable-wasm-opt", - "type": "boolean" - }, "no-color": { "allowNo": false, "description": "Disable color output.", From 219b31cfc931fa772591819c49db181c4b7beae7 Mon Sep 17 00:00:00 2001 From: Alex Bradley Date: Thu, 12 Dec 2024 09:19:06 -0500 Subject: [PATCH 9/9] add debug log for runWasmOpt --- packages/app/src/cli/services/function/build.ts | 2 ++ 1 file changed, 2 insertions(+) diff --git a/packages/app/src/cli/services/function/build.ts b/packages/app/src/cli/services/function/build.ts index e30c62cec79..8d86c1ba643 100644 --- a/packages/app/src/cli/services/function/build.ts +++ b/packages/app/src/cli/services/function/build.ts @@ -206,6 +206,8 @@ export async function runWasmOpt(modulePath: string) { modulePath, ] + outputDebug(`Wasm binary: ${wasmOptBinary().name}`) + outputDebug('Optimizing this wasm binary using wasm-opt.') await exec(command, args, {cwd: wasmOptDir}) }