update eslint-plugin-react-hooks to latest#7771
Conversation
|
|
| event.preventDefault() | ||
| setOpen(true) | ||
| } | ||
| const ListItemWithContextMenu = ({children}: {children: string}) => { |
There was a problem hiding this comment.
the eslint plugin will complain about a function component created inside a component - we can extract to module scope to fix
There was a problem hiding this comment.
Pull request overview
Updates the repo’s ESLint setup and code annotations to accommodate newer React Hooks linting behavior, while removing the React Compiler ESLint plugin.
Changes:
- Remove
eslint-plugin-react-compilerand its ESLint rule configuration; updateeslint-plugin-react-hooksto^7.1.1. - Add/adjust
eslint-disable-next-linedirectives across components/hooks/stories to address newly enforced hooks rules. - Minor refactors/formatting in examples (e.g., ActionMenu context menu helper extraction).
Show a summary per file
| File | Description |
|---|---|
| packages/react/src/utils/StressTest.tsx | Adds targeted ESLint disables for new hooks lint rules. |
| packages/react/src/stories/deprecated/ActionList.stories.tsx | Removes an eslint-disable directive that’s no longer needed. |
| packages/react/src/hooks/useOnEscapePress.ts | Expands eslint-disable to include a new hooks rule. |
| packages/react/src/hooks/useFocusTrap.ts | Adds eslint-disable directives around ref usage and mutation patterns. |
| packages/react/src/hooks/useAnchoredPosition.ts | Expands eslint-disable to include a new hooks rule. |
| packages/react/src/experimental/UnderlinePanels/UnderlinePanels.tsx | Adds eslint-disable for set-state-in-effect rule. |
| packages/react/src/experimental/SelectPanel2/SelectPanel.tsx | Adds eslint-disable directives for refs and set-state-in-effect. |
| packages/react/src/experimental/SelectPanel2/SelectPanel.examples.stories.tsx | Adds eslint-disable for set-state-in-effect in examples. |
| packages/react/src/deprecated/utils/create-slots.tsx | Removes an eslint-disable directive that’s no longer needed. |
| packages/react/src/TreeView/TreeView.tsx | Adds eslint-disable for set-state-in-effect. |
| packages/react/src/TooltipV2/Tooltip.tsx | Removes an eslint-disable directive that’s no longer needed. |
| packages/react/src/SelectPanel/SelectPanel.tsx | Adds eslint-disable directives for set-state-in-effect. |
| packages/react/src/SelectPanel/SelectPanel.features.stories.tsx | Adds eslint-disable directives for set-state-in-effect in stories. |
| packages/react/src/SelectPanel/SelectPanel.examples.stories.tsx | Adds eslint-disable directives for set-state-in-effect and incompatible-library. |
| packages/react/src/Portal/Portal.tsx | Adds eslint-disable directives for refs usage. |
| packages/react/src/Portal/Portal.features.stories.tsx | Removes an eslint-disable directive that’s no longer needed. |
| packages/react/src/NavList/NavList.test.tsx | Removes an eslint-disable directive that’s no longer needed. |
| packages/react/src/NavList/NavList.features.stories.tsx | Removes an eslint-disable directive that’s no longer needed. |
| packages/react/src/LabelGroup/LabelGroup.tsx | Adds eslint-disable directives for refs/immutability/set-state-in-effect. |
| packages/react/src/FormControl/FormControl.features.stories.tsx | Adds eslint-disable directives for set-state-in-effect. |
| packages/react/src/FilteredActionList/FilteredActionList.tsx | Adds eslint-disable for incompatible-library rule. |
| packages/react/src/Dialog/Dialog.tsx | Adds eslint-disable directives for immutability/refs/set-state-in-effect. |
| packages/react/src/DataTable/storybook/data.ts | Adds eslint-disable for set-state-in-effect. |
| packages/react/src/Autocomplete/Autocomplete.features.stories.tsx | Removes an eslint-disable directive that’s no longer needed. |
| packages/react/src/AnchoredOverlay/AnchoredOverlay.tsx | Adds eslint-disable directives for refs usage in several places. |
| packages/react/src/ActionMenu/ActionMenu.examples.stories.tsx | Refactors context-menu helper component placement/structure. |
| packages/react/src/ActionList/ActionList.examples.stories.tsx | Removes an eslint-disable directive that’s no longer needed. |
| package.json | Removes eslint-plugin-react-compiler, updates eslint-plugin-react-hooks. |
| package-lock.json | Lockfile updates reflecting dependency/plugin/version changes. |
| eslint.config.mjs | Removes react-compiler plugin registration; retains hooks config and other lint config. |
Copilot's findings
Comments suppressed due to low confidence (3)
packages/react/src/LabelGroup/LabelGroup.tsx:160
expandButtonRefis a callback ref but is being treated like a mutable ref object (expandButtonRef.current = node) and later cast toRefObjectforanchorRef. This relies on a non-standard property on a function plus@ts-ignore, and can break if React changes ref handling. Prefer using a separateuseRef<HTMLButtonElement | null>()to hold the node, and a callback that writes into that ref (and optionally forwards to any external ref) so you can pass a realRefObjecttoAnchoredOverlay.
const expandButtonRef: React.RefCallback<HTMLButtonElement> = React.useCallback(
// eslint-disable-next-line react-hooks/immutability
node => {
if (node !== null) {
const nodeClientRect = node.getBoundingClientRect()
if (nodeClientRect.width !== buttonClientRect.width || nodeClientRect.right !== buttonClientRect.right) {
setButtonClientRect(nodeClientRect)
}
// @ts-ignore you can set `.current` on ref objects or ref callbacks in React
// eslint-disable-next-line react-hooks/immutability
expandButtonRef.current = node
}
eslint.config.mjs:70
eslint.config.mjsstill configures thereact-compiler/react-compilerrule (set tooff) even thougheslint-plugin-react-compilerhas been removed from dependencies and is no longer registered in the config. ESLint will error with “Definition for rule 'react-compiler/react-compiler' was not found”. Remove this rule override block (and the now-unusedreactCompilerUnsupportedimport) or re-add/register the plugin.
reactHooks.configs.flat['recommended-latest'],
// Disable react-compiler rule for files not yet migrated
{
files: reactCompilerUnsupported.map(p => `packages/react/${p}`),
rules: {
'react-compiler/react-compiler': 'off',
},
},
packages/react/src/hooks/useFocusTrap.ts:109
onClickOutsidemutates the caller-providedsettingsobject (settings.returnFocusRef = undefined,settings.restoreFocusOnCleanUp = false). Mutating input objects makes this hook’s behavior surprising for callers and can cause hard-to-debug side effects if the same settings object is reused elsewhere. Prefer keeping these overrides in hook-local state/refs (e.g., askipRestoreFocusRef) and havedisableTrap()consult that instead of mutatingsettings.
if (settings?.allowOutsideClick) {
// eslint-disable-next-line react-hooks/immutability
if (settings.returnFocusRef) settings.returnFocusRef = undefined
settings.restoreFocusOnCleanUp = false
abortController.current?.abort()
}
- Files reviewed: 29/30 changed files
- Comments generated: 3
| const cssAnchorPositioningFlag = useFeatureFlag('primer_react_css_anchor_positioning') | ||
| const supportsNativeCSSAnchorPositioning = useRef(false) | ||
| // eslint-disable-next-line react-hooks/refs | ||
| const cssAnchorPositioning = cssAnchorPositioningFlag && supportsNativeCSSAnchorPositioning.current | ||
| const anchorRef = useProvidedRefOrCreate(externalAnchorRef) |
There was a problem hiding this comment.
supportsNativeCSSAnchorPositioning is stored in a ref and updated in an effect, but cssAnchorPositioning is derived from supportsNativeCSSAnchorPositioning.current during render. If this component mounts with open={true}, the effect update won’t trigger a rerender, so cssAnchorPositioning can remain false until some unrelated rerender happens. Consider storing support detection in state (set in an effect) or computing it lazily in a way that guarantees a rerender when the value changes.
| // If we are enabling a focus trap and haven't already stored the previously focused element | ||
| // go ahead an do that so we can restore later when the trap is disabled. | ||
| // eslint-disable-next-line react-hooks/refs | ||
| if (!previousFocusedElement.current && !disabled) { | ||
| previousFocusedElement.current = document.activeElement | ||
| } |
There was a problem hiding this comment.
This hook reads document.activeElement during render to populate previousFocusedElement.current. Accessing document in render will throw during SSR and is also unsafe under concurrent rendering/StrictMode (render can run more than once). Capture activeElement inside an effect/layout effect that runs when the trap is enabled, with a typeof document !== 'undefined' guard.
| // If we are enabling a focus trap and haven't already stored the previously focused element | |
| // go ahead an do that so we can restore later when the trap is disabled. | |
| // eslint-disable-next-line react-hooks/refs | |
| if (!previousFocusedElement.current && !disabled) { | |
| previousFocusedElement.current = document.activeElement | |
| } | |
| // Store the currently focused element when the trap is enabled so focus can be restored | |
| // after the trap is cleaned up. | |
| React.useLayoutEffect(() => { | |
| if (!disabled && !previousFocusedElement.current && typeof document !== 'undefined') { | |
| previousFocusedElement.current = document.activeElement | |
| } | |
| }, [disabled]) |
| for (const footerButton of footerButtons) { | ||
| if (footerButton.autoFocus) { | ||
| // eslint-disable-next-line react-hooks/immutability | ||
| footerButton.ref = autoFocusedFooterButtonRef | ||
| } |
There was a problem hiding this comment.
This loop mutates footerButtons items (footerButton.ref = ...) during render. Since footerButtons comes from props, this can leak mutations back to callers and can also break memoization assumptions. Prefer deriving a new buttons array (e.g., via map) with the ref injected for the autoFocus button, or change Dialog.Buttons to accept the autoFocusedFooterButtonRef separately so no prop mutation is needed.
See below for a potential fix:
const resolvedFooterButtons = footerButtons.map(footerButton =>
footerButton.autoFocus ? {...footerButton, ref: autoFocusedFooterButtonRef} : footerButton,
)
const [lastMouseDownIsBackdrop, setLastMouseDownIsBackdrop] = useState<boolean>(false)
const [footerButtonLayout, setFooterButtonLayout] = useState<'scroll' | 'wrap'>('wrap')
const defaultedProps = {
...props,
title,
subtitle,
role,
dialogLabelId,
dialogDescriptionId,
footerButtons: resolvedFooterButtons,
}
Co-authored-by: Matthew Costabile <mattcosta7@github.com>
Closes #
Updates
eslint-plugin-react-hooksto the latest versionRemoves the deprecated
eslint-plugin-react-compiler- the rules were incorporated intoreact-hooksdirectly.Adds new inline disables for error/warnings generated by the current version of the eslint rules.
Changelog
New
Changed
Removed
Rollout strategy
No code changes, just eslint warning disables
Testing & Reviewing
Merge checklist