feat: reenable asar support #272552
Merged
feat: reenable asar support #272552
+124
−53
Conversation
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
deepak1556
force-pushed
the
robo/enable_asar
branch
from
d872cba to
c0f6324
Compare
There was a problem hiding this comment.
Pull Request Overview
This PR re-enables ASAR (Electron Archive) support for VS Code's ESM (ECMAScript Modules) implementation. The main goal is to fix module resolution issues by adjusting the ASAR layout and improving module loading logic.
Key changes:
- Restructured ASAR layout to include a nested
node_modulesfolder (e.g.,node_modules.asar/node_modules/packageAinstead ofnode_modules.asar/packageA) - Removed the
canASARflag and simplified ASAR usage conditions throughout the codebase - Implemented ESM module resolution hooks in
bootstrap-esm.tsto properly handle ASAR paths
Reviewed Changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/vs/amdX.ts | Removed canASAR constant and updated ASAR usage logic to check for Electron environment directly |
| src/vs/base/common/platform.ts | Added hasElectronUserAgent flag to detect Electron environment |
| src/vs/base/common/network.ts | Updated ASAR paths to include nested node_modules folder |
| src/bootstrap-node.ts | Added enableASARSupport() function to hook into Node's module resolution |
| src/bootstrap-esm.ts | Implemented ESM loader hooks for ASAR path resolution |
| src/vs/workbench/services/treeSitter/browser/treeSitterLibraryService.ts | Removed canASAR import and simplified module location logic |
| src/vs/workbench/services/textMate/browser/textMateTokenizationFeatureImpl.ts | Removed canASAR import and simplified WASM loading logic |
| src/vs/workbench/services/textMate/browser/backgroundTokenization/threadedBackgroundTokenizerFactory.ts | Removed canASAR import and simplified ASAR usage check |
| src/vs/workbench/services/languageDetection/browser/languageDetectionWorkerServiceImpl.ts | Removed canASAR import and simplified ASAR usage check |
| build/linux/dependencies-generator.ts | Removed canAsar variable and hardcoded path to unpacked modules |
| build/linux/dependencies-generator.js | Removed canAsar variable and hardcoded path to unpacked modules |
| build/gulpfile.vscode.js | Updated ASAR creation to use process.cwd() and modified vsda exclusion comment |
deepak1556
force-pushed
the
robo/enable_asar
branch
2 times, most recently
from
998621d to
801be2e
Compare
📬 CODENOTIFYThe following users are being notified based on files changed in this PR: @bpaseroMatched files:
@TylerLeonhardtMatched files:
|
deepak1556
force-pushed
the
robo/enable_asar
branch
from
251cd5e to
cfabf85
Compare
bpasero
approved these changes
Oct 23, 2025
joaomoreno
added a commit
that referenced
this pull request
Oct 23, 2025
This reverts commit ff89137.
joaomoreno
added a commit
that referenced
this pull request
Oct 23, 2025
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
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
Testing one approach that adjusts the
parentURLin the module resolution logic to resolve the package url https://github.com/nodejs/node/blob/340e61990b9bf871d8a5e9b9337eaa337d3e9ddc/lib/internal/modules/package_json_reader.js#L294node_modulesto fit the above lookup logicUpdated condition in
importAMDNodeModulesince AMD loading in web workers trip on the condition platform.isWebutilityprocess entrypoints don't preserve drive letter casing due to
FileAccess.asFileUri('bootstrap-fork.js').fsPath, the path checks in resolution are normalized to accommodate this behavior.