Open
Conversation
… configuration When a user specifies `test.exclude` inside their `vitest.config.ts`, the tests are correctly excluded during Vitest's execution phase. However, because the Angular CLI extracts test files earlier in the process using its own `exclude` builder option, those skipped tests are still unnecessarily compiled by esbuild in-memory. This adds a warning to explicitly notify developers of this hidden build overhead and suggests using the Angular CLI `exclude` option instead to improve performance.
clydin
added
the
action: review
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces several valuable improvements to how Vitest configurations are merged, addressing potential conflicts between CLI options and user-defined configurations. The changes to prioritize CLI arguments for reporters and to warn about performance implications of test.exclude are well-implemented. The new logic for merging coverage.exclude arrays is also a good step towards more intuitive behavior. However, I've identified a potential issue in the implementation of the coverage.exclude merging that could lead to incorrect behavior under certain conditions. The new tests are comprehensive and validate the intended changes effectively.
clydin
force-pushed
the
vitest/config-improvements
branch
from
27dfcf6 to
bbf5621
Compare
Previously, providing a --coverage-exclude CLI option to the builder would completely clobber any custom coverage.exclude items defined natively within vitest.config.ts. This correctly merges both sources using an internal Set to prevent duplicate exclusions and preserves configurations so developers can combine global ignores alongside CLI-specific boundaries.
…ding Vitest configuration When leveraging the Angular CLI to run tests with a specific set of `--reporters`, Vitest's underlying `mergeConfig` logic would normally array-concatenate the CLI reporters with any reporters defined natively inside `vitest.config.ts`. This led to duplicate output processors (e.g. running 'default' twice). By explicitly wiping the original configurations when CLI overrides are present, the CLI now acts as a strict Source-of-Truth array replacer, which is the expected behavior for CI and custom targets.
clydin
force-pushed
the
vitest/config-improvements
branch
from
bbf5621 to
58bcba8
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This PR introduces several architectural improvements to how the Angular CLI (
@angular/build) merges and processes customvitest.config.tsoptions.Previously, certain CLI arguments would either silently clobber user-defined configuration arrays, or allow Vite's mergeConfig to create duplicated outputs. Additionally, we identified areas where
vitest.config.tsproperties could cause hidden performance overheads.