Repository navigation
feat: TypeScript 7.1 support, one place for compiler-lived state, shared TypeScript runs - #392
alexander-akait wants to merge 6 commits into
Conversation
…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
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 |
|
No actionable comments were generated in the recent review.
 |
| Check name | Status | Explanation | Resolution |
|---|---|---|---|
| Docstring Coverage |  |
Docstring coverage is 75.00% which is insufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 56 functions across 46 files. | Write docstrings for the functions missing them to satisfy the coverage threshold. |
-
Check name Status Explanation Description Check Check skipped - CodeRabbit’s high-level summary is enabled. Title check The title clearly summarizes the main changes: TypeScript 7.1 support, centralized compiler-lived state, and shared TypeScript runs. 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.
Finishing Touches: 💡 2
-
📝 Generate docstrings: 💡
-
- Commit to this branch
- Create a new PR
🛠️ Fix failing CI checks: 💡
-
- 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
Warning
Some tools did not complete. Review the errors below.
🔧 Biome (2.5.15)
-
src/checks/typescript-native.js
-
Biome could not lint this file: configuration resulted in errors. Check the repository's Biome configuration and plugins.
test/typescript/native.test.js
-
Biome could not lint this file: configuration resulted in errors. Check the repository's Biome configuration and plugins.
-
🔧 ESLint
-
If the error stems from missing dependencies, add them to the package.json file. For unrecoverable errors (e.g., due to private dependencies), disable the tool in the CodeRabbit configuration.
test/typescript/native.test.js
-
Oops! Something went wrong! :(
ESLint: 10.10.0
SyntaxError: Unexpected token ':'
at compileSourceTextModule (node:internal/modules/esm/utils:319:16)
at ModuleLoader.moduleStrategy (node:internal/modules/esm/translators:89:18)
at#translate(node:internal/modules/esm/loader:438:20)
at afterLoad (node:internal/modules/esm/loader:506:29)
at ModuleLoader.loadAndTranslate (node:internal/modules/esm/loader:511:12)
at#getOrCreateModuleJobAfterResolve(node:internal/modules/esm/loader:564:36)
at afterResolve (node:internal/modules/esm/loader:611:52)
at ModuleLoader.getOrCreateModuleJob (node:internal/modules/esm/loader:617:12)
at node:internal/modules/esm/loader:636:32
at TracingChannel.tracePromise (node:diagnostics_channel:370:14)
-
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.
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #392 +/- ##
==========================================
+ Coverage 99.64% 99.68% +0.03%
==========================================
Files 19 21 +2
Lines 5618 6319 +701
==========================================
+ Hits 5598 6299 +701
Misses 20 20 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
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
… 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
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
ℹ️ 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:
54408489-f152-4f8e-88f1-8d39f2e2b612
⛔ Files ignored due to path filters: 1
package-lock.jsonis excluded by!**/package-lock.json
📒 Files selected for processing: 57
.changeset/assets-named-by-typescript.md.changeset/cli-checks-one-run.md.changeset/import-graph-typescript-7.md.changeset/lint-dependents-after-build.md.changeset/ts-share-program-across-compilers.md.changeset/ts-stop-stale-runs.md.changeset/typescript-7.md.cspell.jsonREADME.mdpackage.jsonsrc/check.jssrc/checks/biome.check.jssrc/checks/biome.jssrc/checks/eslint.check.jssrc/checks/index.jssrc/checks/oxlint.check.jssrc/checks/oxlint.jssrc/checks/stylelint.check.jssrc/checks/typescript-native.jssrc/checks/typescript-program.jssrc/checks/typescript-worker.jssrc/checks/typescript.check.jssrc/checks/typescript.jssrc/compiler-state.jssrc/import-graph.jssrc/index.jssrc/options.check.jssrc/options.jssrc/options.jsonsrc/shared-options.jsonsrc/threads.jstest/after-build.test.jstest/compiler-state.test.jstest/import-graph.test.jstest/incremental.test.jstest/lints-once.test.jstest/store.test.jstest/typescript/cancel.test.jstest/typescript/fixtures/cancel/index.jstest/typescript/fixtures/native-watch/index.jstest/typescript/mock/typescript-counter/index.jstest/typescript/mock/typescript-stall/index.jstest/typescript/native.test.jstest/typescript/share.test.jstypes/check.d.tstypes/checks/biome.d.tstypes/checks/index.d.tstypes/checks/oxlint.d.tstypes/checks/stylelint.check.d.tstypes/checks/typescript-native.d.tstypes/checks/typescript-program.d.tstypes/checks/typescript-worker.d.tstypes/checks/typescript.d.tstypes/compiler-state.d.tstypes/import-graph.d.tstypes/options.d.tstypes/threads.d.ts
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
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:
e6f32033-e51d-4c01-a042-55d72da834b6
📒 Files selected for processing: 2
src/checks/typescript-native.jstest/typescript/cancel.test.js
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 0 remain after this review.
…d 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
…ot 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
alexander-akait
changed the title
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to GitHub limitations.
compiler-state.js:110-116
src/compiler-state.js:110-116
🩺 Stability & Availability | 🟡 Minor | ⚡ Quick winWait for the active native turn before closing the API.
A detached advisory TypeScript check can still be awaiting diagnostics when
watchClosereleases the last shared client.stopNative()disposes the snapshot without waiting fornative.turn, so the active check can use a disposed snapshot and lose its result.Suggested fix
async function stopNative(native) { + await native.turn; + if (native.snapshot) await native.snapshot.dispose(); await native.api.close(); }
-
-
- 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:
2db36893-6d78-42ec-9380-d62309aa5834
-
package-lock.jsonis excluded by!**/package-lock.json
-
.changeset/ts-share-program-across-compilers.md.changeset/typescript-7.mdREADME.mdpackage.jsonsrc/check.jssrc/checks/biome.jssrc/checks/oxlint.jssrc/checks/typescript-native.jssrc/checks/typescript-program.jssrc/checks/typescript-worker.jssrc/checks/typescript.jstest/import-graph.test.jstest/typescript/mock/typescript-version-only/index.jstest/typescript/native.test.jstypes/check.d.tstypes/checks/typescript-native.d.tstypes/checks/typescript-program.d.ts
-
- types/check.d.ts
-
- src/checks/oxlint.js
- .changeset/ts-share-program-across-compilers.md
- src/checks/biome.js
- .changeset/typescript-7.md
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
-
… 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
One PR in place of #371, #384, #387, #389, #390 and #391, which are closed.
Core: one place for what lives as long as a compiler
src/compiler-state.jsreplaces the per-compilerWeakMaps and the lifecycle code each check carried on its own:keptFor(compiler, key, create)holds check results, detached reports, the import graph and the TypeScript program state.acquire(compiler, key, start, stop)/release(compiler, key), a worker or process is:MultiCompileris one call rather than something each check implements.The checker API is unchanged. Nothing is added to it, and the options are unchanged too.
TypeScript 7
typescript/async, from 7.1.createSnapshotand kept up to date withsnapshot.update({ fileNotifications, ensurePrograms })from the files webpack saw change.formatDiagnosticsWithColorAndContext. The output is byte-for-byte what the TypeScript 6 check prints for the same project.unstable/*preview, with none of these calls. It is not used. A TypeScript offering no API to check with now says so, rather than failing on a missing function.Other changes
lintsOnceadapter hook. Child CPU went from 2.9 → 2.2 s for Biome and from 1.2–1.7 → 0.3 s for oxlint.Removed after measuring again on this branch
lintDependentsAfterBuild(from feat: add lintDependentsAfterBuild #371): it made webpack's build finish sooner. It did not make an error in an importer appear sooner: 1.96–2.64 s with the option against 2.09–2.21 s without it. What it hid is a gap in the existing import-graph pruning: a file's exported shape is only known after its first change, so the first edit to a file re-lints everything importing it. The second edit to the same file re-lints one file, in 257 ms. Closing that gap is a separate, follow-up change.cancel()on the checker API (from perf: stop a TypeScript run nothing will report once the next rebuild starts #389): it shortened back-to-back full rechecks from 4.4–5.5 s to 2.7–2.8 s. That only applied to a TypeScript 6 check on a worker reporting as advisory, and TypeScript 7 does not use it. It was not worth a new adapter method, a shared stop flag per worker run, and bookkeeping of which runs other compilers joined.Tests
test/compiler-state.test.jsand the share and TypeScript 7 tests. The TypeScript 7 tests run againsttypescript@7.1.0-beta, installed for tests astypescript7; they include one for a TypeScript with no API at all.typescript7'stscbin replaces TypeScript 6's innode_modules/.bin, sobuild:typesandlint:typescall TypeScript 6 by path.🤖 Generated with Claude Code
https://claude.ai/code/session_01App18sYbe6Vf6HDCjCXbvf
Summary by CodeRabbit