jotaijs / jotai-scope

MIT License
55 stars 4 forks source link

Intercepted copy unexpectedly get cached by jotai #21

Closed yf-yang closed 7 months ago

yf-yang commented 7 months ago

Reproduction link

How to reproduce:

  1. click button to add counter
  2. click toggle to hide child
  3. click button to add counter
  4. click toggle to show child, child and parent are no longer synced.

Why:

When the child read countAtom for the first time, the following lines are executed: https://github.com/pmndrs/jotai/blob/80c8d9f54cde6449b728e06508ad00530e90943c/src/vanilla/store.ts#L444-L447

const valueOrPromise = atom.read(getter, options as any)
return setAtomValueOrPromise(atom, valueOrPromise, nextDependencies, () =>
  controller?.abort(),
)

Here the atom is an intercepted copy created by the scope. Then it directly added the value to the atom state map with the intercepted copy as the key.

Then, when the child is shown again, https://github.com/pmndrs/jotai/blob/80c8d9f54cde6449b728e06508ad00530e90943c/src/vanilla/store.ts#L368-L375

// See if we can skip recomputing this atom.
const atomState = getAtomState(atom)
if (!force && atomState) {
  // If the atom is mounted, we can use the cache.
  // because it should have been updated by dependencies.
  if (mountedMap.has(atom)) {
    return atomState
  }

The cached value of the intercepted copy is directly read, without reading from the actual unscoped atom.

How to fix

No idea, I am not sure if this is an issue with atom subscription or atom read. @dai-shi Do you have any comments?

dai-shi commented 7 months ago

I'm not sure if I fully understand it, but is it a newly found issue? It sounds pretty common.

From Jotai core perspective, it doesn't know if an atom is an intercepted one or not. So, it assumes a normal atom. So, if atomB is an intercepted one of atomA, atomB's dependencies should include atomA, and atomA's dependents should include atomB. Then, when atomA updates, it will propagate the change to atomB.

yf-yang commented 7 months ago

Yep, when they are out of sync, if the counter is incremented again, they are sync again. I think the issue is, the scope does not change (so the intercepted copy is still there), then when the atom is mounted again, it reads the cached value of the intercepted copy, instead of the actual atom, until the next update happens and they are synced again.

dai-shi commented 7 months ago

Can this be related? https://github.com/pmndrs/jotai/pull/2363#discussion_r1472011121

yf-yang commented 7 months ago

Let me describe what happens within the ScopeProvider:

useAtomValue(countAtom);
// =>
  store.get(countAtomCopy);
  // =>
    readAtomState(countAtomCopy);
    // => 
      const atomState = getAtomState(countAtomCopy) // undefined
      countAtomCopy.read(getter);
      // =>
        get(countAtom); // Here it reads the original atom, let's assume it is 100
      setAtomValueOrPromise(countAtomCopy); // now countAtomCopy is in atomStateMap, its value is 100

Now, the child is hided, so within the scope countAtom (countAtomCopy) is unmounted, but the ScopeProvider is still there. Therefore, countAtomCopy is still in atomStateMap. When increasing countAtom, the countAtomCopy does not change because it is unmounted. Then, when the child is shown again

useAtomValue(countAtom);
// =>
  store.get(countAtomCopy);
  // =>
    readAtomState(countAtomCopy);
    // => 
      const atomState = getAtomState(countAtomCopy) // 100, although the original atom is increased to 200
      return atomState; // the original atom is no longer read by countAtomCopy, because we have the falsy cached value
      // This line is bypassed --- countAtomCopy.read(getter);

I think it is not an issue with the dependency graph. The dependency subscription works as expected.

yf-yang commented 7 months ago

Seems I know the root cause 🤔