Atoms that are mounted only inside atomEffects may unmount between runs

[dmaskasky]

Related Issue: #21

Atoms that are mounted only inside atomEffects may unmount between runs.
This will happen if:

1. They have an onMount function that calls the setSelf function on mount.
2. They are of type WriteableAtom. In other words, they have a write function.

This edge case may cause infinite loops in nested atomEffect dependencies.

Consider the following example:
https://codesandbox.io/p/sandbox/pedantic-forest-sckny6?file=%2Fsrc%2FApp.tsx%3A41%2C12

[dmaskasky]

Part of the issue is that the atomEffect's EffectFn runs on next tick. So in the current tick, atoms whose only mount point is from inside the atomEffect will unmount on current tick and remount on next tick when the EffectFn runs again. This seems to only affect Writeable Atoms that have a mount function where the setSelf function is called on mount.

One possible solution is to store a set of all atoms "seen" in the synchronous execution of the EffectFn. Then, to apply get to each of these atoms on next evaluation of the EffectFn.

There are a few issues with doing this. First, its a perf hit to unnecessarily fetch atom values only to ensure they stay mounted. Second, its a perf hit, because we are keeping references of atoms outside the weakMap. Third, bistable atom dependencies are not properly handled.

atomEffect((get, set) => {
  if (get(boolAtom)) {
    get(atomA)
  } else {
    get(atomB)
  }
})

When boolAtom is true, the atom dependencies are [boolAtom, atomA]. When bool atom is false, the atom dependencies are [boolAtom, atomB]. If we assume the previous runs atom dependencies are valid in the current run, then the following happens:

1. boolAtom is true: [boolAtom, atomA]
2. boolAtom is false: [boolAtom, atomA, atomB]
3. boolAtom is true: [boolAtom, atomA] // atomA unmounts in current tick and remounts in next tick

[dmaskasky]

Open Questions:

1. Why does this issue only affect writeable (non-primitive) atoms that have an onMount function, that calls setSelf onMount?

[dmaskasky]

Related PR: #23

[dai-shi]

#21 (comment)

atomEffect returns a promise.

Actually, it doesn't.

I just added return, but it doesn't help...
https://livecodes.io/?x=id/d5tfh2vjegc

[dai-shi]

I'm not sure what you are suggesting to keep the atoms mounted while the promise resolves.

I thought it would be mounted after the promise resolved.

[dmaskasky]

the atoms are unmounted on current tick, and remounted on next tick when EffectFn fires. I noticed that only writeable (non-primitive) atoms with an onMount function where the setSelf is called are affected.

i.e

const countAtom = atom(0, (get, set) => {})
countAtom.onMount = (setSelf) => {
  setSelf()
}
const effect = atomEffect((get, set) => {
  get(countAtom) // unmounts when countAtom changes, mounts on next tick when effectFn is called
})

fine

const countAtom = atom(0)
countAtom.onMount = (setSelf) => {
  setSelf()
}
const effect = atomEffect((get, set) => {
  get(countAtom) // fine
})

fine

const countAtom = atom(0, (get, set) => {})
countAtom.onMount = () => {}
const effect = atomEffect((get, set) => {
  get(countAtom) // fine
})

fine

const countAtom = atom(0, (get, set) => {})
const effect = atomEffect((get, set) => {
  get(countAtom) // fine
})

[dai-shi]

That sounds interesting.
I wonder if is has something to do with something else.
I mean, do you understand everything, and it's working as expected?

[dai-shi]

init(null); // <-- necessary to reproduce behavior

I missed it. Hm, interesting. It feels a bit unusual use case though. We usually create two atoms.
But anyway, can it be a bug in Jotai core? It feel like we can reproduce the behavior without jotai-effect, or?

[dai-shi]

#21 (comment)

This issue is reproducible outside React.

Outside React, store.sub(...) is the way to mount an atom.

[dmaskasky]

I found a workaround for preventing infinite loops with nested atomEffects. Its a bit counter-intuitive, but instead of calling setSelf on mount, its called on next tick from the initAtom's read fn. Although the remounting bug still exists for other atoms that have an onMount property.

const initAtom = atom(
    (get, { setSelf }) => {
      const ref = get(refAtom)
      if (ref.mounted) return
      Promise.resolve().then(() => {
        setSelf(true)
      })
    },
    (get, set, mounted: boolean) => {
      const ref = get(refAtom)
      ref.mounted = mounted
      if (mounted) {
        set(refreshAtom, (c) => c + 1)
      } else {
        ref.cleanup?.()
        ref.cleanup = undefined
      }
    }
  )
  initAtom.onMount = (init) => () => init(false)```

[dmaskasky]

This issue has been fixed by jotai-core! 🥳

❌ Failing example with [email protected], [email protected]: https://codesandbox.io/p/sandbox/pedantic-forest-sckny6

✅ Fixed example with [email protected], [email protected]: https://codesandbox.io/p/sandbox/bold-andras-crwwmv