chore: Update eslint plugin react hooks followup #9054

snowystinger · 2025-10-17T05:58:15Z

Closes

This PR focused on these rules
'react-hooks/globals': ERROR,
'react-hooks/purity': ERROR,
'react-hooks/set-state-in-effect': ERROR,
'react-hooks/static-components': ERROR,

✅ Pull Request Checklist:

Included link to corresponding React Spectrum GitHub Issue.
Added/updated unit tests and storybook for this change (for new code or code which already has tests).
Filled out test instructions.
Updated documentation (if it already exists for this component).
Looked at the Accessibility Practices for this feature - Aria Practices

📝 Test Instructions:

🧢 Your Project:

…ugin-react-hooks-followup

rspbot · 2025-10-17T06:06:05Z

Build successful! 🎉

snowystinger · 2025-10-17T06:33:30Z

packages/@react-aria/dnd/stories/Reorderable.tsx


  // Use a random drag type so the items can only be reordered within this list and not dragged elsewhere.
-  let dragType = React.useMemo(() => `keys-${Math.random().toString(36).slice(2)}`, []);
+  let dragType = React.useMemo(() => randomDragTypeReorderExample, []);


there's a lot of these, i just moved the random generator out of the component, that should be fine because you couldn't drag between different examples this way either and I don't think we render the same one twice as an example

snowystinger · 2025-10-17T06:35:27Z

packages/@react-aria/utils/test/mergeRefs.test.tsx

-        ref2 = useRef(null);
+        let internalRef1 = useRef(null);
+        let internalRef2 = useRef(null);
+        useEffect(() => {


other option i guess would be to exclude the compiler from running on test files, but seems better to always be in good habits

snowystinger · 2025-10-17T06:36:11Z

packages/@react-spectrum/tabs/src/Tabs.tsx

  }, [tablistRef, wrapperRef, direction, orientation, setCollapsed, prevTabPositions, setTabPositions]);

-  useEffect(() => {
+  useLayoutEffect(() => {


not sure why this was an effect, it's meant to measure the dom, so layout effect should be more correct

snowystinger · 2025-10-17T06:36:46Z

packages/react-aria-components/src/Table.tsx

    }
  });

+  // TODO: Lint doesn't catch these, it thinks we're not in a component render cycle here?


I'm not sure what to make of this yet, but not going to handle it in this series of PRs

kinda weird that it even catches the other ones, the hook that is being called doesn't create a component like https://react.dev/reference/eslint-plugin-react-hooks/lints/static-components demonstrates but perhaps I'm not understanding quite what React is seeing here. To be safe we could define one for table, tr, td, tbody, and th which should cover it I think

ok, went ahead and updated them

I'm mostly worried that the compiler isn't looking through these components, so won't do any optimisations
I'll raise an issue with React at some point about it probably

LFDanLu

The other changes look good to me, just one comment

LFDanLu · 2025-10-17T21:19:36Z

packages/react-aria-components/src/Table.tsx

    }
  });

+  // TODO: Lint doesn't catch these, it thinks we're not in a component render cycle here?


kinda weird that it even catches the other ones, the hook that is being called doesn't create a component like https://react.dev/reference/eslint-plugin-react-hooks/lints/static-components demonstrates but perhaps I'm not understanding quite what React is seeing here. To be safe we could define one for table, tr, td, tbody, and th which should cover it I think

…ugin-react-hooks-followup

rspbot · 2025-10-19T23:46:05Z

Build successful! 🎉

rspbot · 2025-10-19T23:55:12Z

Build successful! 🎉

rspbot · 2025-10-19T23:55:14Z

## API Changes

react-aria-components

/react-aria-components:RangeCalendarState

-RangeCalendarState <T extends DateValue = DateValue> {
+RangeCalendarState {
   anchorDate: CalendarDate | null
   focusNextDay: () => void
   focusNextPage: () => void
   focusNextSection: (boolean) => void
   focusPreviousDay: () => void
   focusPreviousPage: () => void
   focusPreviousRow: () => void
   focusPreviousSection: (boolean) => void
   focusSectionEnd: () => void
   focusSectionStart: () => void
   focusedDate: CalendarDate
   getDatesInWeek: (number, CalendarDate) => Array<CalendarDate | null>
   highlightDate: (CalendarDate) => void
   highlightedRange: RangeValue<CalendarDate> | null
   isCellDisabled: (CalendarDate) => boolean
   isCellFocused: (CalendarDate) => boolean
   isCellUnavailable: (CalendarDate) => boolean
   isDisabled: boolean
   isDragging: boolean
   isFocused: boolean
   isInvalid: (CalendarDate) => boolean
   isNextVisibleRangeInvalid: () => boolean
   isPreviousVisibleRangeInvalid: () => boolean
   isReadOnly: boolean
   isSelected: (CalendarDate) => boolean
   isValueInvalid: boolean
   maxValue?: DateValue | null
   minValue?: DateValue | null
   selectDate: (CalendarDate) => void
   selectFocusedDate: () => void
   setAnchorDate: (CalendarDate | null) => void
   setDragging: (boolean) => void
   setFocused: (boolean) => void
   setFocusedDate: (CalendarDate) => void
   setValue: (RangeValue<DateValue> | null) => void
   timeZone: string
   value: RangeValue<DateValue> | null
   visibleRange: RangeValue<CalendarDate>
 }

@react-aria/utils

/@react-aria/utils:useUpdateEffect

 useUpdateEffect {
-  effect: EffectCallback
+  cb: EffectCallback
   dependencies: Array<any>
   returnVal: undefined
 }

@react-stately/calendar

/@react-stately/calendar:RangeCalendarState

-RangeCalendarState <T extends DateValue = DateValue> {
+RangeCalendarState {
   anchorDate: CalendarDate | null
   focusNextDay: () => void
   focusNextPage: () => void
   focusNextSection: (boolean) => void
   focusPreviousDay: () => void
   focusPreviousPage: () => void
   focusPreviousRow: () => void
   focusPreviousSection: (boolean) => void
   focusSectionEnd: () => void
   focusSectionStart: () => void
   focusedDate: CalendarDate
   getDatesInWeek: (number, CalendarDate) => Array<CalendarDate | null>
   highlightDate: (CalendarDate) => void
   highlightedRange: RangeValue<CalendarDate> | null
   isCellDisabled: (CalendarDate) => boolean
   isCellFocused: (CalendarDate) => boolean
   isCellUnavailable: (CalendarDate) => boolean
   isDisabled: boolean
   isDragging: boolean
   isFocused: boolean
   isInvalid: (CalendarDate) => boolean
   isNextVisibleRangeInvalid: () => boolean
   isPreviousVisibleRangeInvalid: () => boolean
   isReadOnly: boolean
   isSelected: (CalendarDate) => boolean
   isValueInvalid: boolean
   maxValue?: DateValue | null
   minValue?: DateValue | null
   selectDate: (CalendarDate) => void
   selectFocusedDate: () => void
   setAnchorDate: (CalendarDate | null) => void
   setDragging: (boolean) => void
   setFocused: (boolean) => void
   setFocusedDate: (CalendarDate) => void
   setValue: (RangeValue<DateValue> | null) => void
   timeZone: string
   value: RangeValue<DateValue> | null
   visibleRange: RangeValue<CalendarDate>
 }

devongovett · 2025-10-20T18:10:55Z

packages/@react-spectrum/autocomplete/src/SearchAutocomplete.tsx

+  if (prevIsLoading !== isLoading && !isLoading) {
+    setShowLoading(false);
+    setPrevIsLoading(isLoading);
+  }


yeah this one felt weird to me as well, but https://react.dev/reference/react/useState#storing-information-from-previous-renders claims it is better than setting state in an effect (which the linter will now complain about)

devongovett · 2025-10-20T18:13:07Z

packages/@react-spectrum/menu/src/Menu.tsx

-  }, [leftOffset, popoverContainer]);
+  let [prevPopoverContainer, setPrevPopoverContainer] = useState<HTMLElement | null>(null);
+  if (popoverContainer && prevPopoverContainer !== popoverContainer && leftOffset.left === 0) {
+    let {left} = popoverContainer.getBoundingClientRect();


Reading from the DOM during render?

crud, i missed that one, i'll look at that

snowystinger added 6 commits October 15, 2025 13:39

turn on and fix static-components

ba77f40

turn on and fix set-state-in-effect

8f1cd8e

turn on and fix purity

95687e9

fix lower react version tests

a5803c0

turn on globals and fix errors

be90742

Merge branch 'update-eslint-plugin-react-hooks' into update-eslint-pl…

c073772

…ugin-react-hooks-followup

snowystinger changed the title ~~Update eslint plugin react hooks followup~~ chore: Update eslint plugin react hooks followup Oct 17, 2025

snowystinger added the ready for review label Oct 17, 2025

adobe deleted a comment from rspbot Oct 17, 2025

snowystinger commented Oct 17, 2025

View reviewed changes

LFDanLu reviewed Oct 17, 2025

View reviewed changes

snowystinger added 2 commits October 20, 2025 10:38

Merge branch 'update-eslint-plugin-react-hooks' into update-eslint-pl…

500c564

…ugin-react-hooks-followup

convert remaining useElementTypes

a81fa41

LFDanLu approved these changes Oct 20, 2025

View reviewed changes

devongovett reviewed Oct 20, 2025

View reviewed changes

chore: Update eslint plugin react hooks followup #9054

Are you sure you want to change the base?

chore: Update eslint plugin react hooks followup #9054

Conversation

snowystinger commented Oct 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Pull Request Checklist:

📝 Test Instructions:

🧢 Your Project:

Uh oh!

rspbot commented Oct 17, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

LFDanLu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

rspbot commented Oct 19, 2025

Uh oh!

rspbot commented Oct 19, 2025

Uh oh!

rspbot commented Oct 19, 2025

react-aria-components

/react-aria-components:RangeCalendarState

@react-aria/utils

/@react-aria/utils:useUpdateEffect

@react-stately/calendar

/@react-stately/calendar:RangeCalendarState

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

snowystinger commented Oct 17, 2025 •

edited

Loading