feat: restore asar for node_modules#324084
Draft
deepak1556 wants to merge 14 commits into
Draft
Conversation
deepak1556
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Restores packaged-app ASAR support in the ESM world by re-enabling module and asset resolution from node_modules.asar (JS) and node_modules.asar.unpacked (native/WASM), and updates build/packaging to unpack the right payloads and trim unnecessary binaries (notably @microsoft/mxc-sdk).
Changes:
- Reintroduce ASAR-aware module resolution for both CommonJS (
bootstrap-node) and ESM (bootstrap-esm), and updateamdXto resolve AMD-loaded node modules from ASAR in Electron contexts. - Standardize runtime paths to native binaries/WASM to prefer
node_modules.asar.unpackedin packaged builds (Terminal sandbox, Copilot CLI/agent host, Tree-sitter/TextMate assets). - Optimize packaging by excluding non-target
@microsoft/mxc-sdkarchitectures and updating universal app merge logic and module ignore rules.
Reviewed changes
Copilot reviewed 31 out of 31 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/vs/workbench/services/treeSitter/browser/treeSitterLibraryService.ts | Switch Tree-sitter WASM location selection to use unpacked node_modules for built installs. |
| src/vs/workbench/services/textMate/browser/textMateTokenizationFeatureImpl.ts | Update oniguruma WASM fetch path selection for built installs. |
| src/vs/workbench/services/textMate/browser/backgroundTokenization/threadedBackgroundTokenizerFactory.ts | Adjust worker-side oniguruma asset path selection for packaged builds. |
| src/vs/workbench/services/languageDetection/browser/languageDetectionWorkerServiceImpl.ts | Enable ASAR-based module/model resource loading in built Electron contexts. |
| src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalSandboxService.ts | Provide nativeModulesDir so terminal sandbox can find unpacked native binaries in packaged builds. |
| src/vs/sessions/electron-browser/sessions.ts | Populate _VSCODE_PRODUCT_JSON global for ASAR path decisions (Agents window). |
| src/vs/platform/webWorker/browser/webWorkerServiceImpl.ts | Propagate _VSCODE_PRODUCT_JSON into ESM worker bootstrap. |
| src/vs/platform/sandbox/common/terminalSandboxMxcRuntime.ts | Thread nativeModulesDir into MXC runtime executable resolution. |
| src/vs/platform/sandbox/common/terminalSandboxEngine.ts | Resolve ripgrep/MXC binaries from nativeModulesDir (supports unpacked layout). |
| src/vs/platform/agentHost/node/copilot/copilotAgent.ts | Resolve Copilot CLI and MXC binaries from unpacked node_modules in built installs. |
| src/vs/platform/agentHost/node/copilot/agentHostSandboxEngine.ts | Provide nativeModulesDir in agent host sandbox runtime info. |
| src/vs/platform/agentHost/node/commandAutoApprover.ts | Resolve Tree-sitter WASM assets from unpacked node_modules in built installs. |
| src/vs/code/electron-browser/workbench/workbench.ts | Populate _VSCODE_PRODUCT_JSON global for ASAR path decisions (main workbench). |
| src/vs/base/common/platform.ts | Add hasElectronUserAgent helper for detecting Electron-backed workers. |
| src/vs/amdX.ts | Restore ASAR selection for AMD node module loading and improve ASAR-aware fs usage. |
| src/bootstrap-node.ts | Reintroduce CommonJS lookup-path injection for node_modules.asar in packaged Electron-as-node processes. |
| src/bootstrap-esm.ts | Install an ESM resolution hook to resolve bare specifiers from node_modules.asar in packaged Electron contexts. |
| extensions/copilot/src/extension/chatSessions/copilotcli/node/test/nodePtyShim.spec.ts | Add tests validating node-pty resolution from node_modules.asar.unpacked. |
| extensions/copilot/src/extension/chatSessions/copilotcli/node/test/appNodeModules.spec.ts | New tests for shared app-root node_modules resolution helper. |
| extensions/copilot/src/extension/chatSessions/copilotcli/node/ripgrepShim.ts | Use shared resolver to locate ripgrep in unpacked node_modules for packaged builds. |
| extensions/copilot/src/extension/chatSessions/copilotcli/node/nodePtyShim.ts | Probe both node_modules and node_modules.asar.unpacked for node-pty binaries. |
| extensions/copilot/src/extension/chatSessions/copilotcli/node/copilotCli.ts | Resolve MXC binaries from unpacked node_modules when applicable. |
| extensions/copilot/src/extension/chatSessions/copilotcli/node/appNodeModules.ts | New helper for resolving paths across node_modules and node_modules.asar.unpacked. |
| extensions/copilot/src/extension/chatSessions/claude/node/claudeCodeAgent.ts | Resolve built-in ripgrep path via shared resolver for packaged builds. |
| build/linux/dependencies-generator.ts | Scan native modules from node_modules.asar.unpacked for dependency generation. |
| build/lib/test/copilot.test.ts | Add tests ensuring MXC binaries are filtered to the target architecture. |
| build/lib/copilot.ts | Add MXC architecture exclude filter helper for packaging. |
| build/gulpfile.vscode.ts | Apply MXC exclude filter and adjust ASAR unpack/duplication patterns and Copilot ripgrep shim input path. |
| build/gulpfile.reh.ts | Apply MXC exclude filter to REH packaging pipeline. |
| build/darwin/create-universal-app.ts | Cross-copy MXC binaries and extend ASAR merge allowlist for arch-unique files. |
| build/.moduleignore | Ignore signing _manifest artifacts from module payloads. |
This reverts commit 4747aa5.
- bootstrap-esm: resolve bare specifiers via package self-reference so
Node applies the package's real `exports`/`main` and ESM conditions.
The previous CommonJS `require.resolve()` picked the `require`
condition and loaded the CJS entry of dual CJS/ESM packages (e.g.
playwright-core), which did not expose their named ESM exports.
- amdX: in `_nodeJSLoadScript`, read module files with the ASAR-aware
`require('fs')` instead of `import('fs')`, which the resolution hook
maps to the ASAR-unaware `original-fs` and therefore cannot read
files inside the archive (e.g. @vscode/iconv-lite-umd).
- agentHost commandAutoApprover: load the tree-sitter `.wasm` files
from node_modules.asar.unpacked in built apps (node_modules in dev).
- agentHost copilotAgent: resolve the @github/copilot-<platform> CLI
and the @microsoft/mxc-sdk sandbox binaries from
node_modules.asar.unpacked in built apps, and unpack
@microsoft/mxc-sdk/bin so those executables can be spawned.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Two follow-up fixes for ASAR support:
- bootstrap-esm: resolve packages inside node_modules.asar with relative
specifiers (require.resolve("./<pkg>/package.json") and "./<spec>")
instead of bare specifiers. The archive directory is named
node_modules.asar, so a bare-specifier resolution walks for a
node_modules directory that does not exist and fails. This broke every
native module imported from main.js in the packaged app
(@vscode/spdlog, @vscode/sqlite3, native-keymap, @vscode/deviceid,
@vscode/policy-watcher), preventing startup.
- create-universal-app: pass `singleArchFiles` so the universal merger
allows files that are unique to a single arch inside the merged
node_modules.asar (the @github/copilot-<arch> platform package and the
arch-specific copilot/ripgrep binaries). These paths are ASAR-internal
(top level, no node_modules prefix); without the allowlist the merge
aborts with "Detected unique file ... not covered by allowList rule".
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
deepak1556
force-pushed
the
robo/enable_asar_support
branch
from
b3173e9 to
e617c31
Compare
Contributor
Screenshot ChangesBase: 2 insignificant change(s) omitted (≤20 px, Δ≤2). See CI logs for details. Added (88)
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #228064
Fixes #319428
For #269500