You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
{{ message }}
Repository navigation
perf: skip importers on a file's first edit too, and in projects with allowJs - #393
While webpack watches, a check that reads across files (typed linting, for example) only re-lints a changed file's importers when what the file shows them changes: its declarations and its imports. That had two gaps. This PR closes both, with no new option.
1. The first edit of a file re-linted every importer
The comparison needs what the file showed before the edit. That was only recorded after the file's first change, so the first edit of every file in a session fell back to re-linting everything that imports it.
Now, once the first build of a watch is over, the plugin keeps the text of every file such a check covers. A file's first edit is compared against that text.
edit the body of mod150.ts (301 files, typed ESLint, 150 importers)
before: 1st edit → 151 files re-linted, rebuild 2.0–2.2 s; 2nd edit → 1 file, ~0.26 s
after: 1st edit → 1 file re-linted, rebuild 0.27–0.35 s
An edit that does change an export still re-lints every importer and reports the error, as before.
Safety: a file written after webpack started reading the file system for that build (compiler.fsStartTime) is not kept, because what's on disk may already be the change. Its first edit re-lints every importer, as today.
Timing: the texts are read in the background after the first build. The first rebuild waits for that reading to finish, a few milliseconds.
Cost: memory is about the size of the source files never edited, around 130 KB for the 301 files above. Nothing extra runs up front. Working out every file's exported shape at startup would have cost about 600 ms per watch start, so this compares only the edited file, when it changes.
2. In a project with allowJs, no TypeScript file was ever compared
A file's exported shape was read with transpileDeclaration under the project's own compiler options. With allowJs, or a rootDir a file sits outside of, TypeScript refuses to read the file at all:
Option 'allowJs' cannot be specified with option 'isolatedDeclarations'.
In any project with allowJs set, which is common for mixed JS/TS code, every edit to any file re-linted all its importers.
The fix: the shape is now read with no project options. The old and new versions of a file are read the same way, so the comparison holds; removeComments: false and stripInternal: false still keep what an importer's rules read. .tsx files read the same with or without jsx.
JavaScript files are excluded explicitly, using the extensions TypeScript reports. For a JS export that needs a type checker, transpileDeclaration writes any with no error, which would hide real changes. Before, the allowJs in the options only excluded them by accident.
Tests
test/import-graph.test.js: a first edit after the texts are kept:
an edit to the body only leaves importers alone;
an edit that changes an export reaches them;
a file with declare global reaches every file;
a file written after the build started is not trusted;
a kept text is used only once;
a deleted file is forgotten.
Plus a project with allowJs and a rootDir.
test/incremental.test.js: the first body-only edit to a .ts file in watch mode re-lints only that file. It fails on main.
Locally: lint and the full suite (363 tests) pass.
Watch-mode rebuilds now avoid rechecking unchanged importers after body- or comment-only edits.
Changes to exported types still trigger checks in affected importers. Updates also account for JavaScript files and files outside the configured TypeScript root.
Documentation
Clarified how changes to watched files affect importers, including files first read during a build.
Updated rebuild timing information to include the first edit.
…ad TypeScript files in an allowJs project
The importers of a file changed while webpack watches are linted again
only when what it shows them changes, but that could be told only from a
file's second change on: what it showed before its first was never read.
What each file a check reading across files covers holds is now kept once
the first build of a watch is over, and its first change is told from it.
A file written after webpack started reading the file system for that
build is not kept, and its first change reaches every importer, as before.
What a file shows was read under the project's own compiler options, so
in a project setting allowJs, or a rootDir a file is outside of,
TypeScript refused to read any file and every edit reached every
importer. It is now read under no project option, and only for
TypeScript files: a JavaScript file's exports are written as any without
a word, so a change to them would not show.
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:
Co-authored-by: name <email>
Supported Co-authored-by: formats include:
Anything <id+login@users.noreply.github.com> - it will locate your GitHub user by id part.
Anything <login@users.noreply.github.com> - it will locate your GitHub user by login part.
Anything <public-email> - it will locate your GitHub user by public-email part. Note that this email must be made public on Github.
Anything <other-email> - it will locate your GitHub user by other-email part but only if that email was used before for any other CLA as a main commit author.
login <any-valid-email> - it will locate your GitHub user by login part, note that login part must be at least 3 characters long.
Alternatively, if the co-author should not be included, remove the Co-authored-by: line from the commit message.
Please update your commit message(s) by doing git commit --amend and then git push [--force] and then request re-running CLA check via commenting on this pull request:
No actionable comments were generated in the recent review.
Recent 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: ae8f30a4-7f0e-4f03-90df-72497559a7c1
Commits
Reviewing files that changed from the base of the PR and between 812f6d2 and d0dde41.
Files selected for processing: 2
src/import-graph.js
test/import-graph.test.js
Files skipped from review as they are similar to previous changes: 1
test/import-graph.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.
Walkthrough
The import graph now retains stable file text from before the first watch change and uses it to compare prior and current imports and declaration faces. The plugin captures this state after the initial watch build and waits for capture when calculating affected files. Tests cover first-edit behavior, including body-only edits, exported-type changes, deleted files, and JavaScript settings. Documentation and the changeset describe the updated behavior and rebuild timings.
Priority: Low
Merge Risk: Low · up to d0dde
On filesystems with coarse timestamps, edits during the initial build can leave importer diagnostics stale. The timing-dependent risk is bounded.
Pre-merge checks: 5
Check name
Status
Explanation
Description Check
Check skipped - CodeRabbit’s high-level summary is enabled.
Title check
The title clearly and concisely describes the main changes: skipping importer re-linting on a file’s first edit and supporting projects with allowJs.
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 with cyclomatic complexity of at least 2. Documentat…
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 with cyclomatic complexity of at least 2. Documentation-only edits and deletions within surviving functions are included; unchanged diff context and fully deleted functions are excluded. Analyzed 5 functions across 4 files.
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
Warning
Some tools did not complete. Review the errors below.
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.
ESLint install timed out. The project may have too many dependencies for the sandbox.
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.
The reason will be displayed to describe this comment to others. Learn more.
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: d224e73b-c44c-4e06-b229-3eb400b523d6
Commits
Reviewing files that changed from the base of the PR and between d91d244 and 812f6d2.
Files selected for processing: 6
.changeset/first-edit-face.md
README.md
src/import-graph.js
src/index.js
test/import-graph.test.js
test/incremental.test.js
Included review availability: This review used your included allowance. Your plan provides up to 2 included reviews per hour; 1 remain after this review.
A rebuild that lints every file, because a file that changed declares
something global or a config file changed, stopped before reading the
other files that changed, so what each of them held or showed before
stayed in the graph. An edit putting one of them back as it was then left
its importers be, though they were last linted against what it held in
between. What the files that changed held is now forgotten on both paths.
Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01App18sYbe6Vf6HDCjCXbvf
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
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.
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
While webpack watches, a check that reads across files (typed linting, for example) only re-lints a changed file's importers when what the file shows them changes: its declarations and its imports. That had two gaps. This PR closes both, with no new option.
1. The first edit of a file re-linted every importer
The comparison needs what the file showed before the edit. That was only recorded after the file's first change, so the first edit of every file in a session fell back to re-linting everything that imports it.
Now, once the first build of a watch is over, the plugin keeps the text of every file such a check covers. A file's first edit is compared against that text.
An edit that does change an export still re-lints every importer and reports the error, as before.
compiler.fsStartTime) is not kept, because what's on disk may already be the change. Its first edit re-lints every importer, as today.2. In a project with
allowJs, no TypeScript file was ever comparedA file's exported shape was read with
transpileDeclarationunder the project's own compiler options. WithallowJs, or arootDira file sits outside of, TypeScript refuses to read the file at all:In any project with
allowJsset, which is common for mixed JS/TS code, every edit to any file re-linted all its importers.removeComments: falseandstripInternal: falsestill keep what an importer's rules read..tsxfiles read the same with or withoutjsx.transpileDeclarationwritesanywith no error, which would hide real changes. Before, theallowJsin the options only excluded them by accident.Tests
test/import-graph.test.js: a first edit after the texts are kept:declare globalreaches every file;Plus a project with
allowJsand arootDir.test/incremental.test.js: the first body-only edit to a.tsfile in watch mode re-lints only that file. It fails onmain.Locally: lint and the full suite (363 tests) pass.
Generated by Claude Code
Summary by CodeRabbit