-
Notifications
You must be signed in to change notification settings - Fork 3
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
Atomeffects never run while read within a suspended suspense boundary #41
Comments
May I know some more specifics of your use-case? The example you provided is fairly generic. Hopefully for your specific use-case we can come up with an acceptable workaround to this issue. The issue you describe above is a result of how hooks behave with suspense in React. More details on this behaviorBecause Here is your example extended to demonstrate that Understanding why the atomEffect never reads even when it is mounted before When the component is first rendered
|
mmm i think i understand, my mental model would be that the jotai would suspend after finishing all atom evaluations but this makes a lot of sense actually. (also your sandbox fork isn't public but i believe you!) Basically my use case is to have the atomEffect set up a subscription and then set the result of that subscription to a primitive atom. The primitive atom should start out as an unresolved promise and then resolve on the first subscription callback, then it can be set to synchronous values there after. (This is my attempt to build my own jotai-urql query hook because the library version uses atomWithObservable and atomwithObservable creates infinite loops and permanently suspended suspense boundaries for us in combination with jotai-scope sadly.. we were talking about it here: jotaijs/jotai-scope#25 (comment)) |
oops, ok its public now. |
I don't know for sure, but I suspect that jotai will migrate to React 19's const asyncAtom = atom(async (get) => fetch(url))
function Component() {
const result = use(useAtomValue(asyncAtom))
}
You could put the effect in a parent component that doesn't suspend. Tbh, figuring out where to register the atomEffect feels like an unnecessary and annoying part of the api. But since atoms are just definitions and an atom + store represents a reactive value, it makes the most sense to declare your effects under the jotai provider, or as close to the store as possible. const valueAtom = atom(new Promise())
const subscriptionEffect = atomEffect((get, set) => {
const unsubscribe = subscribeToUrql((data) => {
set(valueAtom, data)
})
return unsubscribe
})
function MyComponent() {
const value = useAtomValue(valueAtom)
}
function ParentComponent() {
useAtom(subscriptionEffect)
return <Suspense fallback={<>Loading...</>}>
<MyComponent />
</Suspense>
} |
Anyways this makes sense right? If a component is suspended, then stuff inside should be suspended too. In fact, I believe the negation, _if a component is suspended, then stuff inside should not be suspended", would be incorrect behavior. My recommendation is to place your effects as close to the store root as possible. Anyways, closing this ticket for now. Let me know if you would like to discuss further, and I can reopen the ticket if necessary. Also feel free to reach out to me on Discord if you would like to chat more realtime. |
Ya i think you're right that since this mirrors a useEffect the behavior is correct. In terms of putting subscriptions near the store that would sorta defeat the purpose. The whole idea of the urql atom is to have an atom that queries and reacts to the urql cache whenever it's mounted as a dep and not otherwise. That link gets lost if we have to move the subscription away from the atom. I'll have to think a bit harder :) Thanks for all the thoughts! |
Another idea is to do |
One more idea is to hold reading the valueAtom until the effect has mounted. const baseValueAtom = atom(new Promise())
const effectReadyAtom = atom(false)
const subscriptionEffect = atomEffect((get, set) => {
set(effectReadyAtom, true)
const unsubscribeURQL = subscribeURQL((data) => {
set(baseValueAtom, data)
})
return () => {
unsubscribeURQL()
set(effectReadyAtom, false)
}
}
const valueAtom = atom((get) => {
get(subscriptionEffect)
if (get(effectReadyAtom)) {
return get(baseValueAtom)
}
return null;
}) |
Ya that's what I have actually but I'm wanting it to start out in suspense and not have Appreciate all these ideas! this is one approach that works but it warns that I shouldn't be setting self synchronously ha export function makeUrqlQueryAtom<Data, Variables extends Record<string, any>>({
scope,
getVariables,
query: document,
getPaused,
}: {
query: DocumentInput<Data, Variables>;
getVariables: (get: Getter) => Variables;
scope: <A extends Atom<any>>(atom: A) => A;
getPaused?: (get: Getter) => boolean;
}) {
type DataResult = {
data?: Data | undefined;
};
const lastResultAtom = scope(atom<null | DataResult>(null));
const variablesAtom = scope(atom(getVariables));
const memoizedSubscriptionAtom = atom<
DataResult | Promise<DataResult>,
[DataResult | null],
void
>(
(get: Getter, { setSelf }): Promise<DataResult> => {
setSelf(null);
const source = get(urqlClientAtom).query(document, get(variablesAtom));
source.subscribe(setSelf);
return source.toPromise();
},
(get, set, update: DataResult | null) => {
set(lastResultAtom, update);
}
);
const urqlQueryAtom = atom((get): DataResult | Promise<DataResult> => {
if (getPaused?.(get)) {
return { data: undefined };
}
const subscriptionPromise = get(memoizedSubscriptionAtom);
const lastResult = get(lastResultAtom);
return lastResult ?? subscriptionPromise;
});
return urqlQueryAtom;
} |
nice!
Oh yeah, you need to put that setSelf in a setTimeout or queueMicrotask. In production, I think it will error or noop (I don't remember which). |
Ya the problem is that it needs to be synchronous to be correct or else the atom will return the previously cached result for the incorrect variable input |
May I recommend jotai-history for your use-case? const targetWithPrevious = atomWithHistory(targetAtom, 2)
You can do synchronous set during read like this, but it does require onMount to be called at least once. type PromiseOrValue<T> = T | Promise<T>
export function makeUrqlQueryAtom<Data, Variables extends Record<string, any>>({
scope,
getVariables,
query: document,
getPaused,
}: {
query: DocumentInput<Data, Variables>
getVariables: (get: Getter) => Variables
scope: <A extends Atom<any>>(atom: A) => A
getPaused?: (get: Getter) => boolean
}) {
type DataResult = { data?: Data | undefined }
const lastResultAtom = scope(atom<null | DataResult>(null))
const variablesAtom = scope(atom(getVariables))
const refreshAtom = atom(0)
const refAtom = atom(
() => ({ queue: [] }),
(get, set) => {
const ref = get(refAtom)
ref.set = set
ref.isMounted = true
set(refreshAtom, c => c + 1)
ref.queue.forEach((data) => set(lastResultAtom, data))
ref.queue.length = 0
return () => {
ref.isMounted = false
}
)
refAtom.onMount = (setSelf) => setSelf()
const memoizedSubscriptionAtom = atom<PromiseOrValue<DataResult>>(
(get) => {
get(refreshAtom)
const ref = get(refAtom)
if (!ref.isMounted) return // <--- synchronous set does require refAtom.onMount to fire
const { set } = ref
set(null) // <-- does this have to be synchronous?
const source = get(urqlClientAtom).query(document, get(variablesAtom))
source.subscribe((data) => {
const ref = get(refAtom)
if (!ref.isMounted) {
ref.queue.push(data)
return
}
set(lastResultAtom, data)
})
return source.toPromise()
}
)
const urqlQueryAtom = atom((get): DataResult | Promise<DataResult> => {
if (getPaused?.(get)) {
return { data: undefined }
}
const subscriptionPromise = get(memoizedSubscriptionAtom)
const lastResult = get(lastResultAtom)
return lastResult ?? subscriptionPromise
})
return urqlQueryAtom
} |
I am investigating making jotai-effect synchronous. I cannot guarantee it will be feasible, but I'm looking into it. |
Closing this issue for now as the original question has been answered, this is expected behavior for atomEffect. |
See the example here:
https://codesandbox.io/p/sandbox/jotai-effect-on-async-r2s4jl?file=%2Fsrc%2FApp.tsx%3A27%2C50
The text was updated successfully, but these errors were encountered: