birdofpreyru/react-global-state

Updating the global state inside a `useAsyncData()` hook's loader is dangerous if that hook depends in turn on the updated state

Closed this issue · 0 comments

Discovered in a recent commercial project, which can't be linked, but the idea is this:

const [dep, setDep] = useGlobalState('dep.path');
useAsyncData('some.path', (old) => {
  if (old === 'a-new-value') return old;

  // This call triggers a re-evaluation of the entire useAsyncData() hook before
  // the "a-new-value" result, returned below, is stored into the global state, so
  // if there is a check similar to the one above it will operate with `old` equal to
  // a previous value, say "null" and it won't short-cut the loader (if that was an intention),
  // and will repeat its entire body.
  setDep('a-new-dep-value');

  return 'a-new-value';
}, {
  deps: [dep],
});

The problem can be worked around by wrapping setDep(..) call in this sample code into a setTimeout(..) operation. It works because update of the value itself (if refreshAge setting is not zero) won't trigger re-loading, and when the dependency triggers re-evaluation of the hook the old value will be equal to "a-new-value" already.

So... this problem is not a bug per se, as it all works correctly, as per current logic... the other thing is that when a need arise to do such state manipulations it is easy to make a mistake, even knowing in-depth how the library works... at very least we need a warning in such case, or better do something additionally to turn the loader into an effectively atomic operation, as far as the surrounding useAsyncData() hook is concerned (i.e. the hook won't re-evaluate until the previously triggered loader returns... however, I think it is not desireble in other scenarios where deps are supposed to effectively abort and redo a previously started loading operation).

Thus... I don't know, perhaps here is nothing to improve, but it should be considered if a more convenient design is feasible. Perhaps the best we can do here is to add a warning to the docs?