chore: Update eslint plugin react hooks immutability #9055
Conversation
# Conflicts: # packages/@react-stately/checkbox/src/useCheckboxGroupState.ts
…eslint-plugin-react-hooks-immutability
snowystinger
changed the title
| }, [document]); | ||
| let getServerSnapshot = useCallback(() => { | ||
| document.isSSR = true; | ||
| document.setSSR(true); |
There was a problem hiding this comment.
had to introduce this because the React compiler is a bit strict about mutating, I think it should be allowed in this case, but React doesn't know that, so I've tricked it. I had tried storing the document in a ref and editing it that way, which fixes the compiler problem, but it broke some tests, I'm not sure why and it makes me a little worried. This is the closest to what we've tested for years though, so I'm ok with this for now.
|
|
||
| let componentIds = new WeakMap(); | ||
|
|
||
| function useCounter(isDisabled = false) { |
There was a problem hiding this comment.
moved these things that can't be readily fixed to other files so the compiler won't bail on the entire file
|
|
||
| // Syncs ref from context with ref passed to hook | ||
| export function useSyncRef<T>(context?: ContextValue<T> | null, ref?: RefObject<T | null>): void { | ||
| export function useSyncRef<T>(contextRef?: RefObject<T | null> | null, ref?: RefObject<T | null>): void { |
There was a problem hiding this comment.
private function so I'm ok changing the API
however, if we're worried, I can make a second one which is safe for the compiler vs this old one
| }; | ||
| } | ||
|
|
||
| function useScrollViewContentSizeChange({updateSize, contentSize, isUpdatingSize, setUpdate}: {updateSize: (flush: typeof flushSync) => void, contentSize: Size, isUpdatingSize: RefObject<boolean>, setUpdate: (update: {}) => void}) { |
There was a problem hiding this comment.
may need to move to a separate file, can't readily fix this one
| } | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, [isFocused]); | ||
| }, [isFocused, style]); |
There was a problem hiding this comment.
not sure why style was omitted?
| useLayoutEffect(() => { | ||
| checkForOverflow(); | ||
| }, [checkForOverflow]); | ||
| }, [checkForOverflow, children, scale]); |
There was a problem hiding this comment.
previous way was wrong and no reason to invalidate the callback function for other dependency arrays
| focusMode: 'cell' | ||
| }); | ||
|
|
||
| // eslint-disable-next-line react-hooks/immutability |
There was a problem hiding this comment.
old component that was never really finished, ignoring
| ? <DashSmall UNSAFE_className={classNames(styles, 'spectrum-Checkbox-partialCheckmark')} /> | ||
| : <CheckmarkSmall UNSAFE_className={classNames(styles, 'spectrum-Checkbox-checkmark')} />; | ||
|
|
||
| return ( |
There was a problem hiding this comment.
probably could join the render itself and share that as a component, but it was easier to just start by copying
| }; | ||
| // eslint-disable-next-line react-hooks/exhaustive-deps | ||
| }, []); | ||
| }, [type]); |
There was a problem hiding this comment.
this one might be incorrect, but I'm not sure what it should be to make lint happy. I think type never changes?
| granularity, | ||
| hasTime, | ||
| setDate(part, date) { | ||
| // Both the start and end datefields update on form reset back to back, so this is a problem due to not supporting setState callbacks |
There was a problem hiding this comment.
comment a little out of date with changes we made
| let value = controlledValue || placeholderValue; | ||
|
|
||
| let setValue = (newValue: RangeValue<DateValue | null> | null) => { | ||
| // TODO: I don't know how to fix this one |
There was a problem hiding this comment.
I tried 3 times to re-write this file, I failed each time. Anyone else want a swing at it?
…eslint-plugin-react-hooks-immutability
…eslint-plugin-react-hooks-immutability
2 participants
Closes
This PR focuses on react-hooks/immutability and removing a few eslint-disable-next-lines
✅ Pull Request Checklist:
📝 Test Instructions:
🧢 Your Project: