feat: migrate firepit standalone to node 26 sea - #10892
Conversation
### Description Migrate the standalone executable builder (Firepit) from the legacy `@yao-pkg/pkg` packager to Node.js 26 native Single Executable Applications (`node --build-sea`). Key changes: - Added `standalone/build-sea.js` using `esbuild` and native Node 26 `--build-sea` to construct standalone binaries without `postject`. - Updated `standalone/firepit.js` to use `node:sea` `getRawAsset` for tarball decompression without binary string encoding corruption. - Configured child process forks to pass `execPath: safeNodePath` to prevent recursive entrypoint re-execution inside SEA binaries. - Updated `scripts/firepit-builder/pipeline.js` to use `build:sea`. ### Scenarios Tested - Cold boot binary execution and cache asset extraction. - Sub-second warm boot execution performance benchmarking. - Command help output (`--help`) and version output (`--version`). - Embedded Node runtime execution via `firebase is:node`. - Embedded NPM execution via `firebase is:npm`. - Native `arm64` Apple Silicon execution and `x64` compilation. - Code linting verified via `npm run lint:changed-files`. ### Sample Commands - `npm run build:sea` (inside `standalone/`) - `node ./pipeline.js --package="/path/to/firebase-tools"` - `./firebase-tools-darwin-arm64 --version` - `./firebase-tools-darwin-arm64 is:node -e "console.log(process.version)"` - `./firebase-tools-darwin-arm64 is:npm --version`
Wiz Scan Summary
To detect these findings earlier in the dev lifecycle, try the Wiz Code extension for VS Code, JetBrains, or Visual Studio. |
There was a problem hiding this comment.
Code Review
This pull request migrates the standalone build system from using pkg to Node 26 Single Executable Application (SEA) workflow, bundling the application with esbuild and embedding a packaged vendor.tar.gz asset. It also replaces the shelljs dependency with a custom lightweight shell utility in standalone/firepit.js. Feedback on these changes includes removing a hardcoded absolute path in build-sea.js that will break builds in other environments, capturing stderr in the custom shell.exec implementation to prevent undefined errors, and ensuring that the custom shell.cp and shell.ln helpers robustly handle both two and three arguments.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request transitions the standalone executable builder from using @yao-pkg/pkg to Node.js 26's native Single Executable Application (--build-sea) capabilities, bundling code with esbuild and implementing a custom lightweight shell utility to replace shelljs. Feedback on these changes includes addressing a Windows path wildcard matching issue in firepit.js, fixing architecture detection for macOS universal binaries in build-sea.js, using the esbuild JavaScript API directly instead of spawning a CLI process, and performing case-insensitive checks when verifying the Node path.
| const actualSrc = dest ? src : flag; | ||
| const actualDest = dest ? dest : src; | ||
| try { | ||
| if (typeof actualSrc === "string" && actualSrc.endsWith("/*")) { |
There was a problem hiding this comment.
On Windows, path.join normalizes path separators to backslashes (\\). Therefore, path.join(__dirname, "vendor/*") will produce a path ending with \\* instead of /*. This causes actualSrc.endsWith("/*") to evaluate to false, falling back to fs.cpSync with a wildcard path, which throws an error and silently fails the copy operation. We should check for both forward and backward slash wildcards.
| if (typeof actualSrc === "string" && actualSrc.endsWith("/*")) { | |
| if (typeof actualSrc === "string" && (actualSrc.endsWith("/*") || actualSrc.endsWith("\\*"))) { |
| let arch = process.arch; | ||
| try { | ||
| const fileOut = execSync(`file "${hostNodeBin}"`, { encoding: "utf8" }); | ||
| if (fileOut.includes("x86_64")) arch = "x64"; | ||
| else if (fileOut.includes("arm64")) arch = "arm64"; | ||
| } catch (e) {} |
There was a problem hiding this comment.
On macOS, the official Node.js binaries are typically universal binaries containing both x86_64 and arm64 slices. Because fileOut.includes("x86_64") is checked first, arch will always be overwritten to "x64" on Apple Silicon Macs running a universal Node binary. We should check if the binary is universal first, and if so, fall back to process.arch.
| let arch = process.arch; | |
| try { | |
| const fileOut = execSync(`file "${hostNodeBin}"`, { encoding: "utf8" }); | |
| if (fileOut.includes("x86_64")) arch = "x64"; | |
| else if (fileOut.includes("arm64")) arch = "arm64"; | |
| } catch (e) {} | |
| let arch = process.arch; | |
| try { | |
| const fileOut = execSync('file "' + hostNodeBin + '"', { encoding: 'utf8' }); | |
| if (!fileOut.includes("universal")) { | |
| if (fileOut.includes("x86_64")) arch = "x64"; | |
| else if (fileOut.includes("arm64")) arch = "arm64"; | |
| } | |
| } catch (e) {} |
| execSync( | ||
| `"${esbuildBin}" "${path.join(standaloneDir, "firepit.js")}" --bundle --platform=node --target=node26 --outfile="${bundlePath}"`, | ||
| { stdio: "inherit", cwd: standaloneDir } | ||
| ); |
There was a problem hiding this comment.
Instead of spawning a child process to run the esbuild CLI, which can be fragile on Windows due to shell execution and file extension differences, you can use the esbuild JavaScript API directly. This is faster, more reliable, and fully cross-platform.
require("esbuild").buildSync({
entryPoints: [path.join(standaloneDir, "firepit.js")],
bundle: true,
platform: "node",
target: "node26",
outfile: bundlePath,
});| const basename = path.basename(nodePath); | ||
| if (nodePath === process.execPath || basename.includes("firepit") || basename.includes("firebase-tools")) { | ||
| return false; | ||
| } |
There was a problem hiding this comment.
To ensure case-insensitive matching on platforms like Windows and macOS where users or build environments might use different casing (e.g., Firepit or Firebase-Tools), we should lowercase the basename before performing .includes() checks.
| const basename = path.basename(nodePath); | |
| if (nodePath === process.execPath || basename.includes("firepit") || basename.includes("firebase-tools")) { | |
| return false; | |
| } | |
| const basename = path.basename(nodePath).toLowerCase(); | |
| if (nodePath === process.execPath || basename.includes("firepit") || basename.includes("firebase-tools")) { | |
| return false; | |
| } |
Description
Migrate the standalone executable builder (Firepit) from the legacy
@yao-pkg/pkgpackager to Node.js 26 native Single Executable Applications (node --build-sea).Key changes:
standalone/build-sea.jsusingesbuildand native Node 26--build-seato construct standalone binaries withoutpostject.standalone/firepit.jsto usenode:seagetRawAssetfor tarball decompression without binary string encoding corruption.execPath: safeNodePathto prevent recursive entrypoint re-execution inside SEA binaries.scripts/firepit-builder/pipeline.jsto usebuild:sea.Scenarios Tested
--help) and version output (--version).firebase is:node.firebase is:npm.arm64Apple Silicon execution andx64compilation.npm run lint:changed-files.Sample Commands
npm run build:sea(insidestandalone/)node ./pipeline.js --package="/path/to/firebase-tools"./firebase-tools-darwin-arm64 --version./firebase-tools-darwin-arm64 is:node -e "console.log(process.version)"./firebase-tools-darwin-arm64 is:npm --version