react icon indicating copy to clipboard operation
react copied to clipboard

Bug: eslint-react-hooks false positives on refs rule

Open marcospgp opened this issue 3 months ago • 17 comments

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>
  );
}

marcospgp avatar Oct 08 '25 14:10 marcospgp

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).

jeffijoe avatar Oct 08 '25 15:10 jeffijoe

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}:

Image

TkDodo avatar Oct 09 '25 13:10 TkDodo

playground repro:

https://playground.react.dev/#N4Igzg9grgTgxgUxALhAMygOzgFwJYSYAEUYCAsgJ4ASEEA1gBQCURwAOsUXIWDkTARoiAXhJkASkJadORAQhyxiHLvLgAbAIZgwAOS0BbBMiIByHAj5mANHPkK09gL6dXmWZgzZ8hIlQBBAAcgljZPdV5+ADctDSgEUXEKGjomZgiFJRhiAB4AEzxoxxFgWPiEADpBNGdubV0DY1LyhMrNHX0jBDqAegA+N09vXAJiQJCAJjDVSMw+IlbEsVIU2gYZLh55-mBHIjqxJft7QWy8wuKa0pq6jsbulri2+67jPsHMdxAbEG20PAAcxQIDwhiCEBg-BwlCCiT2AAV4oC8JgAPJBXzzA5ENAwCCGcwAIy0RIQGgAtEFkaiKYItLgKTxwXgNAgYL1CtYANyeRizIi9XrMoKsrRY8gQfImIjsEBxDRytxEMDivBgAFWIhIqAo9GYsZgZjcn7gAAWEAA7gBJTCWHJxMAoNCOnpAA

renrizzolo avatar Oct 10 '25 04:10 renrizzolo

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.

josephsavona avatar Oct 10 '25 04:10 josephsavona

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.

last-Programmer avatar Oct 10 '25 10:10 last-Programmer

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].

dfdeagle47 avatar Oct 13 '25 11:10 dfdeagle47

It seems the linter should check if .current is accessed, not whether a variable has ref in its name

atomiks avatar Oct 14 '25 08:10 atomiks

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} />
}

playground

phaux avatar Oct 14 '25 15:10 phaux

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.

silverwind avatar Oct 14 '25 16:10 silverwind

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.)

zackdotcomputer avatar Oct 20 '25 13:10 zackdotcomputer

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 }
}

n8tb1t avatar Oct 30 '25 22:10 n8tb1t

Are there any updates on this...?

jamie-nzfunds avatar Nov 04 '25 06:11 jamie-nzfunds

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} />
    </>
  );
}

will-stone avatar Nov 10 '25 17:11 will-stone

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}
    >...

craigkovatch avatar Nov 12 '25 09:11 craigkovatch

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

davitPetrosyanT avatar Nov 20 '25 13:11 davitPetrosyanT

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.

essenmitsosse avatar Nov 22 '25 21:11 essenmitsosse

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. 🙏

RohovDmytro avatar Nov 28 '25 00:11 RohovDmytro