Skip to content

chore(lint): migrate to eslint-plugin-react-hooks 7.1 (set-state-in-effect) — held at ~7.0.1 #1897

Description

@cliffhall

Discovered while doing #1839 (the vitest/Storybook exact-pin untangle).

Regenerating clients/web/package-lock.json to untie the @vitest/browser peer knot floated eslint-plugin-react-hooks 7.0.17.1.1, which enables the new react-hooks/set-state-in-effect rule. That rule fails npm run lint on 8 pre-existing violations — none of them introduced by that PR:

File Line
components/groups/MrtrConversation/MrtrConversation.tsx 135
components/groups/NetworkEntry/NetworkEntry.tsx 456, 469
components/groups/PromptArgumentsForm/PromptArgumentsForm.tsx 71
components/groups/ProtocolEntry/ProtocolEntry.tsx 300
components/groups/ResourceTemplatePanel/ResourceTemplatePanel.tsx 122
components/groups/ServerConfigModal/ServerConfigModal.tsx 215
components/groups/TaskCard/TaskCard.tsx 120

Six of the eight are the same shape — useEffect(() => setIsExpanded(isListExpanded), [isListExpanded]), a prop mirrored into local state so a list-wide expand/collapse control can drive a row that also has its own toggle. The remaining two are reset-on-identity-change (PromptArgumentsForm clearing completions when the prompt changes, ResourceTemplatePanel clearing variables when the template changes) and one modal-open reset (ServerConfigModal).

The hold

clients/web/package.json now declares "eslint-plugin-react-hooks": "~7.0.1" so 7.1.x cannot be picked up. This is a deliberate, temporary hold to keep a security bump from carrying an unrelated lint migration — it needs removing.

What to do

The rule is pointing at something real, so the fix is a refactor, not a suppression. React's own guidance for the dominant shape is to derive during render rather than sync in an effect — either a key to remount the row on the list-wide toggle, or the "adjust state while rendering" pattern that tracks the previous prop value. The reset-on-identity-change cases are the same story with a different trigger.

Each site changes real interaction behavior (a row's own toggle must still win over the list-wide one until the list-wide value next changes), so every one needs its existing tests kept green and, where the current tests do not already pin that precedence, a new one that does. The ≥90% per-file gate applies as usual.

Do not disable the rule to clear the errors — the whole point of taking 7.1 is the new coverage.

Metadata

Metadata

Assignees

No one assigned

    Labels

    v2Issues and PRs for v2

    Type

    No type

    Projects

    No projects

    Milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions