React-apollo: Query result proposal: use actual data or {} to enable easier destructuring

[JonathanZWhite]

In the Query Component, if the query is still loading, the data variable will be undefined. This behavior is due to the following line:

data: isDataFilled(data) ? data : undefined

What do you think of making data {} instead of undefined? This way, it will be possible to do destructuring of data like so:

<Query query={HEROS_QUERY}>
  {({ loading, data: { heroes } }) => {
    if (loading) {
      return null;
    }

    return (...);
  }}
</Query>

Currently, this will throw Uncaught TypeError: Cannot read property 'heroes' of undefined. With the old query HOC pattern, this was not an issue (from the docs):

const ProfileWithData = graphql(CurrentUserForLayout, {
  props: ({ ownProps, data: { loading, currentUser, refetch } }) => ({
    userLoading: loading,
    user: currentUser,
    refetchUser: refetch,
  }),
})(Profile);

Since loading was moved out of data in the new query component, I can also see why it might make more sense to keep data as undefined when loading is true. Just wanted to get some opinions. 🎉

Version
[email protected]

[derek-duncan]

I think this was resolved in #1635 issue report 👍

[blasterpistol]

I think with version 2.1.0-beta.3 you need to change the typings from result.data: TData | undefined to result.data: TData since data now is always an empty object.

[alamothe]

I'm using 2.1.8 and typings say TData or undefined - never empty object: https://github.com/apollographql/react-apollo/blob/3985b47f086f38f891099732842ca2f16c7966c4/src/Query.tsx#L97

However, in runtime data is {}. Can you please confirm this is an error with typings? Is there an open issue for the fix?

[rosskevin]

At one point, there was an empty object. It is now TData | undefined. Related #1917

[alamothe]

@rosskevin I think it's the other way round.

[rosskevin]

Related #1917

[rosskevin]

@alamothe at one point it was returning data | {} and types changed to use Partial, this was done in #2313 and reverted in #2423

As mentioned in https://github.com/apollographql/react-apollo/pull/2423#issuecomment-424842118, the biggest problem with using data || {} for the result, while true it would enable easy destructuring, it makes someone then inspect the result to see if there is actual data.

For example (as that was the case some time ago), I needed all of the below to determine if I needed to render the LoadingComponent or the actual component rendering the data:

      <Query {...rest}>
        {result => {
          if (result.error) {
            // error handling code
          } else if (result.loading || Object.keys(result.data).length < 1) {
            // could be an empty object at this point because {}
            return LoadingComponent ? <LoadingComponent /> : null
          } else if (result.data) {
            // The above code guarantees that at this point it should be our real data
            const data: TData = result.data as TData
            return children({ client: result.client, data })
          } else {
            throw new Error('Unexpected fall through, check the log.')
          }
        }}
      </Query>

While this is easier for destructuring, as shown above, it requires more logic to determine if you have actual data to render.

[alamothe]

I absolutely agree with your rationale, and prefer undefined myself. It would be great if it returned to undefined, I will check.

But as far as I remember, it was undefined way back, then switched to {}. The changes you refer to are only about TypeScript typings (which had misadventures of their own), and not the actual logic.

If it's reverted to undefined, this must be a very recent change.

[hwillson]

React Apollo has been refactored to use React Hooks behind the scenes for everything, which means a lot has changed since this issue was opened (and a lot of outstanding issues have been resolved). We'll close this issue off, but please let us know if you're still encountering this problem using React Apollo >= 3.1.0. Thanks!

[ralvs]

Guys, I'm receiving the error above when I update above @apollo/[email protected]
Last test with @apollo/[email protected] and @apollo/[email protected]
Version 3.0.1 is the last that works for me.

TypeError: Cannot destructure property `tenant` of 'undefined' or 'null'.

My code

const {error, data: { tenant } } = useQuery(CURRENT_TENANT_QUERY);