Open jet2jet opened 1 year ago
I don't think it's good practice anyways? Should we just issue one GraphQL query? But, I'm interested in the fix. Didn't look into closely yet, but it looks something.
@RIP21 what do you think?
The reproduction is a simplified pattern for confirming the bug, but I think there is a case that the query needs to be splitted due to the performance (query cost, speed (e.g. display partial data as fast as possible), server load balancing, dividing authed and non-authed query, etc.).
In this case, nesting (?) atomWithQuery
would be necessary.
we definitely have separate queries using these. And I think we are running into this bug as well. We intermittently get an infinite spinner on our nested query. cc @fractal1729
If the suggested change above works while also passes all the test cases that are there. If somebody tries a changed code against our test suite and then opens PR (and preferably adds a test case with that case) I'll gladly merge it and publish.
However, my gut feeling is that most of the waterfall queries should be sorted by adding a field resolver on the first query that will use the result of the first query to resolve the second query in one query. But, surely, it's not always possible :)
However, my gut feeling is that most of the waterfall queries should be sorted by adding a field resolver
i'm sure there are perf benefits to this but it's not always convenient to structure the code that way (maybe with relay it would be easier :P ). I'll do a little thinking to see if we could build that way instead though in the meantime
@dai-shi i for one would love a clear answer on whether this is safe as well!
is it OK to use atom generators inside the atom getter function?)
Yes, it's a valid and supported hack.
Hi again, I think we are experiencing another related issue here. I'm sorry I don't have a simple reproduction because it seems to be a tricky and complex race condition and only reproduces in our production environment, but it's not the first time I've seen something like this. I'll describe what we've seen in hopes that someone might have an idea on how to workaround or fix:
The basic problem is that we see an infinite render loop. Facts:
Could it help not to create new atoms in the read functions as described above?
If I can't get this root caused and solved I think we'll have to stop using the jotai-urql library and resort to syncing normal urql hooks to the atoms which is a huge bummer so any help here would be much appreciated.
Other things i've noticed: Also the atomWithObservable wraps the urql result in a brand new object. The urql operation result is referentially stable but since the observable's subscribe wraps it i think that forcible recomputes at least some set of the atoms downstream (at some point the operation result is pulled off that object though so I would hope Jotai would stop recomputing at that point, but I think since many of these are promise based they get a new Promise with a new reference regardless.)
cc @bryanjclark @ehzhang
Description
When using
atomWithQuery
with an atom created withatomWithQuery
(andloadable
) (*), it doesn't seem to work properly.(*) using the atom in
getVariables
andgetPause
Reproduction
npm install
npm run start
Detailed
buttonNormal
button andDetailed
button for some times, and detailed list will stayloading
stateOthers
I wonder the following block creates an atom inside the atom getter function.
https://github.com/jotaijs/jotai-urql/blob/v0.7.1/src/common.ts#L43-L64
If I modify the code like followings, it seems to work properly. (Is it OK to use atom generators inside the atom getter function?)
Modified code
```ts const observableAtom = atom((get) => { const args = getArgs(get) const client = getClient(get) const source = getPause(get) ? null : execute(client, args) if (!source) { return null } return pipe(source, toObservable) }) // Note: if the function for `initialValue` accepts `get`, these atoms could be combined into one atom const resultAtomWithSuspense = atomWithObservable# FYI: jotai's
atomWithObservable
also has calling an atom generator function inside the atom getter function:https://github.com/pmndrs/jotai/blob/v2.4.3/src/vanilla/utils/atomWithObservable.ts#L137