From 9a62fb76f0d5619b4cfd28a25a2e6169bf6bae44 Mon Sep 17 00:00:00 2001 From: Alan Agius <17563226+alan-agius4@users.noreply.github.com> Date: Thu, 24 Sep 2026 07:54:38 +0000 Subject: [PATCH] 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 --- .../build/src/builders/unit-test/builder.ts | 6 +- .../src/builders/unit-test/runners/api.ts | 1 + .../unit-test/runners/vitest/build-options.ts | 73 +++++------- .../unit-test/runners/vitest/index.ts | 5 +- .../tests/behavior/vitest-zone-init_spec.ts | 106 +++++++++++++++++- 5 files changed, 138 insertions(+), 53 deletions(-) diff --git a/packages/angular/build/src/builders/unit-test/builder.ts b/packages/angular/build/src/builders/unit-test/builder.ts index dc8751778368..bb1da57a7108 100644 --- a/packages/angular/build/src/builders/unit-test/builder.ts +++ b/packages/angular/build/src/builders/unit-test/builder.ts @@ -295,7 +295,7 @@ export async function* execute( buildOptions: runnerBuildOptions, virtualFiles, testEntryPointMappings, - } = await runner.getBuildOptions(normalizedOptions, buildTargetOptions)); + } = await runner.getBuildOptions(normalizedOptions, buildTargetOptions, context.logger)); } catch (e) { assertIsError(e); context.logger.error( @@ -323,9 +323,7 @@ export async function* execute( const applicationBuildOptions = { ...buildTargetOptions, ...runnerBuildOptions, - ...(normalizedOptions.polyfills !== undefined - ? { polyfills: normalizedOptions.polyfills } - : {}), + polyfills: runnerBuildOptions.polyfills ?? normalizedOptions.polyfills, watch: normalizedOptions.watch, progress: normalizedOptions.buildProgress ?? buildTargetOptions.progress, quiet: normalizedOptions.quiet, diff --git a/packages/angular/build/src/builders/unit-test/runners/api.ts b/packages/angular/build/src/builders/unit-test/runners/api.ts index 43f65ef68adc..4cd455472bfa 100644 --- a/packages/angular/build/src/builders/unit-test/runners/api.ts +++ b/packages/angular/build/src/builders/unit-test/runners/api.ts @@ -63,6 +63,7 @@ export interface TestRunner { getBuildOptions( options: NormalizedUnitTestBuilderOptions, baseBuildOptions: Partial, + logger: BuilderContext['logger'], ): RunnerOptions | Promise; /** diff --git a/packages/angular/build/src/builders/unit-test/runners/vitest/build-options.ts b/packages/angular/build/src/builders/unit-test/runners/vitest/build-options.ts index a97cf05cb9e2..b3a7d8f37662 100644 --- a/packages/angular/build/src/builders/unit-test/runners/vitest/build-options.ts +++ b/packages/angular/build/src/builders/unit-test/runners/vitest/build-options.ts @@ -11,12 +11,13 @@ * Provides Vitest-specific build options and virtual file contents for Angular unit testing. */ +import type { BuilderContext } from '@angular-devkit/architect'; import path from 'node:path'; import { toPosixPath } from '../../../../utils/path'; import { createProjectResolver } from '../../../../utils/resolve-project'; import type { ApplicationBuilderInternalOptions } from '../../../application/options'; import { OutputHashing } from '../../../application/schema'; -import { NormalizedUnitTestBuilderOptions } from '../../options'; +import { type NormalizedUnitTestBuilderOptions, injectTestingPolyfills } from '../../options'; import { findTests, getTestEntrypoints } from '../../test-discovery'; import { RunnerOptions } from '../api'; @@ -26,14 +27,12 @@ import { RunnerOptions } from '../api'; * @param providersFile Optional path to a file that exports default providers. * @param projectSourceRoot The root directory of the project source. * @param teardown Whether to configure TestBed to destroy after each test. - * @param zoneTestingStrategy How zone.js should be loaded during initialization. * @returns The string content of the virtual initialization file. */ function createTestBedInitVirtualFile( providersFile: string | undefined, projectSourceRoot: string, teardown: boolean, - zoneTestingStrategy: 'none' | 'static' | 'dynamic' | 'dynamic-zone', hasLocalize: boolean, ): string { let providersImport = 'const providers = [];'; @@ -44,21 +43,6 @@ function createTestBedInitVirtualFile( providersImport = `import providers from './${importPath}';`; } - let zoneTestingSnippet = ''; - if (zoneTestingStrategy === 'static') { - zoneTestingSnippet = `import 'zone.js/testing';`; - } else if (zoneTestingStrategy === 'dynamic') { - zoneTestingSnippet = `if (typeof Zone !== 'undefined') { - // 'zone.js/testing' is used to initialize the ZoneJS testing environment. - // It must be imported dynamically to avoid a static dependency on 'zone.js'. - await import('zone.js/testing'); - }`; - } else if (zoneTestingStrategy === 'dynamic-zone') { - zoneTestingSnippet = ` - await import('zone.js'); - await import('zone.js/testing');`; - } - // The DynamicDOMTestComponentRenderer is used to avoid stale document references // when running Vitest in non-isolated mode with JSDOM. It looks up the // document dynamically on every operation instead of caching it. @@ -72,8 +56,6 @@ function createTestBedInitVirtualFile( import { afterEach, beforeEach } from 'vitest'; ${providersImport} - ${zoneTestingSnippet} - // The beforeEach and afterEach hooks are registered outside the globalThis guard. // This ensures that the hooks are always applied, even in non-isolated browser environments. // 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 { } /** - * Resolves the Zone.js testing strategy by inspecting polyfills and resolving zone.js package. + * Injects Zone.js and Zone.js testing polyfills into the build options based on the + * project configuration and `polyfills` option. * - * @param buildOptions The partial application builder options. + * @param polyfills The configured polyfills from the test or build target. * @param projectSourceRoot The root directory of the project source. - * @returns The resolved zone testing strategy ('none', 'static', 'dynamic', 'dynamic-zone'). + * @param logger The logger instance for reporting deprecation warnings. + * @returns An array of polyfill specifiers to use for testing. */ -function getZoneTestingStrategy( - buildOptions: Partial, +function injectZoneJsTestingPolyfills( + polyfills: string[] | undefined, projectSourceRoot: string, -): 'none' | 'static' | 'dynamic' | 'dynamic-zone' { - if (buildOptions.polyfills?.includes('zone.js/testing')) { - return 'none'; - } - - if (buildOptions.polyfills?.includes('zone.js')) { - return 'static'; + logger: BuilderContext['logger'], +): string[] { + if (polyfills) { + return injectTestingPolyfills(polyfills); } + // If polyfills is undefined (e.g. library build target), attempt to load zone.js if installed. try { const projectResolve = createProjectResolver(projectSourceRoot); projectResolve('zone.js'); - // If polyfills is undefined (e.g. library build target), load zone.js dynamically. - // If polyfills is defined but doesn't include zone.js (e.g. zoneless application), do NOT load zone.js. - if (buildOptions.polyfills === undefined) { - return 'dynamic-zone'; - } + logger.warn( + 'Zone.js polyfills are being automatically injected because "zone.js" was detected in the project dependencies. ' + + 'This behavior is deprecated. If your project is zoneless, set the "polyfills" option to an empty array ("[]") in the ' + + 'test configuration. Otherwise, explicitly add "zone.js" to the "polyfills" option.', + ); - return 'none'; + return ['zone.js', 'zone.js/testing']; } catch { - return 'none'; + return []; } } @@ -192,16 +174,19 @@ function getZoneTestingStrategy( * * @param options The normalized unit test builder options. * @param baseBuildOptions The base build config to derive testing config from. + * @param logger The logger instance for reporting deprecation warnings. * @returns An async RunnerOptions configuration. */ export async function getVitestBuildOptions( options: NormalizedUnitTestBuilderOptions, baseBuildOptions: Partial, + logger: BuilderContext['logger'], ): Promise { const { workspaceRoot, projectSourceRoot, include, + polyfills, exclude = [], watch, providersFile, @@ -256,7 +241,11 @@ export async function getVitestBuildOptions( const buildOptions: Partial = { ...baseBuildOptions, - ...(options.polyfills !== undefined ? { polyfills: options.polyfills } : {}), + polyfills: injectZoneJsTestingPolyfills( + polyfills ?? baseBuildOptions.polyfills, + projectSourceRoot, + logger, + ), watch, incrementalResults: watch, index: false, @@ -285,9 +274,6 @@ export async function getVitestBuildOptions( externalDependencies, }; - // Inject the zone.js testing polyfill if Zone.js is installed. - const zoneTestingStrategy = getZoneTestingStrategy(buildOptions, projectSourceRoot); - let hasLocalize = false; try { const projectResolve = createProjectResolver(projectSourceRoot); @@ -299,7 +285,6 @@ export async function getVitestBuildOptions( providersFile, projectSourceRoot, !options.debug, - zoneTestingStrategy, hasLocalize, ); diff --git a/packages/angular/build/src/builders/unit-test/runners/vitest/index.ts b/packages/angular/build/src/builders/unit-test/runners/vitest/index.ts index a1342e5abba8..f879628a2462 100644 --- a/packages/angular/build/src/builders/unit-test/runners/vitest/index.ts +++ b/packages/angular/build/src/builders/unit-test/runners/vitest/index.ts @@ -9,7 +9,6 @@ import assert from 'node:assert'; import type { TestRunner } from '../api'; import { DependencyChecker } from '../dependency-checker'; -import { normalizeBrowserName } from './browser-provider'; import { getVitestBuildOptions } from './build-options'; import { VitestExecutor } from './executor'; @@ -60,8 +59,8 @@ const VitestTestRunner: TestRunner = { checker.report(); }, - getBuildOptions(options, baseBuildOptions) { - return getVitestBuildOptions(options, baseBuildOptions); + getBuildOptions(options, baseBuildOptions, logger) { + return getVitestBuildOptions(options, baseBuildOptions, logger); }, async createExecutor(context, options, testEntryPointMappings) { diff --git a/packages/angular/build/src/builders/unit-test/tests/behavior/vitest-zone-init_spec.ts b/packages/angular/build/src/builders/unit-test/tests/behavior/vitest-zone-init_spec.ts index 3caf15a2cf3f..147cfc31c53e 100644 --- a/packages/angular/build/src/builders/unit-test/tests/behavior/vitest-zone-init_spec.ts +++ b/packages/angular/build/src/builders/unit-test/tests/behavior/vitest-zone-init_spec.ts @@ -4,6 +4,8 @@ import { describeBuilder, UNIT_TEST_BUILDER_INFO, setupApplicationTarget, + expectLog, + expectNoLog, } from '../setup'; describeBuilder(execute, UNIT_TEST_BUILDER_INFO, (harness) => { @@ -68,7 +70,61 @@ describeBuilder(execute, UNIT_TEST_BUILDER_INFO, (harness) => { expect(result?.success).toBe(true); }); - it('should load Zone and Zone testing support when testing a library and zone.js is installed', async () => { + it('should NOT load Zone when test polyfills is empty even if zone.js is in build polyfills', async () => { + setupApplicationTarget(harness, { + polyfills: ['zone.js'], + }); + + harness.useTarget('test', { + ...BASE_OPTIONS, + polyfills: [], + }); + + harness.writeFile( + 'src/app/app.component.spec.ts', + ` + import { describe, it, expect } from 'vitest'; + + describe('Zoneless Override Test', () => { + it('should NOT have Zone defined', () => { + expect((globalThis as any).Zone).toBeUndefined(); + }); + }); + `, + ); + + const { result } = await harness.executeOnce(); + expect(result?.success).toBeTrue(); + }); + + it('should load Zone when test polyfills includes zone.js even if build polyfills is empty', async () => { + setupApplicationTarget(harness, { + polyfills: [], + }); + + harness.useTarget('test', { + ...BASE_OPTIONS, + polyfills: ['zone.js'], + }); + + harness.writeFile( + 'src/app/app.component.spec.ts', + ` + import { describe, it, expect } from 'vitest'; + + describe('Zone Forced Test', () => { + it('should have Zone defined', () => { + expect((globalThis as any).Zone).toBeDefined(); + }); + }); + `, + ); + + const { result } = await harness.executeOnce(); + expect(result?.success).toBeTrue(); + }); + + it('should load Zone and emit a deprecation warning when testing a library and zone.js is installed', async () => { harness.withBuilderTarget( 'build', async () => ({ success: true }), @@ -107,8 +163,54 @@ describeBuilder(execute, UNIT_TEST_BUILDER_INFO, (harness) => { `, ); - const { result } = await harness.executeOnce(); + const { result, logs } = await harness.executeOnce(); + expect(result?.success).toBeTrue(); + expectLog(logs, /Zone\.js polyfills are being automatically injected/); + }); + + it('should NOT load Zone and not emit warning when testing a library with polyfills: []', async () => { + harness.withBuilderTarget( + 'build', + async () => ({ success: true }), + { + project: 'ng-package.json', + }, + { + builderName: '@angular/build:ng-packagr', + }, + ); + + await harness.writeFile( + 'ng-package.json', + JSON.stringify({ + lib: { + entryFile: 'src/public-api.ts', + }, + }), + ); + + harness.useTarget('test', { + ...BASE_OPTIONS, + polyfills: [], + include: ['src/app.component.spec.ts'], + }); + + await harness.writeFile( + 'src/app.component.spec.ts', + ` + import { describe, it, expect } from 'vitest'; + + describe('Library Zoneless Test', () => { + it('should NOT have Zone defined', () => { + expect((globalThis as any).Zone).toBeUndefined(); + }); + }); + `, + ); + + const { result, logs } = await harness.executeOnce(); expect(result?.success).toBeTrue(); + expectNoLog(logs, /Zone\.js polyfills are being automatically injected/); }); }); });