No warning for snapshots in callbacks are not recommended

[devnomic]

I literally coped the example. No eslint warning shows. I'm pretty sure my valtio eslint config is working, since if i write state.count in render, it will show a warning from (valtio/state-snapshot-rule).

const state = proxy({ count: 0 })

function App() {
  const snap = useSnapshot(state)
  const handleClick = () => {
    console.log(snap.count) // <- No warning shows here
  }
  return (
    <div>
      {snap.count} <button onClick={handleClick}>click</button> 
    </div>
  )
}

[dai-shi]

Thanks for reporting. Hmmm, not sure what's happening.

@barelyhuman Are you still around? Can take time to investigate it?

[barelyhuman]

always here 😂, yeah i’ll check it out

[barelyhuman]

It's a conflict between #16 and the Readme,

in the past fix we consider it okay to be read but not assigned to, so it let's it past the warning, removing the isReadOnly check would basically mean to revert fixes from #16

[dai-shi]

Thanks for the investigation!

Can we check the useEffect deps, and if so skip it?

btw, how does this work? (does lint pass with/without #16?)

function useExample0(s: typeof state) {
  const snap = useSnapshot(s.a1);

  const c = snap.b.c;
  useEffect(() => {
    if (c === 'a1c') {
      state.a1.b.c = 'example';
    }
  }, [c]);
}

this is less error-prone.

[barelyhuman]

If #16 is disabled

It still breaks, but that's been fixed and the lint does warn about the snapshot being used as expected.

Can we check the useEffect deps, and if so skip it?

So isReadOnly to add a check for the caller being useEffect or useCallback and that the dep uses the called snapshot (or snapshot propery)? It'll still break the test cases in #16 which it should 😂 , but yeah, we'll have to add a comment in #16 to let the devs there know that it's supposed to be in the deps to be considered valid.

[dai-shi]

Published: https://www.npmjs.com/package/eslint-plugin-valtio/v/0.4.3