Skip to content

Commit d09f98e

Browse files
committed
fix(@angular/build): avoid top-level await for Zone.js injection in Vitest runner
Previously, the Vitest unit-test runner used dynamic import strategies (`'dynamic'` and `'dynamic-zone'`) within the generated TestBed initialization virtual file (`createTestBedInitVirtualFile`) to load `zone.js` and `zone.js/testing`. This logic was fundamentally flawed: 1. When Zone.js was loaded dynamically at runtime via the `'dynamic'` strategy, esbuild did not downlevel `async`/`await` because `isZonelessApp` checked the build target's `polyfills` configuration (which did not explicitly list `zone.js`). Consequently, native async/await microtasks bypassed Zone.js context tracking, breaking Zone.js at runtime. 2. If a project used a local polyfills file (e.g. `polyfills: ["src/polyfills.ts"]`), `isZonelessApp` considered the application zoneful and disabled `async-await` support in esbuild. However, esbuild cannot downlevel top-level await when async/await is downleveled, causing esbuild to reject top-level await unconditionally. Hence, the `'dynamic'` strategy never worked as intended. 3. For zoneless applications (such as `polyfills: []`), the syntactic presence of top-level `await` in the virtual file caused esbuild builds to fail when targeting older browsers or Browserslist targets that lack top-level await support, even though Zone was never present at runtime. This commit resolves these issues by: - Eliminating top-level `await import()` from `createTestBedInitVirtualFile` entirely. - Inverting the polyfill strategy so that `zone.js` and `zone.js/testing` are injected directly into `buildOptions.polyfills` before bundling based on the configured `polyfills` option (from the `test` target or inherited from the `build` target). - For library targets where `polyfills` is undefined, `zone.js` and `zone.js/testing` are injected if `zone.js` is installed, accompanied by a deprecation warning advising users to configure the `polyfills` option in their test configuration (`[]` for zoneless projects or `["zone.js"]` for Zone.js projects). Fixes #33324
1 parent feb41cc commit d09f98e

5 files changed

Lines changed: 138 additions & 53 deletions

File tree

‎packages/angular/build/src/builders/unit-test/builder.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -295,7 +295,7 @@ export async function* execute(
295295
buildOptions: runnerBuildOptions,
296296
virtualFiles,
297297
testEntryPointMappings,
298-
} = await runner.getBuildOptions(normalizedOptions, buildTargetOptions));
298+
} = await runner.getBuildOptions(normalizedOptions, buildTargetOptions, context.logger));
299299
} catch (e) {
300300
assertIsError(e);
301301
context.logger.error(
@@ -323,9 +323,7 @@ export async function* execute(
323323
const applicationBuildOptions = {
324324
...buildTargetOptions,
325325
...runnerBuildOptions,
326-
...(normalizedOptions.polyfills !== undefined
327-
? { polyfills: normalizedOptions.polyfills }
328-
: {}),
326+
polyfills: runnerBuildOptions.polyfills ?? normalizedOptions.polyfills,
329327
watch: normalizedOptions.watch,
330328
progress: normalizedOptions.buildProgress ?? buildTargetOptions.progress,
331329
quiet: normalizedOptions.quiet,

‎packages/angular/build/src/builders/unit-test/runners/api.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -63,6 +63,7 @@ export interface TestRunner {
6363
getBuildOptions(
6464
options: NormalizedUnitTestBuilderOptions,
6565
baseBuildOptions: Partial<ApplicationBuilderInternalOptions>,
66+
logger: BuilderContext['logger'],
6667
): RunnerOptions | Promise<RunnerOptions>;
6768

6869
/**

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

Lines changed: 29 additions & 44 deletions
Original file line numberDiff line numberDiff line change
@@ -11,12 +11,13 @@
1111
* Provides Vitest-specific build options and virtual file contents for Angular unit testing.
1212
*/
1313

14+
import type { BuilderContext } from '@angular-devkit/architect';
1415
import path from 'node:path';
1516
import { toPosixPath } from '../../../../utils/path';
1617
import { createProjectResolver } from '../../../../utils/resolve-project';
1718
import type { ApplicationBuilderInternalOptions } from '../../../application/options';
1819
import { OutputHashing } from '../../../application/schema';
19-
import { NormalizedUnitTestBuilderOptions } from '../../options';
20+
import { type NormalizedUnitTestBuilderOptions, injectTestingPolyfills } from '../../options';
2021
import { findTests, getTestEntrypoints } from '../../test-discovery';
2122
import { RunnerOptions } from '../api';
2223

@@ -26,14 +27,12 @@ import { RunnerOptions } from '../api';
2627
* @param providersFile Optional path to a file that exports default providers.
2728
* @param projectSourceRoot The root directory of the project source.
2829
* @param teardown Whether to configure TestBed to destroy after each test.
29-
* @param zoneTestingStrategy How zone.js should be loaded during initialization.
3030
* @returns The string content of the virtual initialization file.
3131
*/
3232
function createTestBedInitVirtualFile(
3333
providersFile: string | undefined,
3434
projectSourceRoot: string,
3535
teardown: boolean,
36-
zoneTestingStrategy: 'none' | 'static' | 'dynamic' | 'dynamic-zone',
3736
hasLocalize: boolean,
3837
): string {
3938
let providersImport = 'const providers = [];';
@@ -44,21 +43,6 @@ function createTestBedInitVirtualFile(
4443
providersImport = `import providers from './${importPath}';`;
4544
}
4645

47-
let zoneTestingSnippet = '';
48-
if (zoneTestingStrategy === 'static') {
49-
zoneTestingSnippet = `import 'zone.js/testing';`;
50-
} else if (zoneTestingStrategy === 'dynamic') {
51-
zoneTestingSnippet = `if (typeof Zone !== 'undefined') {
52-
// 'zone.js/testing' is used to initialize the ZoneJS testing environment.
53-
// It must be imported dynamically to avoid a static dependency on 'zone.js'.
54-
await import('zone.js/testing');
55-
}`;
56-
} else if (zoneTestingStrategy === 'dynamic-zone') {
57-
zoneTestingSnippet = `
58-
await import('zone.js');
59-
await import('zone.js/testing');`;
60-
}
61-
6246
// The DynamicDOMTestComponentRenderer is used to avoid stale document references
6347
// when running Vitest in non-isolated mode with JSDOM. It looks up the
6448
// document dynamically on every operation instead of caching it.
@@ -72,8 +56,6 @@ function createTestBedInitVirtualFile(
7256
import { afterEach, beforeEach } from 'vitest';
7357
${providersImport}
7458
75-
${zoneTestingSnippet}
76-
7759
// The beforeEach and afterEach hooks are registered outside the globalThis guard.
7860
// This ensures that the hooks are always applied, even in non-isolated browser environments.
7961
// Same as https://github.com/angular/angular/blob/05a03d3f975771bb59c7eefd37c01fa127ee2229/packages/core/testing/srcs/test_hooks.ts#L21-L29
@@ -150,37 +132,37 @@ function adjustOutputHashing(hashing?: OutputHashing): OutputHashing {
150132
}
151133

152134
/**
153-
* Resolves the Zone.js testing strategy by inspecting polyfills and resolving zone.js package.
135+
* Injects Zone.js and Zone.js testing polyfills into the build options based on the
136+
* project configuration and `polyfills` option.
154137
*
155-
* @param buildOptions The partial application builder options.
138+
* @param polyfills The configured polyfills from the test or build target.
156139
* @param projectSourceRoot The root directory of the project source.
157-
* @returns The resolved zone testing strategy ('none', 'static', 'dynamic', 'dynamic-zone').
140+
* @param logger The logger instance for reporting deprecation warnings.
141+
* @returns An array of polyfill specifiers to use for testing.
158142
*/
159-
function getZoneTestingStrategy(
160-
buildOptions: Partial<ApplicationBuilderInternalOptions>,
143+
function injectZoneJsTestingPolyfills(
144+
polyfills: string[] | undefined,
161145
projectSourceRoot: string,
162-
): 'none' | 'static' | 'dynamic' | 'dynamic-zone' {
163-
if (buildOptions.polyfills?.includes('zone.js/testing')) {
164-
return 'none';
165-
}
166-
167-
if (buildOptions.polyfills?.includes('zone.js')) {
168-
return 'static';
146+
logger: BuilderContext['logger'],
147+
): string[] {
148+
if (polyfills) {
149+
return injectTestingPolyfills(polyfills);
169150
}
170151

152+
// If polyfills is undefined (e.g. library build target), attempt to load zone.js if installed.
171153
try {
172154
const projectResolve = createProjectResolver(projectSourceRoot);
173155
projectResolve('zone.js');
174156

175-
// If polyfills is undefined (e.g. library build target), load zone.js dynamically.
176-
// If polyfills is defined but doesn't include zone.js (e.g. zoneless application), do NOT load zone.js.
177-
if (buildOptions.polyfills === undefined) {
178-
return 'dynamic-zone';
179-
}
157+
logger.warn(
158+
'Zone.js polyfills are being automatically injected because "zone.js" was detected in the project dependencies. ' +
159+
'This behavior is deprecated. If your project is zoneless, set the "polyfills" option to an empty array ("[]") in the ' +
160+
'test configuration. Otherwise, explicitly add "zone.js" to the "polyfills" option.',
161+
);
180162

181-
return 'none';
163+
return ['zone.js', 'zone.js/testing'];
182164
} catch {
183-
return 'none';
165+
return [];
184166
}
185167
}
186168

@@ -192,16 +174,19 @@ function getZoneTestingStrategy(
192174
*
193175
* @param options The normalized unit test builder options.
194176
* @param baseBuildOptions The base build config to derive testing config from.
177+
* @param logger The logger instance for reporting deprecation warnings.
195178
* @returns An async RunnerOptions configuration.
196179
*/
197180
export async function getVitestBuildOptions(
198181
options: NormalizedUnitTestBuilderOptions,
199182
baseBuildOptions: Partial<ApplicationBuilderInternalOptions>,
183+
logger: BuilderContext['logger'],
200184
): Promise<RunnerOptions> {
201185
const {
202186
workspaceRoot,
203187
projectSourceRoot,
204188
include,
189+
polyfills,
205190
exclude = [],
206191
watch,
207192
providersFile,
@@ -256,7 +241,11 @@ export async function getVitestBuildOptions(
256241

257242
const buildOptions: Partial<ApplicationBuilderInternalOptions> = {
258243
...baseBuildOptions,
259-
...(options.polyfills !== undefined ? { polyfills: options.polyfills } : {}),
244+
polyfills: injectZoneJsTestingPolyfills(
245+
polyfills ?? baseBuildOptions.polyfills,
246+
projectSourceRoot,
247+
logger,
248+
),
260249
watch,
261250
incrementalResults: watch,
262251
index: false,
@@ -285,9 +274,6 @@ export async function getVitestBuildOptions(
285274
externalDependencies,
286275
};
287276

288-
// Inject the zone.js testing polyfill if Zone.js is installed.
289-
const zoneTestingStrategy = getZoneTestingStrategy(buildOptions, projectSourceRoot);
290-
291277
let hasLocalize = false;
292278
try {
293279
const projectResolve = createProjectResolver(projectSourceRoot);
@@ -299,7 +285,6 @@ export async function getVitestBuildOptions(
299285
providersFile,
300286
projectSourceRoot,
301287
!options.debug,
302-
zoneTestingStrategy,
303288
hasLocalize,
304289
);
305290

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

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
import assert from 'node:assert';
1010
import type { TestRunner } from '../api';
1111
import { DependencyChecker } from '../dependency-checker';
12-
import { normalizeBrowserName } from './browser-provider';
1312
import { getVitestBuildOptions } from './build-options';
1413
import { VitestExecutor } from './executor';
1514

@@ -60,8 +59,8 @@ const VitestTestRunner: TestRunner = {
6059
checker.report();
6160
},
6261

63-
getBuildOptions(options, baseBuildOptions) {
64-
return getVitestBuildOptions(options, baseBuildOptions);
62+
getBuildOptions(options, baseBuildOptions, logger) {
63+
return getVitestBuildOptions(options, baseBuildOptions, logger);
6564
},
6665

6766
async createExecutor(context, options, testEntryPointMappings) {

‎packages/angular/build/src/builders/unit-test/tests/behavior/vitest-zone-init_spec.ts‎

Lines changed: 104 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,8 @@ import {
44
describeBuilder,
55
UNIT_TEST_BUILDER_INFO,
66
setupApplicationTarget,
7+
expectLog,
8+
expectNoLog,
79
} from '../setup';
810

911
describeBuilder(execute, UNIT_TEST_BUILDER_INFO, (harness) => {
@@ -68,7 +70,61 @@ describeBuilder(execute, UNIT_TEST_BUILDER_INFO, (harness) => {
6870
expect(result?.success).toBe(true);
6971
});
7072

71-
it('should load Zone and Zone testing support when testing a library and zone.js is installed', async () => {
73+
it('should NOT load Zone when test polyfills is empty even if zone.js is in build polyfills', async () => {
74+
setupApplicationTarget(harness, {
75+
polyfills: ['zone.js'],
76+
});
77+
78+
harness.useTarget('test', {
79+
...BASE_OPTIONS,
80+
polyfills: [],
81+
});
82+
83+
harness.writeFile(
84+
'src/app/app.component.spec.ts',
85+
`
86+
import { describe, it, expect } from 'vitest';
87+
88+
describe('Zoneless Override Test', () => {
89+
it('should NOT have Zone defined', () => {
90+
expect((globalThis as any).Zone).toBeUndefined();
91+
});
92+
});
93+
`,
94+
);
95+
96+
const { result } = await harness.executeOnce();
97+
expect(result?.success).toBeTrue();
98+
});
99+
100+
it('should load Zone when test polyfills includes zone.js even if build polyfills is empty', async () => {
101+
setupApplicationTarget(harness, {
102+
polyfills: [],
103+
});
104+
105+
harness.useTarget('test', {
106+
...BASE_OPTIONS,
107+
polyfills: ['zone.js'],
108+
});
109+
110+
harness.writeFile(
111+
'src/app/app.component.spec.ts',
112+
`
113+
import { describe, it, expect } from 'vitest';
114+
115+
describe('Zone Forced Test', () => {
116+
it('should have Zone defined', () => {
117+
expect((globalThis as any).Zone).toBeDefined();
118+
});
119+
});
120+
`,
121+
);
122+
123+
const { result } = await harness.executeOnce();
124+
expect(result?.success).toBeTrue();
125+
});
126+
127+
it('should load Zone and emit a deprecation warning when testing a library and zone.js is installed', async () => {
72128
harness.withBuilderTarget(
73129
'build',
74130
async () => ({ success: true }),
@@ -107,8 +163,54 @@ describeBuilder(execute, UNIT_TEST_BUILDER_INFO, (harness) => {
107163
`,
108164
);
109165

110-
const { result } = await harness.executeOnce();
166+
const { result, logs } = await harness.executeOnce();
167+
expect(result?.success).toBeTrue();
168+
expectLog(logs, /Zone\.js polyfills are being automatically injected/);
169+
});
170+
171+
it('should NOT load Zone and not emit warning when testing a library with polyfills: []', async () => {
172+
harness.withBuilderTarget(
173+
'build',
174+
async () => ({ success: true }),
175+
{
176+
project: 'ng-package.json',
177+
},
178+
{
179+
builderName: '@angular/build:ng-packagr',
180+
},
181+
);
182+
183+
await harness.writeFile(
184+
'ng-package.json',
185+
JSON.stringify({
186+
lib: {
187+
entryFile: 'src/public-api.ts',
188+
},
189+
}),
190+
);
191+
192+
harness.useTarget('test', {
193+
...BASE_OPTIONS,
194+
polyfills: [],
195+
include: ['src/app.component.spec.ts'],
196+
});
197+
198+
await harness.writeFile(
199+
'src/app.component.spec.ts',
200+
`
201+
import { describe, it, expect } from 'vitest';
202+
203+
describe('Library Zoneless Test', () => {
204+
it('should NOT have Zone defined', () => {
205+
expect((globalThis as any).Zone).toBeUndefined();
206+
});
207+
});
208+
`,
209+
);
210+
211+
const { result, logs } = await harness.executeOnce();
111212
expect(result?.success).toBeTrue();
213+
expectNoLog(logs, /Zone\.js polyfills are being automatically injected/);
112214
});
113215
});
114216
});

0 commit comments

Comments
 (0)