fix(linter/plugins): ensure after hooks always run#20167
Merged
graphite-app[bot] merged 1 commit intomainfrom Mar 10, 2026
Merged
fix(linter/plugins): ensure after hooks always run#20167graphite-app[bot] merged 1 commit intomainfrom
after hooks always run#20167graphite-app[bot] merged 1 commit intomainfrom
Conversation
Member
Author
How to use the Graphite Merge QueueAdd either label to this PR to merge it via the merge queue:
You must have a Graphite account in order to use the merge queue. Sign up using this link. An organization admin has enabled the Graphite Merge Queue in this repository. Please do not merge from GitHub as this will restart CI on PRs being processed by the merge queue. This stack of pull requests is managed by Graphite. Learn more about stacking. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where after hooks in createOnce rules could fail to run if an error was thrown during linting (e.g., in a later rule's before hook or during AST walk). Since after hooks are used to maintain state between files, it's critical they always run when their corresponding before hook has run. The fix extracts after-hook execution into a runAfterHooks helper that is called both at the end of normal linting and from resetStateAfterError. The helper also continues running all remaining after hooks even if one throws, capturing only the first error to re-throw.
Changes:
- Extracted after-hook execution into
runAfterHooks(shouldThrowIfError)inlint.ts, which runs all hooks and suppresses or re-throws errors based on the flag. - Added
runAfterHooks(false)call inresetStateAfterErrorso that after hooks run even when an error is thrown during abeforehook or AST walk. - Replaced the narrow
lint_before_hook_errorandlint_after_hook_errorfixtures with a comprehensivecreateOnce_hook_errorsfixture that covers all error scenarios across multiple files.
Reviewed changes
Copilot reviewed 15 out of 17 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
apps/oxlint/src-js/plugins/lint.ts |
Core fix: new runAfterHooks helper ensures after hooks always run; called from both the normal path and resetStateAfterError |
apps/oxlint/test/fixtures/createOnce_hook_errors/plugin.ts |
New comprehensive test plugin covering throw-in-before, throw-in-after, and throw-in-visit scenarios |
apps/oxlint/test/fixtures/createOnce_hook_errors/output.snap.md |
Expected output snapshot for the new fixture |
apps/oxlint/test/fixtures/createOnce_hook_errors/.oxlintrc.json |
Config enabling specific rules per subdirectory for each scenario |
apps/oxlint/test/fixtures/createOnce_hook_errors/options.json |
Single-thread option to ensure deterministic file ordering |
apps/oxlint/test/fixtures/createOnce_hook_errors/files/*/ |
JS test files (1.js triggers error, 2.js verifies state resets properly) |
apps/oxlint/test/fixtures/lint_before_hook_error/* |
Removed: superseded by the new comprehensive fixture |
apps/oxlint/test/fixtures/lint_after_hook_error/* |
Removed: superseded by the new comprehensive fixture |
69d2c89 to
02cc349
Compare
e2b9045 to
d867fd8
Compare
Member
Author
Merge activity
|
The `createOnce` API allows defining `before` and `after` hooks which run before / after each file. These can be used to track state. It is therefore important that if the `before` hook is called, then the `after` hook is too. It is possible for this invariant to be broken if an error is thrown during AST visitation. Because all rules run together in a single AST pass, an error in one rule can affect another. Previously in such cases, the `after` hook could fail to be called. Fix this by calling `after` hooks in the `try` / `catch` which wraps the whole linting process. The PR adds a comprehensive test fixture which tests behavior is correct in all circumstances, with errors being thrown at various stages, in the rule with the `after` hook, or in another rule. Remove the old `lint_before_hook_error` and `lint_after_hook_error` test fixtures as the new fixture covers the same ground.
02cc349 to
c4812ec
Compare
d867fd8 to
1340307
Compare
Base automatically changed from
om/03-09-fix_linter_plugins_reset_visitor_compilation_state_if_error_during_compilation
to
main
March 10, 2026 01:59
leaysgur
added a commit
that referenced
this pull request
Mar 11, 2026
# Oxlint ### 🚀 Features - 04a5ce0 oxlint: Support `vite.config.ts` `.lint` field (#20214) (leaysgur) - 1735215 linter: Implement `react/no-clone-element` rule. (#20129) (connorshea) - 68e6f6f linter: Implement `react/no-react-children` rule. (#20104) (connorshea) - fe3b32e linter/plugins: Add `oxlint-plugin-eslint` package (#20009) (overlookmotel) ### 🐛 Bug Fixes - 05f6a09 linter/no-inline-comments: Deserialize rule options with serde (#20207) (camc314) - c7eb09d linter/default-case: Deserialize rule options with serde (#20206) (camc314) - f85e16c linter/plugins: Fix types for visitor compilation (#20203) (overlookmotel) - 44e24e0 linter/exhaustive-deps: Ignore type-only typeof deps (#20201) (camc314) - 0b04998 linter/no-fallthrough: Deserialize rule options with serde (#20192) (camc314) - a1031cb linter/new-cap: Deserialize rule options with serde (#20161) (camc314) - ad27fd6 linter: Add help messages to import plugin diagnostics (#20158) (John Costa) - 1340307 linter/plugins: Ensure `after` hooks always run (#20167) (overlookmotel) - c4812ec linter/plugins: Reset visitor compilation state if error during compilation (#20166) (overlookmotel) - 887eecc linter/plugins: Add license notice to `oxlint-plugin-eslint` package (#20164) (overlookmotel) - e1713a4 linter/plugins: Include common chunks in `oxlint-plugin-eslint` package (#20163) (overlookmotel) - a9acb2b linter: Check `globals` entry for `no-undef`, only check es2026 globals for `no-extend-native` and `no-constant-binary-expression` (#20089) (Sysix) - 5559f0d linter/no-unused-vars: `reportUsedIgnorePattern` should not report used rest siblings (#20108) (Don Isaac) - de7c0e2 linter/plugins: Correct error message for `markVariableAsUsed` (#20152) (overlookmotel) ### ⚡ Performance - 3a86427 linter/plugins: Pre-populate cache of `EnterExit` objects at startup (#20194) (overlookmotel) - d243391 linter/plugins: Replace arrays with `Uint8Array`s (#20190) (overlookmotel) - 8742f8b linter/plugins: Pre-populate cache of `VisitProp` objects (#20189) (overlookmotel) - 3061acb linter/plugins: Pre-populate cache of `EnterExit` objects (#20187) (overlookmotel) - c73912b linter/plugins: Free visit functions earlier (#20186) (overlookmotel) - d9f8ff4 linter/plugins: Faster reset of visitor state (#20185) (overlookmotel) - 42aff15 oxlint/lsp: Avoid computing diagnostics for non invoked code actions requests (#20080) (Sysix) ### 📚 Documentation - a080650 linter/plugins: Fix documentation of visitor compilation (#20202) (overlookmotel) - 542a04a linter: Add a link to the cyclomatic complexity Wikipedia article in `eslint/complexity` (#20174) (connorshea) # Oxfmt ### 🚀 Features - 95943aa oxfmt: Support `vite.config.*` `.fmt` field (#20197) (leaysgur) - 172fc07 oxfmt: .js/.ts config file support (#20135) (leaysgur) ### 🐛 Bug Fixes - e483569 oxfmt: Avoid double-escaping in css-in-js (#20211) (leaysgur) Co-authored-by: leaysgur <6259812+leaysgur@users.noreply.github.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.
The
createOnceAPI allows definingbeforeandafterhooks which run before / after each file. These can be used to track state.It is therefore important that if the
beforehook is called, then theafterhook is too.It is possible for this invariant to be broken if an error is thrown during AST visitation. Because all rules run together in a single AST pass, an error in one rule can affect another. Previously in such cases, the
afterhook could fail to be called.Fix this by calling
afterhooks in thetry/catchwhich wraps the whole linting process.The PR adds a comprehensive test fixture which tests behavior is correct in all circumstances, with errors being thrown at various stages, in the rule with the
afterhook, or in another rule.Remove the old
lint_before_hook_errorandlint_after_hook_errortest fixtures as the new fixture covers the same ground.