jaydenseric / graphql-react

A GraphQL client for React using modern context and hooks APIs that is lightweight (< 4 kB) but powerful; the first Relay and Apollo alternative with server side rendering.
https://npm.im/graphql-react
MIT License
718 stars 22 forks source link

Allow configuration for rehydration timeout in useGraphql #37

Closed quantizor closed 5 years ago

quantizor commented 5 years ago

https://github.com/jaydenseric/graphql-react/blob/58e1690c2dbee19f095b40b42b9e92a8c785e08b/src/universal/useGraphQL.mjs#L179

For server-rendered pages that take a little longer to load, it'd be nice to be able to say for a particular GQL query that for example 5 seconds is acceptable for total load time... or perhaps infinity (takes as long as it takes.)

jaydenseric commented 5 years ago

That 500ms limit should work just as effectively regardless of how long the SSR response takes; it only needs to cover how long it takes for the whole app and all nested queries to mount at first render, after all the JS has been downloaded, parsed, and then run on the client.

If the server response takes a long time (say, 6 seconds), the 500ms only kicks in afterwards, when the client app mounts.

If the server response is quick, but the app/route JS bundles take a long time to download and parse (say, 10 seconds), again the 500ms only kicks in after that work has completed.

Are you noticing some issues with post SSR hydration, or were you just pre-empting a possible issue?

quantizor commented 5 years ago

Hmm ok, yeah I'm noticing on one of our heavier pages the 500ms limit is exceeded and it requeries. I guess it just takes a little longer to rehydrate than expected...

jaydenseric commented 5 years ago

Do you think you will be able to investigate why it takes so long for some parts of your app containing queries to mount, compared to the rest of the app?

There is a tradeoff that if you make the hydration time too long, it might stop legitimate client queries form loading. For example, a user might quickly click on nav link and the new route is supposed to load on mount.

quantizor commented 5 years ago

I think it's a combo of styled-components rehydration and some third-party JS slowing things down that I don't have a lot of control of... Is it possible to just configure the rehydration timeout per instance of useGraphql?

mike-marcacci commented 5 years ago

@probablyup it sounds like something asynchronous is happening between when react initially mounts with your GraphQLProvider, and when your component with useGraphQL is mounted – perhaps you have a waterfall of requests going on? This is probably not a typical scenario.

That said, I believe you can get the kind of behavior you want by setting loadOnMount to false until you are sure your page is ready.

EDIT: This won't work, as it will trigger a re-render when you switch loadOnMount back to true.

jaydenseric commented 5 years ago

If the first mount takes longer than half a second something funny must be going on that needs to be investigated, and hopefully fixed – renders usually take milliseconds. It seems Styled Components has had performance issues before, so it could be something like that?

The problem with making the timeout longer in the component is that it is not the component itself that takes a long time to mount, but rather it's context amongst ancestors is the problem.

For example, imagine you have a profile component with a query that is used in two pages. In one page, it is deeply nested in strangely slow mounting components. In the other, the mount is speedy and not a problem. Lengthening the hydration timeout within the component to suit the first page would would unsuitably lengthen the timeout for the other page.

Note that if a query that attempts to load on mount within the hydration time that does not have a cache entry yet, it will still fetch data. So I suppose my earlier example of a user quickly navigating to a new route is not a very good one, since that would still fetch as the doesn't doesn't exist in cache yet. Perhaps a better example is a user clicking on a button to refresh a price ticker that already has a cache entry from SSR. Or a button that queries and displays a random quote would show the same quote again and not fetch a fresh quote if clicked quickly within the hydration timeout.

jaydenseric commented 5 years ago

Honestly I'm clutching at straws trying to think of realistic edge-cases where lengthening the 500ms default to something like 1s would cause problems. My last examples are not strictly right either, because if the button just calls load() then it will definitely fetch fresh data even if it's within the hydration time. The hydration time only applies to loading on mount, so an edge case would involve the user attempting to mount something very quickly that already has a cache entry, where you might reasonably expect the data to have changed in such a short period of time.

Would increasing the arbitrary, hardcoded 500ms value to 1000ms solve most of the issues?

quantizor commented 5 years ago

I’d make it even looser, if you take mobile devices into account. Perhaps 5s, though hopefully no one is building apps that take that long to rehydrate.

jaydenseric commented 5 years ago

Adding and documenting an extra option that takes a lot of technical knowledge to understand is not ideal.

I've increased the limit to 1000ms to cover most slower clients.

A little extra loading is no big deal for clients that take a very long time to hydrate; in that situation fixing the root issue, slow hydration and initial render, will also fix the extra loading.