TS type for "data" return by useResource isn't correct

[mhum]

When data is returned by useResource, its value is undefined until loading is true. The current typing implies it is never undefined so current type checking doesn't ensure it's being used correctly.

[evert]

This choice was deliberate at the time, because if we change the type to something like this:

type UseResourceResult = {
  loading: true,
  error: undefined,
  data: undefined,
} | {
  loading: false,
  error: Error,
  data: undefined,
} | {
  loading: false,
  error: undefined,
  data: T,
}

Then this does better represent what we actually do. However, this becomes an issue with destructuring, which is by far the most common way to use this:

const { data, error, loading } = useResource('...');

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

// Typescript cannot infer that data is non-undefined, so this throws an error in strict mode
return <div>{data.title}</div>;

If we changed the type to:

type UseResourceResult = {
  loading: true,
  error: undefined,
  data: undefined,
} | {
  loading: false,
  error: Error,
  data: undefined,
} | {
  loading: false,
  error: undefined,
  data: T | undefined, // <- this is the only change
}

Then I have a feeling that people are just going to write this as:

const { data, error, loading } = useResource('...');

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

return <div>{data!.title}</div>; // Exclamation marks everywhere almost guaranteed

Typescript 3.7

Typescript 3.7 is changing this and allows Typescript to infer that "if loading is false, and error is undefined then data is non-undefined".

So, I can definitely make this change when this releases, however this will mean that users will always have to also check for errors:

if (error) <div>{error}</div>;

which is probably a frustrating change to make.

The future

All of this is going to start using React-suspense. No more boilerplate, just data:

import { useResource } from 'react-ketting/suspense';

function FancyComponent() {
  const { data } = useResource('...');
  return <div>{data.title}</div>;
}

To intercept loading states, you'll use a 'suspense boundary', which handles the loading state of any component in its subtree:

import { Suspense } from 'react';

function MyWrapper() {

  return  <Suspense fallback={<Spinner />}>
    <FancyComponent>
  </Suspense>;
}

And similarly you can make an <ErrorBoundary> anywhere in your tree.

[mhum]

The extremely common usecase in which I'm running into this problem is when using data within hooks (such as useEffect, useMemo, etc.)

Something like this:

function MyComponent({ myResource }: props) {
  const { data, error, loading } = useResource<MyResourceType>(myResource);
  const someFancyMath = useMemo(() => data !== undefined ? data.value1 * data.value2 : 0, [data]);

  if (loading) return null;

  return (
    <div>
      Look at my math: {someFancyMath}
    </div>
  );
}

With how it's currently typed, the type checker doesn't protect me from not checking if the value is undefined in useMemo. And even if discriminated unions worked, it still doesn't help since it might still be undefined in useMemo and I wouldn't also be checking loading or error there.

It would be great if it complained if I was doing this:

  const someFancyMath = useMemo(() =>  data.value1 * data.value2, [data]);