Skip to content

Commit 63951b3

Browse files
NikolaRHristovNikolaRHristov
committed
fix(Sky): Restore correct workbench initialization order
The bundled Electron entry was pre-importing desktop.main.js to force Rollup to bundle all ~1500 modules into one chunk, then using manualChunks to keep them together. This broke initialization order: desktop.main.js evaluated immediately when its chunk loaded, but Electron-side service registrations (Disk FileSystemProvider, NativeHostService, etc.) only fire when isElectron returns true— which depends on the process polyfill being live and workbench config attached, both done by workbench.js's load() chain. The fix removes the static pre-import and manualChunks, letting workbench.js's dynamic import of desktop.main.js happen in the correct sequence: workbench.js sets up _VSCode_FILE_ROOT, NLS, and resolved configuration first, then desktop.main.js's module-initializers detect Electron mode correctly and register the native services. Additionally, configure moduleSideEffects to mark all VS Code modules as side-effectful, preserving their static initializers (class event emitters, global registries, DI singleton registrations) that Rollup's tree-shaker might otherwise prune.
1 parent c56f263 commit 63951b3

2 files changed

Lines changed: 70 additions & 55 deletions

File tree

‎Source/Workbench/Bundled/Electron/Entry.ts‎

Lines changed: 17 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -53,30 +53,24 @@
5353

5454
performance.mark("land:bundled:electron:start");
5555

56-
// Pre-import the workbench's desktop entry shim with a LITERAL string so
57-
// Vite/Rollup follows desktop.main.js's static import graph (~1500
58-
// modules: contrib/*, services/*, platform/*, base/*, editor/*) and
59-
// pulls them into the bundled chunk. Without this, only workbench.js
60-
// + its small synchronous graph land in the bundle (~26 MB), and the
61-
// workbench loader's runtime `await import(computedURL)` resolves
62-
// against `/Static/Application/` - re-fetching ~1500 separate files
63-
// from disk every cold boot.
56+
// Just load `workbench.js` - its `load()` function (patched by Output's
57+
// `RewriteWorkbenchBaseURL` transform) does the dynamic import of
58+
// `workbench.desktop.main.js` itself, AFTER setting up
59+
// `_VSCODE_FILE_ROOT`, NLS, and the resolved configuration. The
60+
// transform rewrites the runtime-computed-URL import to a literal-
61+
// string `await import("../../../workbench/workbench.desktop.main.js")`
62+
// so Vite/Rollup can statically follow it and bundle the entire
63+
// desktop graph into a separate chunk that loads on demand.
6464
//
65-
// The browser's module cache deduplicates: when workbench.js later
66-
// runs `await import("vs/workbench/workbench.desktop.main.js")` the
67-
// resolved URL hits the same cache entry as the static import here,
68-
// so order-of-execution is preserved (desktop.main.js side-effects
69-
// happen first, registering DI services; workbench.js then runs
70-
// `result.main(configuration)` against the registered surface).
71-
//
72-
// `workbench.web.main.internal.js` is statically imported by
73-
// `workbench.web.main.js`, which `workbench.desktop.main.js` shares
74-
// many modules with - Rollup chunk-deduplicates so we're not paying
75-
// the import twice.
76-
await import(
77-
"@codeeditorland/output/Target/Microsoft/VSCode/vs/workbench/workbench.desktop.main.js"
78-
);
79-
65+
// We previously pre-imported desktop.main.js here, which broke
66+
// initialisation order: desktop.main.js evaluates immediately when
67+
// its chunk loads, but the Electron-side service registrations (Disk
68+
// FileSystemProvider, NativeHostService, etc.) only fire when
69+
// `isElectron` returns true, which depends on the process polyfill
70+
// being live and the workbench config being attached - both done by
71+
// workbench.js's `load()` chain. Pre-importing meant desktop.main.js
72+
// evaluated before that setup, mode-detected as "web", skipped every
73+
// Electron registration, and surfaced as ENOPRO file-system errors.
8074
await import(
8175
"@codeeditorland/output/Target/Microsoft/VSCode/vs/code/electron-browser/workbench/workbench.js"
8276
);

‎astro.config.ts‎

Lines changed: 53 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -902,11 +902,39 @@ export default defineConfig({
902902
treeshake: {
903903
// Preserve all side effects in the worker package so Register.js
904904
// SW registration code is not eliminated by Rollup.
905-
moduleSideEffects: (Id: string) =>
906-
Id.includes("@codeeditorland/worker") ||
907-
Id.includes("Element/Worker")
908-
? true
909-
: "no-external",
905+
//
906+
// VS Code's modules (under `@codeeditorland/output/Target/
907+
// Microsoft/VSCode/`) are FULL of side-effect static
908+
// initialisers - class-static event emitters
909+
// (`_onWillInstantiateEditorPane = new Emitter(...)`),
910+
// global registries (`Registry.add(...)`), DI singleton
911+
// registrations (`registerSingleton(...)`). Rollup's
912+
// default `"no-external"` is too aggressive: it can
913+
// prune imports that look unused statically but whose
914+
// side-effect init populates a registry the workbench
915+
// depends on at runtime. Mark every VS Code module as
916+
// having side effects so Rollup preserves the init
917+
// order verbatim.
918+
moduleSideEffects: (Id: string) => {
919+
if (
920+
Id.includes("@codeeditorland/worker") ||
921+
Id.includes("Element/Worker")
922+
) {
923+
return true;
924+
}
925+
if (
926+
BundledActive &&
927+
(Id.includes(
928+
"/Output/Target/Microsoft/VSCode/",
929+
) ||
930+
Id.includes(
931+
"\\Output\\Target\\Microsoft\\VSCode\\",
932+
))
933+
) {
934+
return true;
935+
}
936+
return "no-external";
937+
},
910938
},
911939
external: [
912940
...External,
@@ -942,33 +970,26 @@ export default defineConfig({
942970
output: {
943971
// Preserve dynamic URL imports in VSCode worker files
944972
hoistTransitiveImports: false,
945-
// Fold every VS Code module reachable from the bundled
946-
// workbench entry into a single chunk. Without this,
947-
// Rollup auto-splits at static-import boundaries and
948-
// emits ~60 separate chunks (the largest is the
949-
// node-named one Rollup elects as "main"). With this,
950-
// one big `workbench-[hash].js` lands and the runtime
951-
// fetches a single JS asset instead of waterfalling
952-
// through dozens. Astro page scripts (NLS,
953-
// TelemetryBridge, Bootstrap, SkyBridge) are left to
954-
// default chunking - they have different lifecycles
955-
// from the workbench and shouldn't share a chunk.
956-
manualChunks: BundledActive
957-
? (id: string) => {
958-
if (
959-
id.includes(
960-
"/Output/Target/Microsoft/VSCode/",
961-
) ||
962-
id.includes(
963-
"\\Output\\Target\\Microsoft\\VSCode\\",
964-
) ||
965-
id.includes("/Workbench/Bundled/")
966-
) {
967-
return "workbench";
968-
}
969-
return null;
970-
}
971-
: undefined,
973+
// No `manualChunks` for the bundled tree. The workbench
974+
// loader (`workbench.js`) dynamically imports
975+
// `workbench.desktop.main.js` via a literal-string
976+
// `await import(...)` (rewritten there by Output's
977+
// `RewriteWorkbenchBaseURL` transform). Rollup
978+
// auto-splits at that boundary, putting workbench.js's
979+
// small sync graph in one chunk and desktop.main.js +
980+
// its 1500-module transitive graph in another. The
981+
// split is REQUIRED for correct initialisation order:
982+
// workbench.js's `load()` function must set up
983+
// `_VSCODE_FILE_ROOT`, NLS, and the resolved
984+
// configuration BEFORE desktop.main.js's contribs
985+
// evaluate (which `isElectron`-check at module-init
986+
// time and otherwise mode-detect as "web", skipping
987+
// the Electron-side service registrations and breaking
988+
// the FS provider chain).
989+
//
990+
// Forcing all VS Code into one chunk via manualChunks
991+
// would defeat the auto-split and re-introduce the
992+
// initialisation-order bug.
972993
// Route bundled entries to `${BundledOutputDir}/<Variant>/
973994
// workbench-[hash].js`; everything else keeps the existing
974995
// `app.js` / `[name]-[hash].js` shape.

0 commit comments

Comments
 (0)