useMemo factory being called more frequently / unnecessarily

[wnayes]

There seems to have been a change with useMemo around the 10.8.1 release, I believe starting with PR #3567.

When a state updater is called during render, and a useMemo hook is present, the memo "factory" function will get called more than once. The prior behavior was that we would reuse the memoed value from the first render.

I believe this is because the code now stores the computed value in _pendingValue and doesn't copy it over to _value until a later point. Renders triggered by a state updater must be happening quicker than this copy over, causing the work to be lost.

To Reproduce

https://codesandbox.io/s/goofy-cherry-emgdox?file=/index.js

A demo component illustrates the behavior.

let idState = 0;
function createId() {
  return "_id" + ++idState;
}

const TestComp = () => {
  const id = useMemo(() => createId(), []);
  const [stateId, setStateId] = useState("");

  if (!stateId) {
    setStateId(id);
  }
  return <div>Last id from memo: {id}</div>;
};

render(<TestComp />, window.root);

The browser displays "Last id from memo: _id2" indicating that createId (the function being memoized) was called twice.

Expected behavior
I would expect the memoized computations not to be lost between renders, even if they are happening in quick succession.

[JoviDeCroock]

This is actually intended behavior, you are calling setState in render which makes us dispose the work in progress from the last render: https://github.com/preactjs/preact/blob/master/hooks/src/index.js#L42-L48

i.e. we don't commit memoization from a canceled render, this ties into future stories for correctness when we talk about partial hydration/...

[wnayes]

Got it - just checked React as well and the behavior is the same there. It was kind of a surprising change for a minor release (broke some tests doing re-renders like this). But as long as it is expected, I don't think a change is needed then. Thanks!

[JoviDeCroock]

I have gone back and forth on this being a minor but in general what played most part in this is that it would only result in weird behavior for side-effects/inpure memoization.

i.e. if we were to do useState(createId) this would not have been an issue or similarly if we defer setting the stateId to an effect/... as it kind off is a side-effect which mutates global state.

I can however relate that in practice this isn't the most fun answer to get and I apologize for any inconvenience this caused you