Match Strada's global diagnostic collection, hide non-diag global errors in editor - #64452
Merged
Jake Bailey (jakebailey) merged 6 commits intoSep 25, 2026
Conversation
Recursive mapped types can check without errors but report TS2589 after an incremental signature calculation or a hover request. Capture those paths alongside genuine deferred globals to distinguish diagnostic collection changes from changes to recursive type computation. For 64405
Diagnostic collection must distinguish errors produced during checking from work done only to serialize types. Otherwise comment-only edits can fail even when a fresh build of the same source succeeds. Follow Strada's collection boundaries while retaining genuine checking errors across cached requests. This also preserves its known limitation: a missing global first discovered during signature generation can be absent from the subsequent semantic result. For 64405
Hover and other editor queries can produce incidental errors while serializing types, even when semantic checking reports no errors. Publishing those globals makes diagnostics depend on which editor features happened to run. Only the diagnostics checker should contribute to the project's reported globals; query and API checkers must remain isolated. For 64405
typescript-automation
Bot
added
Author: Team
For Uncommitted Bug
Globals such as missing iterator types can be discovered only while checking source files. Keep ordinary CLI reporting covered while incremental diagnostic collection is being separated from that path. For 64405
Anders Hejlsberg (ahejlsberg)
approved these changes
Sep 25, 2026
Member
Author
|
I have the cleanup prepared; sorry, didn't intend for you to look at it quite yet. |
Only incremental caching needs to associate newly discovered globals with individual files. Ordinary semantic diagnostic requests should remain file-only, without requiring callers to filter out project diagnostics. Preserve ordinary CLI reporting of globals discovered during checking without collecting incidental signature-generation errors in incremental builds.
Jake Bailey (jakebailey)
marked this pull request as ready for review
September 25, 2026 22:35
Jake Bailey (jakebailey)
requested review from
Anders Hejlsberg (ahejlsberg)
and
a balanced review from Copilot
September 25, 2026 22:35
Copilot started reviewing on behalf of
Jake Bailey (jakebailey)
September 25, 2026 22:36
View session
Contributor
There was a problem hiding this comment.
Copilot review overview
🟡 Changes recommended
Mapped diagnostic directives can incorrectly suppress newly cached no-file global diagnostics.
Get a fresh assessment by requesting another Copilot review.
Review effort: Balanced
Findings: 1
Open (1)
What changed in this PR
Aligns global diagnostic collection with Strada while preventing editor queries from publishing incidental global errors.
Changes:
- Caches newly discovered global diagnostics with incremental semantic results.
- Restricts project-global collection to diagnostic checkers.
- Adds CLI, incremental, and editor regression coverage.
| File | Description |
|---|---|
tsc/testdata/baselines/reference/tsc/incremental/recursive-readonly-mapped-type-after-comment-only-edit.js |
Baselines readonly recursive-type edits. |
tsc/testdata/baselines/reference/tsc/incremental/recursive-mapped-type-after-comment-only-edit.js |
Baselines recursive-type edits. |
tsc/testdata/baselines/reference/tsc/incremental/js-file-with-import-in-jsdoc-in-composite-project.js |
Updates unchecked-JS expectations. |
tsc/testdata/baselines/reference/tsc/incremental/global-diagnostics-produced-during-unchecked-javascript-checking.js |
Covers delayed JavaScript diagnostics. |
tsc/testdata/baselines/reference/tsc/incremental/global-diagnostics-produced-during-semantic-checking.js |
Covers cached global diagnostics. |
tsc/testdata/baselines/reference/tsc/incremental/global-diagnostics-from-function-bodies-after-incremental-edits.js |
Covers body-originated globals. |
tsc/testdata/baselines/reference/tsc/commandLine/global-diagnostics-produced-during-ordinary-semantic-checking.js |
Covers non-incremental CLI behavior. |
tsc/testdata/baselines/reference/project/query-globals-before-semantic-checking.jsonc |
Baselines query-first editor behavior. |
tsc/testdata/baselines/reference/project/query-globals-after-semantic-checking.jsonc |
Baselines diagnostics-first behavior. |
tsc/internal/project/project_test.go |
Tests editor diagnostic isolation. |
tsc/internal/project/checkerpool.go |
Restricts global collection to diagnostic checkers. |
tsc/internal/project/checkerpool_test.go |
Updates checker-pool coverage. |
tsc/internal/execute/tsctests/tsc_test.go |
Adds CLI and incremental scenarios. |
tsc/internal/execute/incremental/program.go |
Uses incremental-specific collection. |
tsc/internal/compiler/program.go |
Tracks globals discovered per file. |
tsc/internal/checker/checker.go |
Flushes deferred diagnostics before global retrieval. |
💡 Add a code-review agent skill for context-aware, tailored reviews. Learn more in the docs.
Mapped directives apply only to diagnostics in their source file. Globals captured for incremental checking must not be suppressed by a directive covering offset zero or count toward satisfying an expect directive.
Anders Hejlsberg (ahejlsberg)
approved these changes
Sep 25, 2026
Jake Bailey (jakebailey)
enabled auto-merge
September 25, 2026 23:21
Jake Bailey (jakebailey)
deleted the
fix-incremental-diagnostic-collection
branch
September 25, 2026 23:51
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
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.
Fixes #64405
After chatting with Anders Hejlsberg (@ahejlsberg), here's another fix for #64405 that just goes back to the way Strada did things.
I'm not happy about the first fix because I'm copying Strada and therefore the super weird thing where GetSemanticsDiagnostics for a file returns non-file diags. I'm looking into a way to split that apart, but it's looking invasive.