Detect misusing snapshot for useEffect deps

[dai-shi]

Following #40.

Valtio's useSnapshot is incompatible with some use cases that are allowed (and recommended) in React.
It would be really nice if we can detect such cases and warn it.

What's not working

const state = proxy({ text: 'hello', count: 0 })
const Component = () => {
  const snap = useSnapshot(state)
  useEffect(() => {
    console.log('something is changed')
  }, [snap])
  return <div>{snap.text}</div>
}

The problem in the example above is when we change the count as ++state.count, the component won't re-render, thus the effect doesn't fire. What's worse is if snap is used in the useEffect callback body, but that's a different story (and already caught by other rules.)

This is not solvable by design and we let people use some workarounds.

Alternative 1

We educate valtio users to use only primitive values for useEffect deps.

The above non-working example would become this:

const state = proxy({ text: 'hello', count: 0 })
const Component = () => {
  const snap = useSnapshot(state)
  const { text, count } = snap // destructure outside useEffect
  useEffect(() => {
    console.log('something is changed')
  }, [text, count])
  return <div>{snap.text}</div>
}

This style doesn't conflict with React linter.

Alternative 2

If we really want to watch the reference change, we shouldn't use snap, but use state instead.

const state = proxy({ text: 'hello', count: 0 })
const Component = () => {
  const snap = useSnapshot(state)
  useEffect(() => subscribe(state, () => {
    console.log('something is changed')
  }), [])
  return <div>{snap.text}</div>
}

It's not exactly the same as the original code intention, but in practice, it would cover most use cases.

Idea

I originally thought this misusage can't be caught by our linter, but if we use TypeScript parser, it should be fairly easy to know if an object is the result of useSnapshot, even for nested objects.

I'm not sure if we can change types only for linter. Or, even without types, eslint has clever enough to do flow analysis?

[barelyhuman]

Can we close this considering we now support memo, or is there still a part I'm missing here?

[dai-shi]

Thanks for the note.
Yes, v2 will be fixing the memo issue.

However, this case is still troublesome.

const state = proxy({ text: 'hello', count: 0 })
const Component = () => {
  const snap = useSnapshot(state)
  useEffect(() => {
    console.log('something is changed')
  }, [snap])
  return <div>{snap.text}</div>
}

Let's leave it open for visibility.

[dai-shi]

Valtio v2 is released.