Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Intercepted copy unexpectedly get cached by jotai #21

Closed
yf-yang opened this issue Jan 31, 2024 · 5 comments · Fixed by pmndrs/jotai#2371
Closed

Intercepted copy unexpectedly get cached by jotai #21

yf-yang opened this issue Jan 31, 2024 · 5 comments · Fixed by pmndrs/jotai#2371

Comments

@yf-yang
Copy link
Collaborator

yf-yang commented Jan 31, 2024

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
Copy link
Member

dai-shi commented Jan 31, 2024

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
Copy link
Collaborator Author

yf-yang commented Jan 31, 2024

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
Copy link
Member

dai-shi commented Jan 31, 2024

Can this be related? pmndrs/jotai#2363 (comment)

@yf-yang
Copy link
Collaborator Author

yf-yang commented Jan 31, 2024

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
Copy link
Collaborator Author

yf-yang commented Jan 31, 2024

Seems I know the root cause 🤔

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants