urql-graphql / urql

The highly customizable and versatile GraphQL client with which you add on features like normalized caching as you grow.
https://urql.dev/goto/docs
MIT License
8.65k stars 454 forks source link

Suspense: 'network-only' fetches twice #1243

Closed StevenLangbroek closed 3 years ago

StevenLangbroek commented 3 years ago

Hey folks!

Opening an issue after a long chat with @kitten. When suspense is enabled, and you set a hook or component to network-only, the request fires twice.

Phil said this is expected behavior given the current implementation (which makes sense, the component is rendered twice, once before suspending and then again after the initial request resolves). That being said; I think this will still surprise folks when they migrate to suspense in the future, and should be considered a bug I think.

urql version & exchanges:

Steps to reproduce

https://codesandbox.io/s/gifted-bohr-honuf?file=/src/App.tsx

Expected behavior

The request only fires once

Actual behavior

The request fires twice

Is there anything else I can provide or explain? Here to help 🙏

kitten commented 3 years ago

Just leaving a note here for debugging, as discussed, with network-only this is likely expected behaviour and not avoidable, but this behaviour also happens with cache-and-network

StevenLangbroek commented 3 years ago

@kitten so I think I mentioned this, but if useState values can be persisted during suspense, can't we hang on to a reference somehow to know we shouldn't make this request again?

See https://codesandbox.io/s/pedantic-volhard-8fvbu

kitten commented 3 years ago

My concern is more with cache-and-network and effects. We can't hang on to the effect and our state is computed. So it's unfortunately not a matter of hanging on to state because we can't accurately determine and predict what lifecycle we're currently following (which is intended on React's part) and we also won't know when and how data has or will change from a Client perspective.

In other words, detecting Suspense and predicting what to do after won't do us any good when we can't tell what component we're returning to. Concurrent makes this situation even harder since we can't tell if and when a component is active again; for all intents and purposes it'll be a "new instance". Lastly having state around still or not won't either do us any good if we need the query stream to become active (while it can't stay active during an interrupt) when the component "resumes"