Apollo-client: Synchronous Local-State Resolvers Don't Return Data Synchronously

Created on 7 Apr 2020  Ā·  10Comments  Ā·  Source: apollographql/apollo-client

Intended outcome:

Admittedly, this might fall into feature request territory, but here goes.

I have a local state resolver that returns something synchronously. Whenever I go to query it, though, it always returns data: undefined at first then an instant later returns the actual data.

I suspect that this is because Apollo will always await the result of a local resolver, even if it returns a non-thennable. This mandates an event loop tick and thus the component is rendered with an initial value of null.

Actual outcome:

See above.

How to reproduce the issue:
https://codesandbox.io/s/quizzical-firefly-oyxwn?file=/src/App.js

Open the console and see that component is initially rendered with appVersion: null.

Versions
See reproduction. :^)

Most helpful comment

I appreciate your feedback, and I respectfully disagree. Your code snippet is a decent workaround but is also a fair amount of boilerplate. It also makes it hard to use hooks that depend on the data returned due to the early return rule.

const { data, loading } = useQuery(...);
if (loading) return null;
if (!data) throw new Error(...);

// Uh-oh, can't do this because of early return above
const foo = useFoo(data.bar);

One "solution" to _that_ problem is allowing useFoo to take undefined, but that just starts to infect code like wildfire and is still just a workaround because data that is known synchronously isn't returned synchronously. The workaround beyond that (which I do actually use in some places) is to have a presentational component that is rendered once all of the data is available, which I do in some places, but is even more boilerplate and redundant types I have to specify in my TS code. It feels very much like going back to the old style of Redux, which isn't even recommended anymore (see edit at top).

I adopted Apollo to simplify my state management, and I'm simply advocating for a feature that I think would make it even better.


All that being said, I'd appreciate input from an Apollo contributor rather than disagreeing about the merits of a feature with a stranger. :^)

All 10 comments

This is definitely intended. The whole point in using apollo and graphql is that you can't know if your query requires going to a server or fetching the data from cache, or if the query is asynchronous or synchronous.

You as the application developer are supposed to always write a fallback for when that data doesn't exist. If the data is local, the user will probably never see this fallback state because the data should be fetched and rendered before the DOM had a chance to actually draw the fallback result.

You will find this is a better approach to application development, as it means you are not having to think about your data fetching differently for when it's local data or serverside data, it's all just data.

All valid points, but not my use case.

In particular, I'm putting a bit of data inside of Apollo just for simplicity and I'd rather it be available synchronously. I could do workarounds with state and context (or... god forbid... Redux), but I'd much rather be able to manage it all from inside Apollo. I don't want to write fallbacks that I know I won't need.

What you are asking for would break the simplicity of the Apollo API hooks api. If this was type checked, how would the query hook function know that your query will always return data straight away? How does it know that you have actually implemented a resolver for that data?

A fallback can be as simple as if (!data) { return null } which will just not show the component if there is no data, I can't imagine a scenario using react hooks where this isn't possible to deal with.

What you are asking for would break the simplicity of the Apollo API hooks api.

It really wouldn't. I'm not asking for some different API, just a different way of handling synchronous resolvers internally. The API is still const { data, loading, ... } = useQuery(...) which is itself completely agnostic to whether or not data is returned synchronously (which is currently the case when using a cache) or asynchronously.

If this was type checked, how would the query hook function know that your query will always return data straight away?

It wouldn't. It would be up to the app code to do that if it so intended.

const { data } = useQuery(...);
if (!data) {
  throw new Error(`Failed to load data!`);
}

// use data

How does it know that you have actually implemented a resolver for that data?

I'm not sure what "it" is here, but this would actually enable a more "fail-fast" mechanism since we now _know_ that if the data isn't returned synchronously, then we can throw an error.

A fallback can be as simple as if (!data) { return null } which will just not show the component if there is no data, I can't imagine a scenario using react hooks where this isn't possible to deal with.

It possible to, and I have, run into bugs where you have wonky UI's because your data fails to fetch and your app is in an unexpected state. I want to be able to write invariants so that we fail fast if we detect an "impossible" state (like some local data being unavailable).


I think there are some reasons why I think this might not be feasible (and am open to input from the people that actually work on Apollo client). The main would be increasing the complexity of the Apollo's internals (I'm curious as to if this would actually happen since _cached_ data is already returned synchronously). But I'm happy to have that discussion. :^)

The API is still const { data, loading, ... } = useQuery(...)

You are right, this wouldn't actually break the API. For this I think it's totally feasible that the data could be returned instantly, but why is my main issue.

It would be up to the app code to do that if it so intended.

If this is what you are suggesting, then I see no reason you can't use just the loading variable to throw your invariants if the data never came back when it 100% should come back:

const { data, loading } = useQuery(...);

if (!data && loading) {
  return null
}

if (!data && !loading) {
  throw new Error(`Failed to load data!`);
}

// use data

This way you know your data should be there.

My reasoning for suggesting it as "breaking the simplicity" is because it's not expressible as types in the generic useQuery hook, due the output data having to be T | undefined as it is not possible for the type system to deduce if your query is synchronous or asynchronous.

The only option is for apollo to provide a useQuerySync hook which would, as your want, throw an error if the data is not instantly available. I just don't see a good reasoning behind this, the above solution achieves the same result. The only issue, if you really care about it, is the above solution might take 3 renders to achieve.

I appreciate your feedback, and I respectfully disagree. Your code snippet is a decent workaround but is also a fair amount of boilerplate. It also makes it hard to use hooks that depend on the data returned due to the early return rule.

const { data, loading } = useQuery(...);
if (loading) return null;
if (!data) throw new Error(...);

// Uh-oh, can't do this because of early return above
const foo = useFoo(data.bar);

One "solution" to _that_ problem is allowing useFoo to take undefined, but that just starts to infect code like wildfire and is still just a workaround because data that is known synchronously isn't returned synchronously. The workaround beyond that (which I do actually use in some places) is to have a presentational component that is rendered once all of the data is available, which I do in some places, but is even more boilerplate and redundant types I have to specify in my TS code. It feels very much like going back to the old style of Redux, which isn't even recommended anymore (see edit at top).

I adopted Apollo to simplify my state management, and I'm simply advocating for a feature that I think would make it even better.


All that being said, I'd appreciate input from an Apollo contributor rather than disagreeing about the merits of a feature with a stranger. :^)

Fair enough, I've written a large amount with apollo state management, and the work around for what you describe is as simple as using fallback values while waiting for the data to load: https://github.com/freshollie/fresh-configurator/blob/b07d7785189bbf62a6cfdab148ed647028a3ed30/packages/configurator/src/providers/FcStatusProvider.tsx#L41

Also, I know you're not looking for using this with suspense, but the suspense API has already been requested to be part of apollo and would solve your issue when implemented https://github.com/apollographql/apollo-feature-requests/issues/162

Sorry for the harsh tone btw, I think your reasoning makes more sense to me now I've read over it a few times but I've just never see it as an issue :)

Also, I know you're not looking for using this with suspense, but the suspense API has already been requested to be part of apollo and would solve your issue when implemented apollographql/apollo-feature-requests#162

This presumes that suspense will ever be released... šŸ˜‰

For getting data synchronously from Apollo state you need to use function readQuery() which exists on ApolloClient.

Sure, but readQuery isn’t reactive. It doesn’t update when the Apollo store changes. I think I was able to hack around this by using a custom hook that did useQuery then if it didn’t return a value did readQuery, but that was very much a hack.

Was this page helpful?
0 / 5 - 0 ratings