Repository navigation
perf: stop a TypeScript run nothing will report once the next rebuild starts - #389
Closed
alexander-akait wants to merge 2 commits into
Closed
alexander-akait wants to merge 2 commits into
alexander-akait wants to merge 2 commits into
Conversation
… starts A check that reportAs keeps off the compilation runs after a watch rebuild. When the next rebuild starts, its report is dropped, yet the TypeScript worker kept checking the whole program before it would start the run that matters. The plugin now tells a check to stop a run of an older compilation as soon as it takes over from it, through an optional cancel() on the check instance. The TypeScript check passes a CancellationToken backed by a SharedArrayBuffer to its worker, so the run stops between one file and the next and the builder keeps what it got through. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01App18sYbe6Vf6HDCjCXbvf
One or more co-authors of this pull request were not found. You must specify co-authors in commit message trailer via: Supported
Alternatively, if the co-author should not be included, remove the Please update your commit message(s) by doing |
|
Warning Review limit reachedYou've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository. Next included review available in 5 minutes. View limit details
WalkthroughThe changes add cancellation for TypeScript checks running in a worker. New runs can mark older runs as stopped, and TypeScript diagnostic queries receive a cancellation token. Detached reports request cancellation and retain a promise that settles after the report completes. A test exercises cancellation during watch builds with a stalling TypeScript mock. The README and changeset describe the behavior. Priority: ➖ Normal Merge Risk: 🟡 Moderate · up to
|
| Check name | Status | Explanation |
|---|---|---|
| Description Check |  |
Check skipped - CodeRabbit’s high-level summary is enabled. |
| Title check | |
The title clearly describes the main change: canceling a stale TypeScript run when the next rebuild starts. It is specific and related to the pull request objective. |
| Docstring Coverage | |
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 13 files. (2 skipped: … |
| Linked Issues check | |
Check skipped because no linked issues were found for this pull request. |
| Out of Scope Changes check | |
Check skipped because no linked issues were found for this pull request. |
Full details: Docstring Coverage
-
Explanation
Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 10 functions across 13 files. (2 skipped: 2 unsupported.)
✨ Finishing Touches: 💡 1
-
🛠️ Fix failing CI checks: 💡
-
- Commit to this branch
- Create a new PR
📝 Generate docstrings
-
- Commit to this branch
- Create a new PR
🧪 Generate unit tests (beta)
-
- Commit to this branch
- Create a new PR
-
- Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.
Comment @coderabbitai help to get the list of available commands.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
- Source mode: Auto
- Repository: Unavailable
- Branch: Unavailable
- File: No configuration file from this repository applied
- Commit: Unavailable
- Layers: Organization UI
- Review profile: CHILL
- Plan: Advanced
- Run ID:
5d97d564-c8c9-4b66-a709-eae767cb54c4
📒 Files selected for processing: 15
.changeset/ts-stop-stale-runs.mdREADME.mdsrc/check.jssrc/checks/index.jssrc/checks/typescript-program.jssrc/checks/typescript-worker.jssrc/checks/typescript.jssrc/threads.jstest/typescript/cancel.test.jstest/typescript/fixtures/cancel/index.jstest/typescript/mock/typescript-stall/index.jstypes/checks/index.d.tstypes/checks/typescript-program.d.tstypes/checks/typescript-worker.d.tstypes/threads.d.ts
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.
Two compilers linting the same files at once share one run. Stopping it for the compiler whose compilation is stale would have handed the other one an empty answer. A run another compiler joined is now left to finish, and a run that is stopped is taken out of the shared runs, so a compiler coming later starts its own. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01App18sYbe6Vf6HDCjCXbvf
Member
Author
|
EasyCLA fails because the commits carry a Generated by Claude Code |
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #389 +/- ##
=======================================
Coverage 99.64% 99.64%
=======================================
Files 19 19
Lines 5618 5706 +88
=======================================
+ Hits 5598 5686 +88
Misses 20 20 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
alexander-akait
added a commit
that referenced
this pull request
Oct 29, 2026
…red TypeScript runs (#392) * perf: watch rebuilds, TypeScript 7, and shared or stopped TypeScript runs One change in place of #371, #384, #387, #389, #390 and #391. Core: - One place keeps what lives as long as a compiler (src/compiler-state.js): keptFor() for state kept per compiler (results, detached reports, the import graph, the TypeScript program), and acquire()/release() for the workers and processes compilers share, started once per key and stopped when the last compiler using them shuts down or closes its watch. The TypeScript check's worker and TypeScript 7's process both go through it, in place of a lifecycle of their own each. - lintDependentsAfterBuild (opt-in): a watch rebuild of a check that reads across files lints only what webpack rebuilt, and what the change reaches after the build. - A check can stop a run of an older compilation nothing will report (cancel()); one another compiler joined is never stopped. - Biome and oxlint are handed every file at once rather than in batches. TypeScript: - TypeScript 7 runs through the API it ships, in a process of its own kept open while webpack watches; the import graph answers without a graph there. - A stale run stops between files; compilers checking the same project alike share one worker and one program. Nothing is listed by hand where the tool can say it: assets and the scripts a config file picks up are told apart by the extensions TypeScript reads, the fallback resolution mode and the error category come from the loaded TypeScript, and each run carries its own stop flag. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01App18sYbe6Vf6HDCjCXbvf * test: cover the TypeScript 7 check's error paths rather than ignore them The line under a diagnostic is printed from the text the native compiler checked (Program.getSourceFile) rather than read from disk again, and a file gone since webpack saw it is told by statSync without throwing, so neither needs a catch. A project with no tsconfig.json above it is now tested from a directory outside this repository. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01App18sYbe6Vf6HDCjCXbvf * fix: match the files webpack saw change against the native compiler's on Windows webpack names a changed file with the platform's separators, the native compiler with forward slashes, and Windows tells no case apart, so both are now compared by the same key. The cancel test accepts a run already stopped by the time it looks. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01App18sYbe6Vf6HDCjCXbvf * fix: take turns on a shared TypeScript 7 process, and forget a stopped run Compilers checking the same project share the native compiler's process, and a run replaces the snapshot and disposes of the last one, which a run for another compiler could still be reading: runs on one process now take turns. A run stopped for a newer compilation found nothing, so it is no longer kept as what a rebuild the program does not see is answered with. A directory no config file covers is now tested in the import graph. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01App18sYbe6Vf6HDCjCXbvf * refactor: check TypeScript 7 through its 7.1 API, and drop what did not pay off TypeScript 7.1 ships its API as typescript/async, makes snapshots with createSnapshot and snapshot.update, and formats diagnostics itself. The native check now uses that API, found by what the package exports rather than by its version, and TypeScript's own formatter in place of one written here. TypeScript 7.0's API was an unstable preview with none of these calls; a TypeScript offering no API to check with now says so. Measured again on this branch, two additions did not earn their place: - lintDependentsAfterBuild finished the build sooner but printed an error it found in an importer no sooner (1.96-2.64 s against 2.09-2.21 s without it), at the cost of an option and a second report. - cancel() on the checker API only ever served a TypeScript 6 check on a worker reporting as advisory, and TypeScript 7 does not use it. Both are removed, with what they needed: a stop flag per worker run, bookkeeping of which runs another compiler joined, and their tests. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01App18sYbe6Vf6HDCjCXbvf * fix: watch the config files a TypeScript 7 project extends, and let a run finish before closing The native check reported only the root config file to the watcher, so an edit to a config file it extends, outside the folder watched, went unnoticed unless webpack's own tsconfig resolution happened to watch it. It now reports every config file the program names (getConfigFileNames), and tells the compiler of a change to one as a change rather than a new file. Closing the native process now waits for a run still going. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01App18sYbe6Vf6HDCjCXbvf --------- Co-authored-by: Claude <noreply@anthropic.com>
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.
Summary
A check that
reportAskeeps off the compilation ("log",false, …) runs after a watch rebuild. When a newer rebuild starts, the older run's report is dropped. Until now the TypeScript worker still finished checking the whole program for that run, and only then started the run whose result is printed.cancel(). When a runner takes over a detached run of an older compilation (takeDetachedReport), it callscancel()before waiting for that run to settle. Nothing prints that report any more.threadson):createSolonumbers each run and shares anInt32Arrayover aSharedArrayBufferwith the worker.cancel()marks every run issued so far as stopped. A message would wait behind the run it is meant to stop.CancellationTokento the builder: syntactic, global, declaration and semantic diagnostics.OperationCanceledException. The builder keeps what it got through, and the next run checks the rest. The cached declaration diagnostics are dropped, because which affected files were drained is lost with the run.Measurements
.tsfiles (scratchpad/bench-heavy), watch mode,reportAs: "log",threads: "auto".The logged report is identical on both: 601 errors, same hash after stripping the temp dir.
Tests
test/typescript/cancel.test.js:// stallfile until the run is cancelled, and writes which way it ended.Atomics.storeincancel()removed, the test fails with'timed out' !== 'cancelled'.countThreads("auto") < 2, as the check only has a worker of its own there.tsc, schema lint, prettier, cspell and the full suite (331 tests) pass locally.Notes
This touches
typescript-program.jscheck(), which #383 and #379 also change, so whichever lands second needs a small merge.Generated by Claude Code
Summary by CodeRabbit