fix(typecheck): log experimental feature warning once per run#10033
fix(typecheck): log experimental feature warning once per run#10033bwyard wants to merge 4 commits intovitest-dev:mainfrom
Conversation
✅ Deploy Preview for vitest-dev ready!Built without sensitive environment variables
To edit notification comments on pull requests, go to your Netlify project configuration. |
Replaces the module-level Set<string> with a WeakSet<Logger>. The Set persisted across runVitest calls in the same process, suppressing the warning for subsequent runs. WeakSet keyed by logger instance scopes dedup to one warning per Vitest run while still deduplicating across multiple projects in the same run. Adds a regression test: two projects with typecheck.enabled: true in a single runVitest call produce exactly one occurrence of the warning. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
bwyard
changed the title
|
The Rolldown&Test: node-22, macos-latest failure looks like a pre-existing flaky UI test (test/ui/test/ui.spec.ts locator timeout). Same job is also failing on main and on other unrelated PRs (e.g. #7100). Flagging in case it's worth a look before reviewing. Will appreciate any guidance on how to proceed. |
This comment was marked as low quality.
This comment was marked as low quality.
Sorry, something went wrong.
|
Thanks I actually started with set as a fix and off the top of my head can't remember but I ran into issue or tests or something that caused set to not work how I washed to so swapped to weak set. Not at computer now but the regression and the gc are good observations, when I'm at my computer I'll sit down and double check things based on your feedback as well just to be sure I covered everything. |
|
Thanks. Checked the Logger lifecycle: On the test: I believe the original did cover it, but replaced it with a version that makes the intent clearer. |
| if (resolved.typecheck.enabled) { | ||
| if (resolved.typecheck.enabled && !loggedExperimentalWarnings.has(logger)) { | ||
| loggedExperimentalWarnings.add(logger) | ||
| logger.console.warn( |
There was a problem hiding this comment.
I think we should define logger.warnOnce like vite does. And maybe we should use it for all experimental warning
There was a problem hiding this comment.
I'm definitely not against that, and id prefer that as well was just trying to keep the pr small and to scope since it's first contribution. But I'll go ahead and work on doing it that's way it's probably cleaner.
There was a problem hiding this comment.
Got real sick I'll look at this again this weekend
3 participants
When using Vitest with multiple projects that have typecheck.enabled: true, the experimental feature warning is logged once per project config resolved. In a large monorepo this can mean 30+ identical warnings per run. Fixes #9823.
Changed the deduplication key from a string in a module-level Set to the Logger instance in a WeakSet. The logger is constructed per Vitest instance, so the warning fires once per run regardless of how many project configs are resolved - and correctly resets between separate runs.
Added a test: two projects with typecheck.enabled: true in a single run produce exactly one occurrence of the warning.
Noticed the same pattern in the benchmark() function in cac.ts. Left it out, out of scope.
AI assisted: Claude