Skip to content

Commit 18757a9

Browse files
committed
fix(@angular/build): re-enable code splitting for unit tests
Re-enables esbuild code splitting for unit test builds by removing `disableCodeSplitting` and resolving the underlying live export binding issue when chunks are loaded by Vitest under Zone.js (#33728): 1. Ensures test spec files are treated as ES modules by appending `export {};` in the Angular compiler plugin if absent. In Zone.js applications, downleveled `async` functions capture module-level `this` via `__async(this, ...)`. Without an explicit export, esbuild misclassified spec files as CommonJS and wrapped them in `__commonJS`, which led to lazy `__esm` initializers for shared dependencies. 2. In the Vitest in-memory provider plugin, eagerly invokes any lazy `__esm` initializers detected in chunks when loaded for `vite-node`. Because `vite-node` statically captures exports upon initial chunk evaluation, eagerly executing the initializers ensures exported values are populated before consumers read them. 3. Enforces `namedChunks: false` for unit test builds to ensure chunks consistently match chunk naming patterns. closes #33948
1 parent e3d55b2 commit 18757a9

5 files changed

Lines changed: 64 additions & 49 deletions

File tree

packages/angular/build/src/builders/application/options.ts

Lines changed: 0 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -126,20 +126,6 @@ interface InternalOptions {
126126
* Suppress build summary and stats table.
127127
*/
128128
quiet?: boolean;
129-
130-
/**
131-
* Disables esbuild code splitting for the browser code bundle.
132-
*
133-
* Splitting emits shared chunks whose exports are read across chunk boundaries as live ESM
134-
* bindings. A module hoisted into a shared chunk is wrapped in a lazy initializer, so its exported
135-
* value is only assigned once that initializer runs. Runners that load the generated output
136-
* through a module runner rather than the browser's own ESM implementation do not reliably
137-
* preserve those bindings, and an importing chunk can observe the export as `undefined`.
138-
*
139-
* Test bundles are never downloaded by a browser, so there is nothing for splitting to optimize
140-
* there. Used exclusively for tests and shouldn't be used for other kinds of builds.
141-
*/
142-
disableCodeSplitting?: boolean;
143129
}
144130

145131
/** Full set of options for `application` builder. */
@@ -453,7 +439,6 @@ export async function normalizeOptions(
453439
partialSSRBuild = false,
454440
externalRuntimeStyles,
455441
instrumentForCoverage,
456-
disableCodeSplitting,
457442
} = options;
458443

459444
// Return all the normalized options
@@ -490,7 +475,6 @@ export async function normalizeOptions(
490475
watch,
491476
workspaceRoot,
492477
entryPoints,
493-
disableCodeSplitting,
494478
optimizationOptions,
495479
outputOptions,
496480
outExtension,

packages/angular/build/src/builders/unit-test/runners/vitest/build-options.ts

Lines changed: 2 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -256,13 +256,9 @@ export async function getVitestBuildOptions(
256256
sourceMap: { scripts: true, vendor: false, styles: false },
257257
outputHashing: adjustOutputHashing(baseBuildOptions.outputHashing),
258258
optimization: false,
259+
namedChunks: false,
259260
entryPoints,
260-
// Every spec file is its own entry point, so splitting hoists any module shared between two
261-
// specs into a chunk whose exports are then read across a chunk boundary. Those reads rely on
262-
// live ESM bindings, and a module placed in a shared chunk is only assigned its exported value
263-
// when that chunk's lazy initializer runs, so an importing chunk can read `undefined`. Nothing
264-
// downloads these bundles, so there is no benefit to weigh against that.
265-
disableCodeSplitting: true,
261+
266262
// Enable support for vitest browser prebundling. Excludes can be controlled with a runnerConfig
267263
// and the `optimizeDeps.exclude` option.
268264
externalPackages: true,

packages/angular/build/src/builders/unit-test/runners/vitest/plugins.ts

Lines changed: 17 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -404,7 +404,7 @@ export function createVitestPlugins(pluginOptions: PluginOptions): VitestPlugins
404404

405405
const outputFile = buildResultFiles.get(outputPath);
406406
if (outputFile) {
407-
const code = await loadResultFile(outputFile);
407+
let code = await loadResultFile(outputFile);
408408
const sourceMapPath = outputPath + '.map';
409409
const sourceMapFile = buildResultFiles.get(sourceMapPath);
410410
const sourceMapText = sourceMapFile ? await loadResultFile(sourceMapFile) : undefined;
@@ -414,6 +414,21 @@ export function createVitestPlugins(pluginOptions: PluginOptions): VitestPlugins
414414
adjustSourcemapSources(map, true, workspaceRoot, id);
415415
}
416416

417+
// Eagerly invoke any __esm initializers in chunks / bundles.
418+
// In Node-based Vitest runs (jsdom/happy-dom), vite-node evaluates code via ssrTransform,
419+
// which copies exports statically and does not maintain live ESM bindings across chunk
420+
// boundaries for variables initialized inside lazy __esm wrappers. Eagerly executing
421+
// these initializers when the chunk is loaded ensures that all exported values are
422+
// populated before vite-node reads them.
423+
const inits = [
424+
...new Set(
425+
[...code.matchAll(/\b(init_[a-zA-Z0-9_$]+)\s*=\s*__esm\b/g)].map((m) => m[1]),
426+
),
427+
];
428+
if (inits.length > 0) {
429+
code += `\n${inits.map((fn) => `typeof ${fn} === 'function' && ${fn}();`).join('\n')}\n`;
430+
}
431+
417432
return {
418433
code,
419434
map,
@@ -557,6 +572,7 @@ async function generateCoverageOption(
557572
? {
558573
exclude: Array.from(
559574
new Set([
575+
'virtual:*',
560576
// Augment the default exclude https://vitest.dev/config/#coverage-exclude
561577
// with the user defined exclusions
562578
...(configCoverage?.exclude || []),

packages/angular/build/src/tools/esbuild/angular/compiler-plugin.ts

Lines changed: 45 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -440,6 +440,10 @@ export function createCompilerPlugin(
440440
if (contents === undefined && compilation.transformFile) {
441441
try {
442442
directContents = await readFile(request, 'utf-8');
443+
if (pluginOptions.includeTestMetadata) {
444+
directContents = ensureTestFileEsm(request, directContents);
445+
}
446+
443447
const transformResult = await compilation.transformFile(request, directContents);
444448
if (transformResult) {
445449
contents = transformResult.contents;
@@ -483,6 +487,10 @@ export function createCompilerPlugin(
483487
// Evaluate whether the file requires the Angular compiler transpilation.
484488
// If not, issue a warning but allow bundler to process the file (no type-checking).
485489
directContents ??= await readFile(request, 'utf-8');
490+
if (pluginOptions.includeTestMetadata) {
491+
directContents = ensureTestFileEsm(request, directContents);
492+
}
493+
486494
if (!requiresAngularCompiler(directContents)) {
487495
return {
488496
warnings: [createMissingFileDiagnostic(request, args.path, diangosticRoot, false)],
@@ -496,21 +504,27 @@ export function createCompilerPlugin(
496504
return {
497505
errors: [createMissingFileDiagnostic(request, args.path, diangosticRoot, true)],
498506
};
499-
} else if (typeof contents === 'string' && (useTypeScriptTranspilation || isJS)) {
500-
// A string indicates untransformed output from the TS/NG compiler.
501-
// This step is unneeded when using esbuild transpilation.
502-
const sideEffects = await hasSideEffects(request);
503-
const instrumentForCoverage = pluginOptions.instrumentForCoverage?.(request);
504-
contents = await javascriptTransformer.transformData(
505-
request,
506-
contents,
507-
true /* skipLinker */,
508-
sideEffects,
509-
instrumentForCoverage,
510-
);
507+
} else if (typeof contents === 'string') {
508+
if (pluginOptions.includeTestMetadata) {
509+
contents = ensureTestFileEsm(request, contents);
510+
}
511+
512+
if (useTypeScriptTranspilation || isJS) {
513+
// A string indicates untransformed output from the TS/NG compiler.
514+
// This step is unneeded when using esbuild transpilation.
515+
const sideEffects = await hasSideEffects(request);
516+
const instrumentForCoverage = pluginOptions.instrumentForCoverage?.(request);
517+
contents = await javascriptTransformer.transformData(
518+
request,
519+
contents,
520+
true /* skipLinker */,
521+
sideEffects,
522+
instrumentForCoverage,
523+
);
511524

512-
// Store as the returned Uint8Array to allow caching the fully transformed code
513-
typeScriptFileCache.set(request, contents);
525+
// Store as the returned Uint8Array to allow caching the fully transformed code
526+
typeScriptFileCache.set(request, contents);
527+
}
514528
}
515529

516530
let loader: Loader;
@@ -576,9 +590,7 @@ export function createCompilerPlugin(
576590
const replacement = pluginOptions.fileReplacements?.[path.normalize(args.path)];
577591
if (replacement) {
578592
return {
579-
contents: await import('node:fs/promises').then(({ readFile }) =>
580-
readFile(path.normalize(replacement)),
581-
),
593+
contents: await readFile(path.normalize(replacement)),
582594
loader: 'json' as const,
583595
watchFiles: [replacement],
584596
};
@@ -860,3 +872,19 @@ const POTENTIAL_METADATA_REGEX = /@angular\/core|@Component|@Directive|@Injectab
860872
function requiresAngularCompiler(contents: string): boolean {
861873
return POTENTIAL_METADATA_REGEX.test(contents);
862874
}
875+
876+
const SPECS_REGEXP = /\.(?:spec|test)\.[cm]?[jt]sx?$/;
877+
878+
/**
879+
* Ensures test spec files lacking export statements are treated as ES modules by esbuild.
880+
* In Zone.js apps, async test callbacks are downleveled to `__async(this, ...)`, which captures
881+
* module-level `this`. If a file has a top-level `this` and no `export` statements, esbuild's
882+
* module detection treats it as CommonJS and wraps it in `__commonJS`. This causes esbuild to wrap
883+
* imported modules in lazy `__esm` initializers, which breaks live export bindings in Vite SSR
884+
* (https://github.com/angular/angular-cli/issues/33728). Adding an empty export guarantees the file is bundled as an ES module.
885+
*/
886+
function ensureTestFileEsm(request: string, contents: string): string {
887+
return SPECS_REGEXP.test(request) && !contents.includes('export {};')
888+
? contents + '\nexport {};\n'
889+
: contents;
890+
}

packages/angular/build/src/tools/esbuild/application-code-bundle.ts

Lines changed: 0 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -70,15 +70,6 @@ export function createBrowserCodeBundleOptions(
7070
supported: getFeatureSupport(zoneless),
7171
};
7272

73-
if (options.disableCodeSplitting) {
74-
// Splitting emits shared chunks that are read across chunk boundaries as live ESM bindings,
75-
// which the unit-test runners' module loading does not reliably preserve.
76-
buildOptions.splitting = false;
77-
// In unit test builds, package.json "sideEffects": false annotations can cause esbuild
78-
// to incorrectly elide statically-referenced barrel module bodies across multiple entry points.
79-
buildOptions.ignoreAnnotations = true;
80-
}
81-
8273
buildOptions.plugins ??= [];
8374
buildOptions.plugins.push(
8475
createWasmPlugin({ allowAsync: zoneless, cache: loadCache }),

0 commit comments

Comments
 (0)