useComputed doesn't update on deps change

[stephenh]

We have our own mobx-based useComputed, and I was looking at Legend's implementation just out of curiosity.

I thought this approach was neat:

                // setValue may be undefined if this is the first run
                setValue?.(v);

But in trying the approach out for our useComputed test suite, it doesn't actually work b/c contrary to the comment of "setValue is only undefined on the first run", it's actually always undefined, b/c on the 2nd run, i.e. useMemo has noticed that deps has changed and running effect again (for us autorun), setValue is still undefined, b/c this is a brand new invocation of the useComputed function, and setValue-the-setter still hasn't been set by useState.

I think for this to work you'd need to use a ref to hold the setValue? i.e. something like:

function useComputed(...) {
  const setValueRef = useRef();
  const initial = useMemo(() => {
    ...
    if (setValueRef.current) setValueRef.current(v);
  });
  const [value, setValue] = useState(initial);
  // Let the next invocation of useComputed get setValue
  setValueRef.current = setValue
  return value;

[jmeistrich]

Ah, good point! We hadn't actually tested it with the deps array yet since we made that change. I'll look into fixing that. The ref solution looks like it would work 👍.

I'll experiment with it to try find a solution without adding a hook if possible, but otherwise will go for the ref solution. Thanks!

[jmeistrich]

I'm actually changing it significantly based on some other changes I've been working on. useComputed will now be the basis of observer so everything is more consistent. I'm finishing testing another branch and will hopefully release it tomorrow.

Preview of the change if you're interested: https://github.com/LegendApp/legend-state/blob/primitive/src/react/useComputed.ts

Note that it looks longer but a bug chunk of it is wrapped in process.env.NODE_ENV === 'development' to workaround React 18 strict mode's double useEffect. And it's just got the two hooks (useReducer to force render and useEffect to cleanup).

[jmeistrich]

This should be fixed in 0.16, especially since it doesn't have deps anymore. Please let me know if there's still any issues.