0

Does the upcoming concurrent-mode break the old guarantee, that setState updates within a click handler are flushed synchronously at the event boundary?

If i have e.g. a button, that should only ever be pressed once, a supposedly working pattern was to "just set the state to disabled in the click handler":

let counter = 0;
const C = () => {
  const [disabled, setDisabled] = React.useState(false);
  const handler = React.useCallback(
    () => { setDisabled(true); counter++; },
    [], // setDisabled is guaranteed to never change
  );
  return (<button onClick={handler} disabled={disabled}>click me</button>);
};
// Assert: `counter` can never be made >1 by clicking the button with one C

This pattern used to be guaranteed to work (at least given that setting the disabled-attribute prevents any further click events, which seems to be the case). The biggest related question i could find discusses this, and also shows a more or less obvious alternative (and easier to prove it works), of using a ref (unlike the answer in the linked question, maybe rather a boolean ref, but same idea, it's always sync).

Side questions: Is this information up-to-date, or did something change? It's more than three years old after all. It mentions "interactive events (such as clicks)", what are the others?

However, in concurrent-mode, rendering can be paused, which i interpret as "the js thread will be released", to allow potential key presses or whatever events to trickle in, and in that phase, additional click events could also happen, before the next render disables the button. Is therefore the way to go to use some kind of ref, or maybe explicitly adding ReactDOM.flushSync?

Doofus
  • 952
  • 4
  • 19

2 Answers2

1

My current understanding of how concurrent mode works is this:

1 - a re-render starts

2 - hooks are called, they change internal state

3a - re-render is suspended

4a - internal state changes are rolled back

OR

3b - re-render is not suspended

4b - internal state changes are commited

useCallback is a thin wrapper over useMemo and uses "internal state" to save the cached value. (4a) is the key here, and from what I understand your solution is not guaranteed to work anymore.

The useRef (with a boolean flag value) solution has the same issue too because you're not guaranteed that the new value of the ref is actually going to be "commited" when re-rendering is suspended.

The useRef solution where you keep a ref to the DOM button element and directly manipulate the disabled attribute will still work even in concurrent mode. React has no way of blocking you from directly manipulating DOM.

"suspending" means reverting "internal state" + not applying the generated DOM manipulations, does not mean any side effects (like manipulating DOM directly) can be affected.

flushSync will not help either, it simply forces re-renders, does not guarantee that the current render won't be suspended.

Sergiu Paraschiv
  • 9,929
  • 5
  • 36
  • 47
  • Interesting, but can you elaborate, why you think even a ref wouldn't work (assuming generic reference cell)? As mentioned, and like the other answer suggests, i'd probably do a `useRef(false)`, for a generic mutable reference cell (alternatively, use a class-component and some member on the instance), and i don't think react has any say there. – Doofus May 05 '21 at 19:18
  • Oooh sorry, I did not properly read the answer you linked to. Looks like the ref is pointing to the actual DOM button. That will 100% still work. React has no way of blocking direct DOM manipulation. I was mistakenly assuming the ref solution uses a bool ref value. That version would indeed be broken in concurrent mode for the same reason the `useMemo` one would. Edit: updated my answer to clarify this. Again, my bad, did not properly read that answer. – Sergiu Paraschiv May 05 '21 at 19:26
  • To be clear, the solution proposed in the other answer _will most definitely not work_ in concurrent mode because `isDisabled.current = true;` is not guaranteed to be _commited_. – Sergiu Paraschiv May 05 '21 at 19:31
  • Given the `isDisabled.current = true` is being executed inside of the click handler, that should be before any `setState`, and therefore before any render phase even begins, yet react may undo it? I am kind of surprised, that react even touches mutable reference cells obtained by `useRef` at all, if they are not passed as a ref attribute, or similar. – Doofus May 05 '21 at 19:40
  • Does not matter when `setState` is called or when you change a ref's value, what matters is that it's effects won't be _commited_ (and visible in render calls) until the next render is finalized without being suspended. Otherwise you would end up with inconsistent state. Think about `useEffect`: if you allow `setState` to be commited but do not actually apply the DOM manipulations, what version of the state will `useEffect` see after next render? Worse, what about possible refs in that DOM you did not update? – Sergiu Paraschiv May 05 '21 at 19:48
  • Note, that the other answer suggests checking the ref inside of the click handler (disabled-attribute is only secondary there, so the next render result isn't too important). How i understood it, `useRef` gives a mutable reference cell, which isn't in any way bound to state, and doesn't care, what you do with it. It's essentially a helper for function-components, to simulate members of an equivalent class-component (keep a reference across render calls). For a class component, if i just add a member `disabled`, and then do `this.disabled = true;`, react wouldn't interfere with that either? – Doofus May 05 '21 at 19:58
  • You might be right. My current understanding of concurrent mode based on _limited_ experience with it is that ref value changes _should_ be rolled back when rendering is suspended too, otherwise strange things would happen. Unfortunately at the moment I can't think of how we would test this, except brute force :) – Sergiu Paraschiv May 05 '21 at 20:01
0

As far as I know the setState call was always async, and you never had a way to warranty that the button will be disabled right after the click. Also there is no such thing as concurrency in JS, it has single thread, the problem is that the render can happen latter than you expect so you can receive another click until React made re-render for you.

If you need to fire the logic only once I would advice to use useRef hook and when you need to make sure that we have not clicked the button just check the value.

const isDisabled = useRef(false);
const onClick = () => {
     if (!isDisabled.current) {
        isDisabled.current = true;
     }
}
Ayzrian
  • 2,279
  • 1
  • 7
  • 14