Bug: eslint-react-hooks false positives on refs rule
after updating eslint-react-hooks from v5 to v6, I'm getting false positives on the rule eslint(react-hooks/refs)
every props.<something> access in the code below is being flagged by this rule, when clearly it is not a ref access.
function FieldInternal(props: FieldBaseProps & { children?: React.ReactNode }) {
return (
<F.Root
name={props.name}
className="contents"
validate={(value: unknown) => props.validate?.(value as string) ?? null}
validationMode={props.validationMode}
>
{props.label && <F.Label>{props.label}</F.Label>}
<div className={`gap-x-sm gap-2xs flex min-w-0 items-start ${props.grow ? "grow" : ""} ${!props.label ? "col-span-2" : ""}`}>
<div className={`gap-3xs w-md flex min-w-0 flex-col ${props.grow ? "grow" : ""}`}>
<F.Control
ref={props.ref}
placeholder={props.placeholder}
autoComplete={props.autoComplete}
autoFocus={props.autoFocus}
disabled={props.disabled}
className={`bg-bg-2 px-2xs py-2xs bevel-inset-bg-2 w-full min-w-0 outline-0 data-[disabled]:cursor-not-allowed ${props.disabled && "cursor-not-allowed"}`}
/>
<F.Error className="text-text-danger min-w-0 text-sm" />
</div>
{props.children}
</div>
</F.Root>
);
}
I have a similar issue.
export function DocumentTitleProvider({
children,
}: DocumentTitleProviderProps) {
const [value, set] = useState('')
const currentTitleRef = useRef('')
const setTitle = useCallback((v: string) => {
set(v)
setDocumentTitle(v)
currentTitleRef.current = v
}, [])
const contextValue: DocumentTitleContextValue = useMemo(
() => ({
currentTitleRef,
value,
set: setTitle,
}),
[value, setTitle],
)
return (
<DocumentTitleContext value={contextValue}>
<ScreenReaderAnnounce text={contextValue.value} />
{children}
</DocumentTitleContext>
)
}
Every contextValue access is redlined with:
Error: Cannot access refs during render
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
Yep, seeing this too in the sentry codebase. Minimal reproduction:
export function Parent() {
const ref = useCallback(() => {
console.log('ref');
}, []);
return <Child myRef={ref} className="foo" />;
}
function Child(props: {className: string; myRef: () => void}) {
const internalRef = useCallback(() => {
console.log('ref');
}, []);
return (
<div>
<div ref={internalRef} className={props.className} />
<div ref={props.myRef} className={props.className} />
</div>
);
}
here, the first div:
<div ref={internalRef} className={props.className} />
works fine but the second div that uses the ref from the prop yields not only a false-positive error on ref={props.myRef}, but also on className={props.className}:
playground repro:
https://playground.react.dev/#N4Igzg9grgTgxgUxALhAMygOzgFwJYSYAEUYCAsgJ4ASEEA1gBQCURwAOsUXIWDkTARoiAXhJkASkJadORAQhyxiHLvLgAbAIZgwAOS0BbBMiIByHAj5mANHPkK09gL6dXmWZgzZ8hIlQBBAAcgljZPdV5+ADctDSgEUXEKGjomZgiFJRhiAB4AEzxoxxFgWPiEADpBNGdubV0DY1LyhMrNHX0jBDqAegA+N09vXAJiQJCAJjDVSMw+IlbEsVIU2gYZLh55-mBHIjqxJft7QWy8wuKa0pq6jsbulri2+67jPsHMdxAbEG20PAAcxQIDwhiCEBg-BwlCCiT2AAV4oC8JgAPJBXzzA5ENAwCCGcwAIy0RIQGgAtEFkaiKYItLgKTxwXgNAgYL1CtYANyeRizIi9XrMoKsrRY8gQfImIjsEBxDRytxEMDivBgAFWIhIqAo9GYsZgZjcn7gAAWEAA7gBJTCWHJxMAoNCOnpAA
Thanks for reporting, I think I know what’s going on. This has to do with the way we’re inferring types that contain refs.
This is causing the react compiler to skip the problem file from compilation. we are getting lots of files skipped from compilation because of this issue.
A similar issue happens when using this library (https://floating-ui.com/docs/usefloating):
function App() {
const { refs, floatingStyles } = useFloating();
return (
<>
<div ref={refs.setReference} />
<div ref={refs.setFloating} style={floatingStyles} />
</>
);
}
->
Found 1 error:
Error: Cannot access refs during render
React refs are values that are not needed for rendering. Refs should only be accessed outside of render, such as in event handlers or effects. Accessing a ref value (the `current` property) during render can cause your component not to update as expected (https://react.dev/reference/react/useRef).
4 | <>
5 | <div ref={refs.setReference} />
> 6 | <div ref={refs.setFloating} style={floatingStyles} />
| ^^^^^^^^^^^^^^^^ Cannot access ref value during render
7 | </>
8 | );
9 | }
This started happening with [email protected].
It seems the linter should check if .current is accessed, not whether a variable has ref in its name
This fails:
export function App() {
const ref = React.useRef(null)
const elem = <div />
return React.cloneElement(elem, { ref })
}
even though its equivalent to this:
export function App() {
const ref = React.useRef(null)
return <div ref={ref} />
}
A similar issue happens when using this library (https://floating-ui.com/docs/usefloating):
This is a false-positive as per https://github.com/floating-ui/floating-ui/discussions/3405.
I'm also seeing an issue with false-positives for this rule but with a slightly different repo case:
const userLocationRef = useRef<string | null>(null);
const computeHeaders = useCallback(
() =>
userLocationRef.current
? {
"x-user-location": userLocationRef.current
}
: ({} as Record<string, string>),
[userLocationRef]
);
const { messages, sendMessage } = useChat({
transport: new DefaultChatTransport({
headers: computeHeaders
})
});
I'm getting a flag on the useChat call because the rule is incorrectly flagging passing a callback which checks the .current value of the ref the same as if I was accessing .current right away during render. (I understand that while technically the API could call the callback immediately during the render loop, it seems to me that passing a hooks-memoized function checks .current should be given the benefit of the doubt and not flagged as an issue since this is a pretty common pattern.)
Thats very disappointing of a react team to release a rule that will brake 99% of legacy code, considering ref is not only used for DOM, and a lot of times used for storing some flag value, meaning being fully controlled by the developer and totally not related to rendering cycle. so the right way would have been to introduce some alternative hook for arbitrary non reactive state
Something like
const useValue = <Value>(value: Value) => {
const store = useRef(value)
const set = useCallback((value: Value) => {
store.current = value
}, [])
return { value: store, set }
}
Are there any updates on this...?
As for floating UI, I've found if you destructure the refs then the compiler works and no ESLint error. To use the same example by @phaux you can do this:
function App() {
const { refs, floatingStyles } = useFloating();
const { setReference, setFloating } = refs
return (
<>
<div ref={setReference} />
<div ref={setFloating} style={floatingStyles} />
</>
);
}
Our false positive has this shape:
const DropdownPopup: React.FC<DropdownPopupProps> = React.memo(props => {
return !props.children ? null :
<DropdownOverlay
alignment={props.popupAlignment}
getElementToFocus={props.getElementToFocus}
^^^^^^^^^^^^^^^^^^^^^^^ Cannot access ref value during render
ref={props.overlayRef}
>...
Destructuring like this fixes the linter error, but very disappointing that this is necessary:
const DropdownPopup: React.FC<DropdownPopupProps> = React.memo(({ overlayRef, ...props }) => {
return !props.children ? null :
<DropdownOverlay
alignment={props.popupAlignment}
getElementToFocus={props.getElementToFocus}
ref={overlayRef}
>...
Thats very disappointing of a react team to release a rule that will brake 99% of legacy code, considering ref is not only used for DOM, and a lot of times used for storing some flag value, meaning being fully controlled by the developer and totally not related to rendering cycle. so the right way would have been to introduce some alternative hook for arbitrary non reactive state
Something like
const useValue = <Value>(value: Value) => { const store = useRef(value)
const set = useCallback((value: Value) => { store.current = value }, [])
return { value: store, set } }
I want to add that I have similar patterns across 35+ production projects. How should we migrate or fix all of this? If React introduces this rule, then there should also be a recommended built-in way to create a callback whose referance never changes but always has access to the latest state
Destructering works for regular refs in floating-ui, but this still creates an error, I don't know how to recover from:
const MyComp = (props) => {
const refArrow = useRef(null)
const {
refs: { setReference, setFloating },
floatingStyles,
context,
} = useFloating({
middleware: [
arrow({ element: refArrow }),
// ^^^^^^^^^^^^^^^^^
// Passing a ref to a function may read its value during render
],
})
return (
<div ref={setReference}>
{props.children}
<div ref={setFloating} style={floatingStyles}>
<FloatingArrow ref={refArrow} context={context}/>
Tooltip
</div>
</div>
)
}
Also these components actually have become buggy, at least in dev mode for me. So this might actually be breaking the library here.
Cool. It is called 1.0 though, that should cheer us up.
I have 300+ false-positives, not being able to get files compliled properly. Would be very nice to find a solution or a workaround for this. 🙏