improved QueryResult type for better loading/error/data inference

[Kumagor0]

Here's typical useQuery use case:

const DataFetchingComponent = () => {

  const { loading, error, data } = useQuery<myQueryResultType>(
    myQuery
  );

  if (loading)
    return <div>Loading...</div>;

  if (error)
    return <div>{`Error! ${error.message}`}</div>;
  
  return <DataRenderingComponent data={data}/>
};

It has one problem: currently data always has type TData | undefined, even when loading is false and there is no error which is clearly not something that should happen in runtime so it'd be awesome if typescript could realize that as well. Right now I have to use non-null assertion operator to convince typescript everything is okay:

  return <DataRenderingComponent data={data!}/>

But it appears to me that this issue is rather simple to fix on typings level which I've tried my best to do in this PR.

[codecov[bot]]

Codecov Report

Merging #6006 into master will not change coverage by %. The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #6006   +/-   ##
=======================================
  Coverage   95.24%   95.24%           
=======================================
  Files          89       89           
  Lines        3703     3703           
  Branches      879      879           
=======================================
  Hits         3527     3527           
  Misses        154      154           
  Partials       22       22           

---

Continue to review full report at Codecov.

Legend - Click here to learn more Δ = absolute <relative> (impact), ø = not affected, ? = missing data Powered by Codecov. Last update 04035e8...a29a068. Read the comment docs.

[abenhamdine]

Looks like a good idea ! Have you tested it ? Ts pick the correct interface in each case ?

[Kumagor0]

@abenhamdine yes

[colin-alexa]

I was about to make this PR when I saw there was already one open! @jcreighton @Kumagor0 is there anything I can do to help make this happen?

[lmerotta-zz]

Hello, sorry to bother, is there any chance that this PR gets merged ?

[mccraveiro]

Conflicts with https://github.com/apollographql/apollo-client/pull/10356 where we are ignoring errors and not returning any data.

[Kumagor0]

well seems like #10356 has been closed without merge anyway

[mccraveiro]

Totally. I'd love to see this improvement on the types shipped :)

[jerelmiller]

Hey @Kumagor0 👋

Thanks for your patience on this PR! I know you opened it quite some time ago.

While I appreciate the effort to improve the types, there are still use cases that would be needed to handle this correctly. This change could result in some wrong types for certain scenarios.

For example, if you set errorPolicy to ignore, there is a chance you could end up with the following state:

{ data: undefined, errors: undefined, loading: false }

This is also true when skip is set to true since no request has been sent until skip is set back to false.

---

More robust typing are definitely something we want to improve down the line with a v4 of the library. We'll have the ability to completely rethink these types at that time and can make breaking changes to better align with some of these more complex scenarios.

Appreciate the contribution regardless! At the very least, this is a good signal that we can be more diligent about our types.