Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

useControlledState setState callback does not work #2320

Closed
snowystinger opened this issue Sep 10, 2021 · 0 comments · Fixed by #2304
Closed

useControlledState setState callback does not work #2320

snowystinger opened this issue Sep 10, 2021 · 0 comments · Fixed by #2304
Assignees

Comments

@snowystinger
Copy link
Member

snowystinger commented Sep 10, 2021

🐛 Bug Report

We have two issues in useControlledState, both in the setState callback syntax case.

The first we see intermittently as something along these lines:

Warning: Cannot update a component (`ControlledToggleButton`) while rendering a different component (`ToggleButton`). To locate the bad setState() call inside `ToggleButton`, follow the stack trace as described in https://fb.me/setstate-in-render

This is due to a side effect that we call 'onChange' inside the callback function and onChange in a controlling component frequently calls setState itself, therefore we call 'setState' while we're rendering a different component.

setState(prev => {
  let next = something based on prev);
  onChange(next);
  return next;
})

This can be observed in this codesandbox in the application https://codesandbox.io/s/jolly-cache-8esil?file=/src/App.js easiest to see by clicking the 'plain controlled' example twice.

However, there is a worse but related bug, in which useControlledState in a controlled usage with the setState callback functionality does not work. This can be seen in the same codesandbox's tests https://codesandbox.io/s/jolly-cache-8esil?file=/src/test.spec.js:2720-2751
We try to perform two state updates within one render cycle. This will result in useControlledState reusing the same 'prev' state for both calls and so it'll only be incremented once.

🤔 Expected Behavior

We would expect the useState callback syntax 'prev' value to actually have the previous value.
onChange would be called an appropriate number of times with the right value to make the next change.

😯 Current Behavior

useControlledState support for setState callback is broken.

💁 Possible Solution

We couldn't determine the appropriate number of times for onChange to be called, see https://codesandbox.io/s/jolly-cache-8esil?file=/src/test.spec.js:4279-4358
Should it be once with the final state we think we should be at? or should we fire onChange for every value we go through?

In a controlled mode, we may say that the next state should be x, however, the component controlling ours may elect to ignore that. We cannot know if they will or will not use the next value we give them. Because of this, we cannot tell when to reset our own state to the original.
Example:
We have a NumberField and the controlling component has the value at 1, “3” is typed on the end and then the user blurs, we store our internal state as 13 and fire onChange with 13, and the application decides to ignore it because 13 is unlucky and they don’t set state. We don’t know to update our state back to the controlled value passed in. Next the user presses the increment button. We take the state we currently have, 13, and increment it, yielding 14 in onChange instead of 2.

🔦 Context

💻 Code Sample

🌍 Your Environment

Software Version(s)
react-spectrum
Browser
Operating System

🧢 Your Company/Team

🕷 Tracking Issue (optional)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant