fix: tell two entries of the same check apart - #351
Merged
Merged
Conversation
Everything a check is remembered by between compilations was keyed on the tool it runs — the incremental result store and the detached report in `check.js`, the watched files in `index.js`, the worker and the program in the typescript check. Two entries running the same tool therefore shared one of each, so a rebuild that linted neither of them handed the first entry whatever the second had stored: results it was told to ignore, report as a warning, or not report at all. Each entry now carries an id of its own, which is what all four are keyed on. `name` stays what a diagnostic is labelled with.
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
1 participant
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
Everything a check is remembered by between compilations was keyed on the tool it runs rather than on the entry that configured it: the incremental result store and the detached report (
check.js), the files handed to the watcher (index.js), and the worker and held program (typescript.js). Two entries of the same tool therefore shared one of each.What that looks like: two
eslintentries, one withignoreDiagnostics: ["no-var"]. Build 1 reports the error once, correctly. On a rebuild that lints neither file,remember()hands the ignoring entry the whole shared store — so the error is reported twice, once by the entry told to ignore it. The same sharing would leakreportAs,fixand a formatter across entries.Each entry now carries an id of its own (
<check>\0<index>), which is what all four are keyed on.namestays what a diagnostic is labelled with.What kind of change does this PR introduce?
fix.
Did you add tests for your changes?
Yes —
test/store.test.jscovers the seam directly (one entry's results stay out of another's report), andtest/multiple-checks.test.jsdrives the user-visible case through a real watch. Both fail against unmodifiedsrc/.Does this PR introduce a breaking change?
No.
If relevant, what needs to be documented once your changes are merged or what have you already documented?
n/a —
checksalready documents several entries of one tool; this makes it behave that way.Use of AI
AI was used. It found this while auditing the plugin for bugs, reproduced it as a two-build watch before touching anything, then wrote the fix and both tests and checked each fails with the fix reverted.
🤖 Generated with Claude Code
https://claude.ai/code/session_01GzZci4NQeiqwdrVfd7dGXy
Generated by Claude Code