Infinite render loop when the toggle button's ref is stored in a state variable

[nilscox]

• downshift version: 8.1.0
• node version: 18
• pnpm version: 8.6.12

Relevant code or config

<div {...getToggleButtonProps({ref: setRef})}>

What you did:

I am trying to set a ref on the toggle button of a custom Select component.

What happened:

Selecting an item on a mobile device triggers a re-render loop.

Uncaught Error: Maximum update depth exceeded. This can happen when a component repeatedly calls setState inside componentWillUpdate or componentDidUpdate. React limits the number of nested updates to prevent infinite loops.
    at checkForNestedUpdates (react-dom.development.js:27292:11)
    at scheduleUpdateOnFiber (react-dom.development.js:25475:3)
    at dispatchSetState (react-dom.development.js:17527:7)
    at eval (downshift.esm.js:124:9)
    at Array.forEach (<anonymous>)
    at eval (downshift.esm.js:122:10)
    at safelyDetachRef (react-dom.development.js:22908:22)
    at commitMutationEffectsOnFiber (react-dom.development.js:24351:13)
    at recursivelyTraverseMutationEffects (react-dom.development.js:24273:7)
    at commitMutationEffectsOnFiber (react-dom.development.js:24293:9)
    ...

Reproduction repository:

Open this codesandbox's app in a new tab, open the devtool with touch simulation enabled, click the label and select an item.

Problem description:

This only happens when trying to store the toggle button's ref in a state variable. The issue does not happen when removing the ref prop given to getToggleButtonProps.

This only happens with react-dom's createRoot function, so it may be related to #1384.

This issue is similar to #1511, but it happens without any other library like formik or floating-ui and the stack trace isn't the same, so I assumed this is a different underlying issue.

[aliceHendicott]

Experiencing this issue as well. Originally, I noticed this as we use Downshift alongside Floating UI which uses this method to store their refs through a callback ref - I've replicated that behaviour in CodeSandbox as well.

I believe this comment in the Formik issue is similar to what we're experiencing, with the stack trace pointing to the handleRefs function.

Also similar to the other issue reported with Formik, it seems like the itemClick function is being called multiple times (also logged in the code sandbox above as well).

[MateuszLeo]

Hi

I've run into the same kind of the issue today, If you look closer into implementation it does make sense that re-render occurs, following props getters:

• getMenuProps
• getInputProps
• getToggleButtonProps

Are calling ref callback (in this case, setState) when we call getXProps getter. If you're using setState for storing ref you're running into setState/re-render loop, pseudo code:

function Component() {
  const combobox = useCombobox({ items: [] });
  const [ref, setRef] = useState(null)

  return <div {...combobox.getToggleButtonProps({ ref: setRef })} />
}

getToggleProps() {
   ....
   refs.forEach(() => ref(node)) //call setRef (which is actually `setState` call) and re-renders if underlying dom node is different (different reference).
   ... 
}

One thing is weird to me though, why this is only reproducible on touch devices. That's why my theory might be completely invalidated by downshift team.

My fix for this issue is wrapping call getXProps into useMemo with proper dependencies. In your case @aliceHendicott my fix is:

  const setReference = useCallback(
    (node) => {
      _setReference(node);
    },
    [_setReference]
  );

  const toggleProps = useMemo(() => {
    return getToggleButtonProps({ ref: setReference });
  }, [setReference]);
  
  <div {...toggleProps} />

[silviuaavram]

I tried to repro the issue on local, and I could not do it so far. I tried to copy all the relevant code from both @aliceHendicott and @nilscox sandboxes, and when I go to the responsive mode and pick an element, it works as expected.

Can someone help me repro this on local? I am using the docusaurus based npm run docs:dev, specifically the useSelect example in docusaurus/pages.

I'm thinking that having this repro on local will help with debugging. Thanks!

[silviuaavram]

Actually just use this branch. #1549

[silviuaavram]

What does not make sense at all is why only the clicking items triggers this infinite loop. Toggling the button works, arrow keys work, enter key works.

So far I can only tell that, after the clicking action happens, the dispatch kicks a ItemClick state change, state changes, render happens, and then the useControlledReducer will go wild. It will re-calculate state with the ItemClick state change, apparently the new state is different, another render happens, then the re-calculate happens again, and so on.

[LisaHao]

I'm also experiencing this issue but only with Android devices! (I've only tried it on Chrome so far)

[silviuaavram]

Everyone, try [email protected]. If this solves the issue and does not break anything, we will release it as normal 8.2.4. Thank you!

https://www.npmjs.com/package/downshift/v/8.2.4-alpha.2

[nilscox]

Hey @silviuaavram, thanks for taking the time to look into it. I added a commit on a fork of your branch to move the setReference to the getToggleButtonProps callback. This reproduces the original issue.

I was able to test [email protected], but it seems that the issue still occurs. Here is a codesandbox with this version installed.

[danichim]

#1564, i will follow here. Thanks

[silviuaavram]

If someone wants to investigate why this issue happens, feel free.

We should definitely try to fix it on our end, it might also have something to do with #1447.

As a workaround, why not use useRef instead of useState for the Ref? I would like to understand this use case better.

[danichim]

Hi @silviuaavram, problem seems to be on getToggleButtonProps when i pass ref: setReference from floating-ui i have error:

If the ref prop is not passed, everything works fine.
If i will pass ref directly to button works everthing fine but i have other error from downshift:
downshift: The ref prop "ref" from getToggleButtonProps was not applied correctly on your element.

[silviuaavram]

@danichim why don't you use useRef?

[danichim]

I use refs from useFloating, needs to be re-render when you scroll. On 9 Jan 2024, at 11:54, Silviu Alexandru Avram ***@***.***> wrote: @danichim why don't you use useRef? —Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you were mentioned.Message ID: ***@***.***>

[silviuaavram]

Got it. I was simply curious. It's important that we fix this issue, but I don't have bandwidth at the moment for it. If someone can figure it out I will gladly review the changes. Thanks!

[danichim]

I solved my issue adding suppressRefError to getToggleButtonProps and pass ref prop to button. Many thanks.

[danichim]

Experiencing this issue as well. Originally, I noticed this as we use Downshift alongside Floating UI which uses this method to store their refs through a callback ref - I've replicated that behaviour in CodeSandbox as well.

I believe this comment in the Formik issue is similar to what we're experiencing, with the stack trace pointing to the handleRefs function.

Also similar to the other issue reported with Formik, it seems like the itemClick function is being called multiple times (also logged in the code sandbox above as well).

Experiencing this issue as well. Originally, I noticed this as we use Downshift alongside Floating UI which uses this method to store their refs through a callback ref - I've replicated that behaviour in CodeSandbox as well.

I believe this comment in the Formik issue is similar to what we're experiencing, with the stack trace pointing to the handleRefs function.

Also similar to the other issue reported with Formik, it seems like the itemClick function is being called multiple times (also logged in the code sandbox above as well).

@aliceHendicott Can you check this sandbox https://codesandbox.io/p/sandbox/react-v18-callback-ref-downshift-forked-ggr3wp it works without any change on library.

[danichim]

Hey @silviuaavram, thanks for taking the time to look into it. I added a commit on a fork of your branch to move the setReference to the getToggleButtonProps callback. This reproduces the original issue.

I was able to test [email protected], but it seems that the issue still occurs. Here is a codesandbox with this version installed.

https://codesandbox.io/p/sandbox/polished-smoke-tt737w A solution for your code @nilscox

[nilscox]

My use case is also to use downshift with floating-ui, here is a minimal repro with both libraries.

Thanks for finding workarounds @danichim, your solution works if we also memoize the menu props (and exclude the getMenuProps callback from the dependencies array, see the other file). Looks like the issue isn't strictly linked to the getToggleButtonProps callback.

I won't have much bandwidth myself these days, but I'll let you know if I do one day.

[silviuaavram]

Hey everyone! I think I got it.

#1571

I also released the 8.3.2-alpha.0 version if you'd like to test it out. Please also check the details in the PR, see if they make sense, and let me know what you think!

[cabello]

It might be a new issue, but with very similar outcome, I still see a complete crash of the app when using mobile Safari and selecting a value in the modal, reporting it just in case someone is also pulling their hairs out, I haven't received a useful stack trace yet.

[1kuko3]

I'm a bit confused about versions currently - in my case, the 8.3.2-alpha.0 fixes the issue with infinite re-render after touch interaction. But I can still reproduce the issue when I use 8.5.0 or the latest 9.0.0. Will changes from 8.3.2-alpha.0 be present in future versions of downshift?