React-apollo: useQuery: data should be undefined while loading for the first time

Created on 7 Aug 2019  路  9Comments  路  Source: apollographql/react-apollo

Intended outcome:

Here is a simple graphql schema:

type Foo {
  bar: Bar!
}

extend type Query {
  foo: Foo!
}

The schema explicity ensures that it isn't possible to have foo === null as well as foo.bar === null.

So, as a developer, I expect to be allowed to safely write something like this:

const { data } = useQuery(query);
const bar = data && data.foo.bar; // if there is data, then there are foo and bar

Actual outcome:

Having the loading state to return an empty object forces the developer to check that foo is defined, which looks like to go against the schema:

const { data } = useQuery(query);
const bar = data && data.foo && data.foo.bar; // if there is data, why should I check for foo to exist ?

According to this comment, this is an expected behavior but the explanation itself confesses that undefined would be more correct and I can't see where is the benefit of having an empty object at the first place.

Moreover, the fact that data goes back to undefined in case of error kind of looks inconsistent:

const response = useQuery(query);
console.log(response.loading, response.data, response.error);
// true, {}, undefined
// false, undefined, Error

What do you think ?

Version

@apollo/react-hooks 3.0.0

Most helpful comment

I think that as a simple workaround, people who use this feature can set a default value upon destructuring.

so this:

<Query>({ data: { foo }, loading }) => <div /></Query>

became into:

<Query>({ data: { foo } = {}, loading }) => <div /></Query>

All 9 comments

I encountered this as well.
The current behavior also conflicts with the typescript typings. First of all the typings specify that data can be undefined, which is not true at the moment. And since data can be an empty object, it conflicts with the expected type of the query result, when the schema specifies the fields are non-nullable.

The comment linked above mentions that this behavior where data is {} while loading, states that it is more developer friendly, but I'd argue the opposite, you end up needing to check for the presence of every single member of the response.

For now, I've wrapped useQuery and set data = undefined when Object.keys(data).length === 0

I kind of did the same thing by wrapping it like this:

import { QueryResult } from '@apollo/react-common';
import { QueryHookOptions, useQuery as originalUseQuery } from '@apollo/react-hooks';
import { OperationVariables } from 'apollo-client';
import { DocumentNode } from 'graphql';

export function useQuery<TData = any, TVariables = OperationVariables>(
  query: DocumentNode,
  options?: QueryHookOptions<TData, TVariables>,
): QueryResult<TData, TVariables> {
  const result = originalUseQuery(query, options);
  if (!Object.keys(result.data).length) {
    delete result.data;
  }
  return result;
}

Indeed. Either the data itself needs to be made undefined while loading, as promised by the typings, or the typings need to be adjusted.

At any rate, this does make destructuring the response rather annoying, and it is particularly annoying when the types are lying to you. This is hopefully something Suspense can eventually help with.

It's quite a pain actually, been switching from react-apollo-hooks and all my destructuring are going crazy, and typescript doesn't prevent it as apollo codegen typings doesn't reflect this behavior. Data should not be set if query hasn't returned yet.

Thanks all - great points. We're considering changing this - more details shortly.

I also agree the type should be changed so we don't have to check for data.

I've published new beta's (3.1.0-beta.0) that include the changes from https://github.com/apollographql/react-apollo/pull/3388. I'd love to hear what people think. We're still not entirely sure how we'll get these changes out, since these can be considered backwards incompatible. The React Apollo 3.x line is the last major bump we're going to do (since RA is being merged into Apollo Client). Given this we'll probably push these out as 3.1.0 and hope we don't upset too many people 馃槬.

I think that as a simple workaround, people who use this feature can set a default value upon destructuring.

so this:

<Query>({ data: { foo }, loading }) => <div /></Query>

became into:

<Query>({ data: { foo } = {}, loading }) => <div /></Query>

If anyone is interested I created this transformer to fix all my files using @ekfn approach.
https://gist.github.com/lnmunhoz/e9b096f6bb89ab584b9bf4164ae81caa

Hope it helps. Cheers!

Was this page helpful?
0 / 5 - 0 ratings