Skip to content
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

doesn't seem to recalc the atom on cache updates #18

Closed
scamden opened this issue Feb 22, 2024 · 8 comments
Closed

doesn't seem to recalc the atom on cache updates #18

scamden opened this issue Feb 22, 2024 · 8 comments

Comments

@scamden
Copy link

scamden commented Feb 22, 2024

Repro:

  1. Make an atomWithQuery
  2. useAtom on your query atom
  3. Have some other UI update the graphcache entry that the query returns

Expected:
that atom updates with the new value

Actual:
No update

cc: @juliette-pouchol

@RIP21
Copy link
Collaborator

RIP21 commented Feb 23, 2024

Hey, we would need an actual code snippet, or, better a reproduction.
Info given isn't enough to triage it.

@scamden
Copy link
Author

scamden commented Feb 23, 2024

Well I guess the first question would be: is this expected to work? I can work on a repro but I thought I would see if this was obvious to you first. Cache updates are pretty basic urql

@RIP21
Copy link
Collaborator

RIP21 commented Mar 2, 2024

It should work, it subscribes to the changes of the items that are in the query.

@scamden
Copy link
Author

scamden commented Mar 27, 2024

i'll work on nailing down a repro soon then

@scamden
Copy link
Author

scamden commented Mar 28, 2024

so i finally painstakingly put together our use case in codesandbox: https://codesandbox.io/p/sandbox/urql-client-side-suspense-demo-81obe?file=%2Fsrc%2Findex.js%3A22%2C44

And it didn't repro 🤦

I dug harder in our code and it turns out we weren't using the same client, because we were using useHydrateAtoms([clientAtom, client]) but our client gets recreated and memoized on a couple props. Changing to both hydrate and set the atom in a useEffect fixed this. Sorry to take up attention, but leaving our root cause and fix here in case anyone else runs into this.

Thanks again for this great library, which does not have this bug :D

@scamden scamden closed this as completed Mar 28, 2024
@RIP21
Copy link
Collaborator

RIP21 commented Mar 29, 2024

@scamden Yay! Pretty happy my work (and a lot of tests) did pay off :D

Happy hacking! Also, be aware that Next + Suspense may cause some weird freezes and hangouts of the entire page on route change that irrecoverable. But only in production build and with old router.
At least, once, I had this problem and I didn't manage to solve it for weeks. Only solution was to revert the changes and keep using 'loading' and stuff like that.

But I hope you'll never run into that problem :)

@scamden
Copy link
Author

scamden commented Mar 29, 2024

(and a lot of tests)

love this.

Also, be aware that Next + Suspense may cause some weird freezes and hangouts of the entire page on route change that irrecoverable.

oh dang! good to know! I have seen some "stuck in suspense" things, but so far the fix was to put a suspense boundary between the nearest jotai-scope provider and the suspending atom. This frightens me that this happened to you only in production though and I love the ergonomics of suspense so I don't want to stop lol.

Any more detail on when this happened to you? What version of next? was it a route change during an urql query?

@RIP21
Copy link
Collaborator

RIP21 commented Mar 29, 2024

@scamden to be completely honest I don't remember details, but it was super weird and it didn't work either it was suspended by URQL, or was suspended by just an async atom. And even a different sequence of visiting the routes was affecting it. If you do it in the 1 - 2 - 3 sequence it freezes at 3. But if you do 1 - 3 - 2 it does not :) So it was pretty weird. It was on Next 12 AFAIR.

So yeah, weird stuff :) Maybe with the app router it's no longer a thing, but for a page router it was there and it was a major PAIN as I had to revert 3 weeks of work :P

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants