fix(@angular/build): respect tsconfig customConditions in unit-test builder#33246
fix(@angular/build): respect tsconfig customConditions in unit-test builder#33246RobbyRabbitman wants to merge 1 commit into
Conversation
|
Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). View this failed invocation of the CLA check for more information. For the most up to date status, view the checks section at the bottom of the pull request. |
320be5f to
7c4e758
Compare
There was a problem hiding this comment.
Code Review
This pull request introduces support for customConditions in the unit-test builder, ensuring parity with ng build when using @angular/build:ng-packagr. It adds logic to read compilerOptions.customConditions from the test tsconfig and forwards these conditions to the application build and Vitest runner. Feedback suggests simplifying the tsconfig parsing logic by using the higher-level ts.getParsedCommandLineOfConfigFile API.
| function readCustomConditionsFromTsConfig(tsConfigPath: string): string[] | undefined { | ||
| const { config, error } = ts.readConfigFile(tsConfigPath, (p) => ts.sys.readFile(p)); | ||
| if (error || !config) { | ||
| return undefined; | ||
| } | ||
|
|
||
| const parsed = ts.parseJsonConfigFileContent( | ||
| config, | ||
| ts.sys, | ||
| path.dirname(tsConfigPath), | ||
| /* existingOptions */ undefined, | ||
| tsConfigPath, | ||
| ); | ||
|
|
||
| const conditions = parsed.options.customConditions; | ||
|
|
||
| return Array.isArray(conditions) && conditions.length > 0 ? [...conditions] : undefined; | ||
| } |
There was a problem hiding this comment.
The logic for reading and parsing the TypeScript configuration can be simplified by using ts.getParsedCommandLineOfConfigFile. This high-level API handles reading the file and resolving the extends chain in a single call, making the code more concise. Additionally, while ignoring parsing errors (available in parsed.errors) is acceptable for this optional feature, logging them could help users debug issues with their tsconfig configuration.
function readCustomConditionsFromTsConfig(tsConfigPath: string): string[] | undefined {
const parsed = ts.getParsedCommandLineOfConfigFile(tsConfigPath, {}, ts.sys);
const conditions = parsed?.options.customConditions;
return Array.isArray(conditions) && conditions.length > 0 ? [...conditions] : undefined;
}References
- When refactoring code that handles configuration properties, ensure that any implicit type validation previously performed is still maintained, especially if the configuration is guaranteed to be type-safe by an upstream process.
…uilder The unit-test builder synthesizes an application-builder build from the configured buildTarget. The application builder forwards `conditions` into esbuild's `build.initialOptions.conditions`, and the Angular compiler plugin in turn assigns that array onto the in-plugin TypeScript program's `compilerOptions.customConditions`. When the buildTarget is `@angular/build:ng-packagr`, the synthesized application options never set `conditions` (ng-packagr has no such schema field; it honors `compilerOptions.customConditions` natively at the tsconfig level instead). As a result both esbuild and the TypeScript program resolve with default conditions only, diverging from `ng build` and silently breaking monorepo setups that use `customConditions` to redirect workspace library imports to local sources during development. Read `compilerOptions.customConditions` from the test tsconfig (following the `extends` chain via `ts.parseJsonConfigFileContent`) and: - forward them as `conditions` to the synthesized application build, but only when the buildTarget did not already set `conditions` (preserves application-builder behavior including explicit `conditions: []` and user-supplied lists), and - append them to Vite's `resolve.conditions` so the Vitest runner's own resolver matches the build-time resolution. This aligns esbuild, the compiler plugin's TypeScript program, and Vitest on the same condition set without introducing new options or schema fields; the new behavior is opt-in via the existing tsconfig field.
7c4e758 to
34a4902
Compare
PR Checklist
Please check to confirm your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
What is the current behavior?
When
@angular/build:unit-testis configured with an@angular/build:ng-packagrbuild target, the synthesized application-builder build never receives the test tsconfig'scompilerOptions.customConditions. Module resolution therefore diverges fromng build, which honorscustomConditionsnatively through ng-packagr:transformNgPackagrOptions()only forwardsstylePreprocessorOptions,assetsandinlineStyleLanguagefromng-package.json; ng-packagr's schema has noconditionsfield, so nothing is carried over.ApplicationBuilderInternalOptionsis then handed tobuildApplicationInternal()withoutconditions, so esbuild resolves library imports with only the default conditions (module,development/production, plusbrowser/node).resolve.conditionswas hardcoded to['es2015', 'es2020', 'module', (browser)], also ignoring custom conditions.Concrete impact: monorepo setups that use
customConditions(e.g. asourcecondition) to redirect workspace library imports to local TypeScript sources during development work as expected forng build, butng testsilently falls back to the published entry points — defeating the purpose of source‑mapped local development and making library tests harder to debug.Issue Number: N/A
What is the new behavior?
The
unit-testbuilder now readscompilerOptions.customConditionsfrom the test tsconfig (following theextendschain via the TypeScript config parser) and:conditionsinto the synthesized application-builder options, but only when the build target did not already setconditions. This preserves the existing application-builder behavior, including explicit opt‑outs (conditions: []) and user-supplied lists.resolve.conditionsin the Vitest plugin, so the runner's own resolver matches the build-time resolution.No schema changes and no public API surface changes; the new behavior is opt‑in via the existing
compilerOptions.customConditionstsconfig field (TS 5.0+).A new integration spec at
packages/angular/build/src/builders/unit-test/tests/options/conditions_spec.tslocks the behavior in with two cases:customConditions: ['staging']intsconfig.spec.jsonand a#targetimportsmap that exposes the good source only under thestagingcondition, the spec resolves to the good source and passes.customConditionsdeclared; the spec must fall back to thedefault(bad) target, proving the new behavior is opt‑in and does not inject unrelated conditions.Does this PR introduce a breaking change?
The new behavior only activates when
compilerOptions.customConditionsis present in the test tsconfig — a field most projects do not set today. For@angular/build:applicationbuild targets that explicitly setconditions, that value is preserved (the backfill is guarded byconditions === undefined). For ng-packagr targets, this bringsng testinto parity withng build, which is a convergence rather than a divergence.Other information
Files changed:
packages/angular/build/src/builders/unit-test/options.ts— readcustomConditionsfrom the test tsconfig viats.parseJsonConfigFileContent(extends‑aware) and expose it on the normalized options.packages/angular/build/src/builders/unit-test/builder.ts— backfillconditionsinto the application-builder options only when the build target did not set it.packages/angular/build/src/builders/unit-test/runners/vitest/executor.ts— threadcustomConditionsthrough to the Vitest config plugin.packages/angular/build/src/builders/unit-test/runners/vitest/plugins.ts— appendcustomConditionsto Vite'sresolve.conditions.packages/angular/build/src/builders/unit-test/tests/options/conditions_spec.ts— new integration spec.The Karma runner intentionally was not touched: it does not share the Vite resolver code path, and forwarding
conditionsthrough the application build it consumes is already covered by the samebuilder.tschange.](#33246)