Problems with derived atom which has a promise as its initial value

[hirokibeta]

I played around with the codesandbox in this issue #1054 (comment) to understand the behavior when the atom has Promise as its initial value.

Then, I have noticed that this pull request #1089 does not solve the problem in many cases, and that even in cases where it appears to solve the problem, there is a another problem.

Cases where the problem is not resolved

This is just one atom added to the first codesandbox. Now the atomWithSuspense no longer works.

https://codesandbox.io/s/vigorous-roman-45dbx5?file=/src/App.js

After digging deeper into the issue, I realized that this behavior was the expected behavior.

Why this codesandbox is the expected behavior

The reason why atomWithSuspense does not work when the initial value is a infinite Promise is because it is not registered in the Dependents of nullableAtom.

Normally, a derived atom is registered in Dependents of dependencies at store[SUBSCRIBE_ATOM] in useAtomValue, but if initial value is a Promise, it is not executed because throw Promise occurs before that. So, atomWithSuspense will not registered in the Dependents of nullableAtom and will not work.

Why does the first codesandbox still work

Basically, the only chance for a derived atom to be registered in dependencies' Dependents is when it is mounted for the first time. However, there is one more place where this can happen. That is flushPending.

Inside flushPending, atoms registered in the pendingMap are registered in dependencies' Dependents under certain conditions. Note that atoms that are not mounted are also registered in the pendingMap. Atoms are registered in the pendingMap when they are first obtained, so atomWithSuspense, which is not mounted, is also registered in the pendingMap.

The conditions under which atoms are registered as Dependents are below:

• The dependencies of the atom must be changed from the last time
• The dependencies must have been mounted

For this to work, nullableAtom must be mounted before the first flushPending, as in the first codesandbox. In other words, nullableAtom must be the first atom to be mounted.

What is a another problem in cases where it works

In the first codesandbox, atomWithSuspense was registered in Dependents of nullableAtom as stated above. However, if SuspenseViewer is detached without ever clicking the "Fetch new value" button, atomWithSuspense is not unmounted because it was not mounted, and it is not removed from the Dependents of nullableAtom. Therefore, atomWithSuspense continue to be retained even if it is no longer referenced by anyone. This problem occurs even if the initial value is not an infinite promise.

[hirokibeta]

It seems like this pull request fix(core): handle returning infinite promise in derived atoms #1089 created another issue.

Issue

A loadable atom of a derived atom trigger infinite loop under the following conditions.

• the value of the derived atom is Promise
• the set function of an atom on which the derived atom depend is triggered without changing its value

Reproduction

A third click within 5 seconds of the second click will trigger infinite loop.

https://codesandbox.io/s/jovial-firefly-bhb2s2?file=/src/App.tsx

How this happen

The infinite loop is occurred in writeGetter function as below.

• The loadable atom get the value of derivedAtom using writeGetter with { unstable_promise: true }
• If the Promise is resolved normally, setAtomPromiseOrValue will set the resolved value, but if it is resolved by cancelSuspensePromise, this will not happen.
• Even if resolved by cancelSuspensePromise, the derivedAtom is recalculated in case the value of dependencies are changed.
• If neither of the above is the case, writeGetter is repeated recursively, resulting in an infinite loop.

[dai-shi]

A loadable atom of a derived atom trigger infinite loop under the following conditions.

Too bad we didn't have a test for it. I felt a little too naive when I did #1089.

I haven't followed all of your comments, but seems like you can help the project, by creating failing test cases and even open a PR to fix them.

[hirokibeta]

I created a pull request (#1118) to revert #1089 to solve infinite loop.
Not only was #1089 unable to solve infinite promise issue in derived atoms, it was also the cause of the infinite loop.

I also added a test for a infinite loop in this loadable case. I don't think this test is comprehensive enough, but we need to fix this infinite loop for now.

"Not comprehensive enough" means that writeGetter with { unstable_promise: true } has the risk of potentially infinite loops as below, but I don't feel that this test adequately covers that.

• writeGetter with { unstable_promise: true } is executed recursively if the value is a promise.
• This design assumes, but does not guarantee, that when writeGetter is executed after the resolution of a promise value, it will get a non-promise value through readAtomState.

[dai-shi]

I will take a look later. Do you mean we can't fix #1054? Or, do you have any other idea to fix?

[hirokibeta]

So far, I can't think of a solution that doesn't have problems.

If we update readAtom like below and keep #1089, we can support #1054 and all tests will be passed with it.

const readAtom = <Value>(
  readingAtom: Atom<Value>,
  version?: VersionObject
): AtomState<Value> => {
  const atomState = readAtomState(version, readingAtom)
  addAtom(readingAtom) // add this line
  return atomState
}

But this change might cause a problem as I mentioned in the last part of the first comment. I'm not 100% sure so I want to ask for yor insight.

[dai-shi]

Hi, I just followed your comments, and see that you mean #1054 is an expected behavior in the first place. That's the first thought of mine too, but then #1089 was something that passed all the tests...

Okay, I agree that it was a wrong fix, so let's revert it.

[hirokibeta]

I am glad you agree. And yes, I understand that this is a tricky bug.

I also understand that the pattern in #1054 is useful. Here is a workaround to use a derived atom, which has Promise as its initial value, in case somebody else comes across this issue. This has a sort of watcher atom to keep the derived atom mounted.

https://codesandbox.io/s/drived-async-atom-workaround-gpsi4d?file=/src/App.js

I'm excited to use Jotai for my next project! Thank you very much for this wonderful work.