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
Sorry, something went wrong.
|
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. |
Sorry, something went wrong.
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.
Sorry, something went wrong.
| 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;
}- 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.
Sorry, something went wrong.
…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.
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-test is configured with an @angular/build:ng-packagr build target, the synthesized application-builder build never receives the test tsconfig's compilerOptions.customConditions. Module resolution therefore diverges from ng build, which honors customConditions natively through ng-packagr:
Concrete impact: monorepo setups that use customConditions (e.g. a source condition) to redirect workspace library imports to local TypeScript sources during development work as expected for ng build, but ng test silently 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-test builder now reads compilerOptions.customConditions from the test tsconfig (following the extends chain via the TypeScript config parser) and:
No schema changes and no public API surface changes; the new behavior is opt‑in via the existing compilerOptions.customConditions tsconfig field (TS 5.0+).
A new integration spec at packages/angular/build/src/builders/unit-test/tests/options/conditions_spec.ts locks the behavior in with two cases:
Does this PR introduce a breaking change?
The new behavior only activates when compilerOptions.customConditions is present in the test tsconfig — a field most projects do not set today. For @angular/build:application build targets that explicitly set conditions, that value is preserved (the backfill is guarded by conditions === undefined). For ng-packagr targets, this brings ng test into parity with ng build, which is a convergence rather than a divergence.
Other information
Files changed:
The Karma runner intentionally was not touched: it does not share the Vite resolver code path, and forwarding conditions through the application build it consumes is already covered by the same builder.ts change.](#33246)