jotaijs / jotai-scope

MIT License
55 stars 4 forks source link

Unstoppable initializer function call when using async atom with ScopeProvider #12

Closed yf-yang closed 10 months ago

yf-yang commented 10 months ago

Reproduction: https://codesandbox.io/s/flamboyant-margulis-2rczql?file=/src/App.js

If I got it right, the problem is here: https://github.com/pmndrs/jotai/blob/247d9b1a0d6b3ffd018df5a82e4ab87bdea1c07f/src/react/useAtomValue.ts#L106

Not so sure about the root cause. Each time the previous promise resolves, when calling useAtom again by React, another new promise is returned. However, I still don't know why the init function get called again.

dai-shi commented 10 months ago

https://jotai.org/docs/guides/async#suspense

If you have a <Provider>, place at least one <Suspense> inside said <Provider>; otherwise, it may cause an endless loop while rendering the components.

It applies to ScopeProvider too.

https://codesandbox.io/s/twilight-sun-2pwlsg?file=/src/App.js

yf-yang commented 10 months ago

What about nested provider 🤔 https://codesandbox.io/s/flamboyant-shannon-zd4stg?file=/src/App.js

dai-shi commented 10 months ago

I think we always need Suspense right under Provider.

yf-yang commented 10 months ago

Can you take a look of the example's console? It works but I got thousands of function calls before it successfully renders.

yf-yang commented 10 months ago

Also, I am thinking about another stuff yesterday.

First of all, let me clarify an idea: If I create a atom with a read function (either sync or async) that does not depend on any other atoms, then can this atom be written? If not, then in this case the atom acts as a dependency injection tool instead of a state management tool, right?

Then, the problem comes back again: we need to avoid calling read functions many times of an un-scoped atom, especially when the read function is heavy.

I am not sure if there exist patterns that can actually bypass the problem, i.e. call the heavy part only once, then return another atom which is scoped.

yf-yang commented 10 months ago

Another approach would be, when calling useAtom, adjust the order of making copy of atom and building dependencies. I mean, first detect dependency (edges in the graph) to check if current atom needs to be scoped, if so, then actually make a copy. I don't know if this can be truly implemented. Just an idea.

yf-yang commented 10 months ago

Thoughts:

The equivalent of

const initializer = () => { return someHeavyWorks() };
useState(initializer);

for globally unique atom, it should be:

const initializer = () => { return someHeavyWorks() };
const anAtom = atom(initializer());
// not: const anAtom = atom(initializer);

For initialize atom in component, it should be

const initializer = () => { return someHeavyWorks() };
useHydrateAtoms([[anAtom, initializer]]);

Therefore, my suggestion 1 would be allow function parameter in useHydrateAtoms.

However, this one does not solve the async atom problem. It is similar to

const initializer = () => { return someHeavyWorks() };
const anAtom = atom(initializer);

Once the atom or the atom's children get covered by a ScopeProvider, although they are not scoped, the initializer get called multiple times.

yf-yang commented 10 months ago

However, this one does not solve the async atom problem

Forget about the analysis of async atoms. I find myself not clearly get the meaning of async atoms. Let's focus on the behavior of this example: https://github.com/jotaijs/jotai-scope/issues/12#issuecomment-1780371475 and the suggestion to support initialize function in useHydrateAtoms.

yf-yang commented 10 months ago

What about nested provider 🤔 https://codesandbox.io/s/flamboyant-shannon-zd4stg?file=/src/App.js

I realize the problem is with React.use implementation (18.2.0 has not exposed it yet, so polling is used to determine if the promise is resolved.). Upgrading to 18.3.0 latest build fixes the issue.