[Frontend] Adopt the React Compiler lint rules from eslint-plugin-react-hooks v7 #182

Closed
opened 2026-07-17 02:12:14 +00:00 by claude-bot · 2 comments
Contributor

Context

#21 wired up the real eslint-plugin-react-hooks (v7.1.1) and enabled exactly the two rules that issue asked for:

"react-hooks/rules-of-hooks": "error",
"react-hooks/exhaustive-deps": "warn",

It deliberately did not spread the plugin's recommended preset. In v7 that preset no longer means "the two classic hooks rules" — it enables 16, adding 14 React Compiler rules, mostly at error:

Group Rules
Rules of React during render purity, set-state-in-render, set-state-in-effect, immutability, refs, globals
Component / memoization shape static-components, use-memo, preserve-manual-memoization
Compiler-compat guards error-boundaries, incompatible-library, unsupported-syntax, config, gating

(recommended-latest is 17 — it also adds void-use-memo.)

These exist so the React Compiler can safely auto-memoize components: it must be able to prove the code follows the Rules of React. Adopting them is a step toward enabling the compiler, not general lint hygiene — which is why it was scoped out of #21 rather than smuggled in.

The rationale is recorded in webapp/frontend/eslint.config.js.

Actual scope (measured, not estimated)

Ran the full v7 recommended preset against main (4328226, i.e. after #21 and #103) as a throwaway probe config. It is much smaller than expected — 12 findings, all errors:

 11  react-hooks/set-state-in-effect
  1  react-hooks/preserve-manual-memoization

By file:

  4  WikiArticle.jsx
  2  Admin.jsx
  2  CampaignPlanning.jsx
  1  CampaignDetail.jsx
  1  CampaignStoryline.jsx
  1  WikiDraftReview.jsx
  1  WikiProposals.jsx

The other 12 compiler rules report zero violations — the codebase is already broadly compliant.

Why this is still non-trivial despite being only 12

set-state-in-effect is the bulk of it, and it is rarely a one-line fix. Calling setState inside an effect usually signals state that should be derived during render, lifted, or keyed instead — so each site needs a judgement call about the intended data flow, and a careless "fix" can introduce a render loop or change behaviour. Expect real review per site, not a mechanical pass.

preserve-manual-memoization (1 site) flags a useMemo/useCallback whose memoization the compiler couldn't preserve.

Proposed approach

  1. Fix the 12 sites first, with the rules off, so each change stands on its own and is reviewable.
  2. Then flip eslint.config.js from the two explicit rules to spreading reactHooks.configs.recommended (or recommended-latest), and drop the explanatory comment.
  3. Keep exhaustive-deps at warn and the rest at their preset severities.
  4. Only then consider actually turning the React Compiler on — that is a separate decision with its own build/runtime implications, and this issue is the prerequisite, not that decision.

Acceptance criteria

  • All 12 findings resolved, each with a deliberate fix (not an eslint-disable).
  • eslint.config.js uses the plugin's recommended preset rather than listing rules explicitly.
  • npm run lint clean (0 errors, 0 warnings).
  • Frontend test suite still green, with no behaviour change intended — call out explicitly in the PR if any set-state-in-effect fix does change behaviour.

Reproducing the measurement

The probe config used (not committed):

import js from "@eslint/js";
import globals from "globals";
import reactHooks from "eslint-plugin-react-hooks";
export default [{
  files: ["src/**/*.{js,jsx}"],
  plugins: { "react-hooks": reactHooks },
  languageOptions: {
    ecmaVersion: "latest", sourceType: "module",
    parserOptions: { ecmaFeatures: { jsx: true } },
    globals: { ...globals.browser, ...globals.node },
  },
  rules: { ...js.configs.recommended.rules, ...reactHooks.configs.recommended.rules },
}];
npx eslint src -c eslint.probe.mjs

Filed as a follow-up to #21 (v3.5.0 — Frontend Platform & PWA).

## Context #21 wired up the real `eslint-plugin-react-hooks` (v7.1.1) and enabled exactly the two rules that issue asked for: ```js "react-hooks/rules-of-hooks": "error", "react-hooks/exhaustive-deps": "warn", ``` It deliberately did **not** spread the plugin's `recommended` preset. In v7 that preset no longer means "the two classic hooks rules" — it enables **16**, adding 14 **React Compiler** rules, mostly at `error`: | Group | Rules | |---|---| | Rules of React during render | `purity`, `set-state-in-render`, `set-state-in-effect`, `immutability`, `refs`, `globals` | | Component / memoization shape | `static-components`, `use-memo`, `preserve-manual-memoization` | | Compiler-compat guards | `error-boundaries`, `incompatible-library`, `unsupported-syntax`, `config`, `gating` | (`recommended-latest` is 17 — it also adds `void-use-memo`.) These exist so the **React Compiler** can safely auto-memoize components: it must be able to prove the code follows the Rules of React. Adopting them is a step toward enabling the compiler, not general lint hygiene — which is why it was scoped out of #21 rather than smuggled in. The rationale is recorded in `webapp/frontend/eslint.config.js`. ## Actual scope (measured, not estimated) Ran the full v7 `recommended` preset against `main` (`4328226`, i.e. after #21 and #103) as a throwaway probe config. It is **much smaller than expected — 12 findings, all errors**: ``` 11 react-hooks/set-state-in-effect 1 react-hooks/preserve-manual-memoization ``` By file: ``` 4 WikiArticle.jsx 2 Admin.jsx 2 CampaignPlanning.jsx 1 CampaignDetail.jsx 1 CampaignStoryline.jsx 1 WikiDraftReview.jsx 1 WikiProposals.jsx ``` The other 12 compiler rules report **zero** violations — the codebase is already broadly compliant. ## Why this is still non-trivial despite being only 12 `set-state-in-effect` is the bulk of it, and it is rarely a one-line fix. Calling `setState` inside an effect usually signals state that should be *derived during render*, lifted, or keyed instead — so each site needs a judgement call about the intended data flow, and a careless "fix" can introduce a render loop or change behaviour. Expect real review per site, not a mechanical pass. `preserve-manual-memoization` (1 site) flags a `useMemo`/`useCallback` whose memoization the compiler couldn't preserve. ## Proposed approach 1. Fix the 12 sites first, with the rules off, so each change stands on its own and is reviewable. 2. Then flip `eslint.config.js` from the two explicit rules to spreading `reactHooks.configs.recommended` (or `recommended-latest`), and drop the explanatory comment. 3. Keep `exhaustive-deps` at `warn` and the rest at their preset severities. 4. **Only then** consider actually turning the React Compiler on — that is a separate decision with its own build/runtime implications, and this issue is the prerequisite, not that decision. ## Acceptance criteria - [ ] All 12 findings resolved, each with a deliberate fix (not an `eslint-disable`). - [ ] `eslint.config.js` uses the plugin's `recommended` preset rather than listing rules explicitly. - [ ] `npm run lint` clean (0 errors, 0 warnings). - [ ] Frontend test suite still green, with no behaviour change intended — call out explicitly in the PR if any `set-state-in-effect` fix does change behaviour. ## Reproducing the measurement The probe config used (not committed): ```js import js from "@eslint/js"; import globals from "globals"; import reactHooks from "eslint-plugin-react-hooks"; export default [{ files: ["src/**/*.{js,jsx}"], plugins: { "react-hooks": reactHooks }, languageOptions: { ecmaVersion: "latest", sourceType: "module", parserOptions: { ecmaFeatures: { jsx: true } }, globals: { ...globals.browser, ...globals.node }, }, rules: { ...js.configs.recommended.rules, ...reactHooks.configs.recommended.rules }, }]; ``` ``` npx eslint src -c eslint.probe.mjs ``` _Filed as a follow-up to #21 (v3.5.0 — Frontend Platform & PWA)._
Author
Contributor

Picking this up alongside #464 as a v4.4.0 engineering lane. The rules should be on before the rebuild writes new components, so that new code is compliant from the start rather than swept afterwards. The July measurement (12 sites) predates v4.3.0's changes to most of the named files, so the first step is to re-run the probe config and work from the current count. Each site gets a deliberate fix, per the body; the preset flip comes last.

Picking this up alongside #464 as a v4.4.0 engineering lane. The rules should be on before the rebuild writes new components, so that new code is compliant from the start rather than swept afterwards. The July measurement (12 sites) predates v4.3.0's changes to most of the named files, so the first step is to re-run the probe config and work from the current count. Each site gets a deliberate fix, per the body; the preset flip comes last.
Author
Contributor

Done; pull request to follow this comment (auto-merge on green CI).

The re-measurement mattered: 40 findings across 22 files, not the 12 from July. Every one is fixed by restructuring, none suppressed. The dominant pattern was a loading or stale flag an effect raised on its way past, now a value derived during render from what the data in hand was fetched for. The rule also turned up two genuine hazards beyond hygiene: ElapsedTimer read the clock during render (a re-render would move its origin), and the Shelf's onPointerUp removed itself by a binding that was not yet initialised, so the listener it detached was whichever the closure captured; both are rewritten.

Config: recommended-latest (a strict superset of recommended in v7), exhaustive-deps kept at warn, comment rewritten. The React Compiler itself stays a separate decision, as the body says.

One user-visible improvement rode along with its own changelog entry (the stat-block autosave indicator now keeps "Saved"); its twin on the body autosave is #539 for the rebuild's wiki lane. Two follow-ups filed from the phase 0 lanes overall: #536 and #539.

Done; pull request to follow this comment (auto-merge on green CI). The re-measurement mattered: **40 findings across 22 files**, not the 12 from July. Every one is fixed by restructuring, none suppressed. The dominant pattern was a `loading` or `stale` flag an effect raised on its way past, now a value derived during render from what the data in hand was fetched for. The rule also turned up two genuine hazards beyond hygiene: `ElapsedTimer` read the clock during render (a re-render would move its origin), and the Shelf's `onPointerUp` removed itself by a binding that was not yet initialised, so the listener it detached was whichever the closure captured; both are rewritten. Config: `recommended-latest` (a strict superset of `recommended` in v7), `exhaustive-deps` kept at `warn`, comment rewritten. The React Compiler itself stays a separate decision, as the body says. One user-visible improvement rode along with its own changelog entry (the stat-block autosave indicator now keeps "Saved"); its twin on the body autosave is #539 for the rebuild's wiki lane. Two follow-ups filed from the phase 0 lanes overall: #536 and #539.
Sign in to join this conversation.
No project
No assignees
1 participant
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rbrooks/Quest-Board#182
No description provided.