chore: wire up eslint with the React hooks rules - #81
Closed
matej21 wants to merge 1 commit into
Closed
Conversation
The repo had no eslint config at all, while the source carried `// eslint-disable-next-line react-hooks/rules-of-hooks` comments that nothing has ever enforced. One of them sat over a `useField` call inside a loop in <Switch>, under a comment asserting a "stable count" that was false; it crashes React as soon as a <Case> is conditionally rendered. Enable exactly two rules over packages/*/src and tests: rules-of-hooks as an error, exhaustive-deps as a warning. eslint-plugin-react-hooks v7 ships 28 rules (the React Compiler set); none of the others are turned on and no style preset is added, so the signal stays readable in a repo with no lint history. No CI job is added. `bun run lint` exits 1 today: 9 rules-of-hooks sites (7 genuine hazards, 2 provably-stable false positives) and 25 phantom errors from dead `@typescript-eslint/*` disable directives naming rules no config defines. Turning the gate on has to wait for those. Flat config is named .mjs because the root package.json has no "type": "module".
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.
Why
The repo has no eslint config. None — no
.eslintrc*, noeslint.config.*. Yet the source is sprinkled with// eslint-disable-next-line react-hooks/rules-of-hooks. Every one of those comments is decoration.That is not theoretical. One sat over a
useFieldcall inside aforloop in<Switch>, under a comment asserting a "stable count, stable order" that nothing enforced and that was false. It crashes React withRendered more hooks than during the previous renderthe moment a<Case>is conditionally rendered (fixed separately in #78). It shipped because no linter existed to contradict the comment.What this adds
eslint+eslint-plugin-react-hooks+@typescript-eslint/parser, a flat config, and alintscript. Two rules only:react-hooks/rules-of-hooksreact-hooks/exhaustive-depseslint-plugin-react-hooksv7 ships 28 rules (the React Compiler set —purity,set-state-in-effect,immutability, …). The plugin is registered but none of the others are enabled, and no style preset is added. A repo with no lint history does not need a thousand formatting complaints burying the signal.No CI job is added, deliberately — a gate that fails on day one would block every open PR. The step is written out at the bottom for when it is ready.
What it finds
Inline disables suppress reports, so there are two numbers:
bun run linteslint . --no-inline-configThe 9
rules-of-hookssitesbindx-react/src/jsx/components/Switch.tsx:147bindx-react/src/jsx/components/Field.tsx:31useFieldafterif (field == null) return null. The code's own comment names the toggling condition: "when accessing field on disconnected has-one relation"bindx-react/src/jsx/components/Attribute.tsx:41bindx-ui/src/datagrid/filters/common.tsx:11name ??= useDataViewFilterName();??=short-circuits, so the hook runs only when the prop is nullishbindx-ui/src/datagrid/filters/mobile.tsx:11??=bindx-ui/src/datagrid/column-header.tsx:39useDataViewFilter(filterName)insideif (filterName); toggling a column filter changes the hook countbindx-dataview/src/filterComponents.tsx:68useOptionalDataViewFilterName()inside a plain function called from ~16 component bodies, and only whenname === undefined. Notuse-prefixed and conditionalbindx-react/src/jsx/componentFactory.ts:81bindx-react/src/jsx/componentFactory.ts:1507 genuine hazards, 2 false positives. Note the inversion: the two sites carrying reassuring comments are the false positives; the six carrying bare disables are all real.
The caveat you will hit immediately
25 of the 28 default-run errors are not hooks findings — they are
Definition for rule … was not foundon existing@typescript-eslint/*disable comments (17 ×no-explicit-any, 6 ×ban-types, 1 ×no-empty-interface, 1 ×no-unused-vars). ESLint errors on any disable directive naming an undefined rule and offers no knob to downgrade it.This is the same disease, one plugin over: those comments are decoration too.
Installing
@typescript-eslint/eslint-pluginto define the namespace was tried and rejected — it converts 19 phantom errors into 19Unused eslint-disable directivewarnings and still leaves 6, becauseban-typeswas deleted in typescript-eslint v8. A dependency that buys nothing. The two real options are to delete the 25 dead directives or to accept the noise; both are follow-ups.Before the CI gate can go on
bun run lintmust exit 0. Blocking it today: the 25 dead directives, plus 3 realrules-of-hookserrors that are unsuppressed (componentFactory.ts:150,Field.tsx:31,Attribute.tsx:41) — two of which are genuine crash hazards. The other 6 stay hidden behind their disables and can be worked through afterwards.Gates
bun run typecheckexit 0 ·bun test --path-ignore-patterns='**/tests/browser/**'1737 pass / 0 fail / 149 files · no source file modified.Not done
No finding is fixed and no existing
eslint-disableis removed — several become load-bearing once the linter is real, and each is its own decision. Extending thefilesglob topackages/example(sources sit at the package root, notsrc/) was probed: 23 files, 0 problems. Free to add.