hydrogen icon indicating copy to clipboard operation
hydrogen copied to clipboard

Improve the TypeScript experience for Storefront API errors

Open frehner opened this issue 1 year ago • 4 comments

Per https://shopify.dev/api/storefront#status_and_error_codes we should be able to create a standard shape for errors when there is a 200 response.

When there is a 4xx/5xx response, however, that's a little more nebulous. It seems that it doesn't follow the 200 error shape, and in some cases isn't even JSON at all.

frehner avatar Sep 08 '22 19:09 frehner

Given the query:

const query = `
  {
    shop {
      name
    }
    products(first: 1) {
      nodes {
        id
        title
        publishedAt
        handle
        variants(first: 1) {
          nodes {
            id
            image {
              url
              altText
              width
              height
            }
          }
        }
      }
    }
  }
`;

I can create this TS type for the response:

import type { FormattedExecutionResult } from "graphql";
import type {
  Shop,
  ProductConnection,
} from "@shopify/hydrogen-ui-alpha/storefront-api-types";

type StorefrontApiResponse = FormattedExecutionResult<{
  shop: Pick<Shop, "name">;
  products: ProductConnection;
}>;

This type provides the data and errors properties of the GraphQL response, and gets the errors object correctly typed as well (whereas right now we have errors: any :/ )

For the data object, you'll see that we have to provide the shape of that. Here it is again for emphasis:

type StorefrontApiResponse = FormattedExecutionResult<{
  shop: Pick<Shop, "name">;
  products: ProductConnection;
}>;

You'll see that, for shop, I can easily say "I'm only querying the name field" and it's good.

Things get more tricky for products, however. Here are the problems with that:

  • You have to realize to use ProductConnection instead of Product for the base type
  • There's no easy way to pick and choose what you're querying, especially as it gets deeply nested. This is solved in other libraries by inspecting the query (generally at build time, and almost always through an npm script) and generating a specific type for it, but for now we don't have that luxury. So we can either leave it as is, and say that all the fields are there (and the dev has to pay attention to their query to know if it's actually there or not), or we can also change it to say all of the fields are maybe there and you have to check them (using PartialDeep<> from type-fest). Maybe we can provide an easy way to do both, and let the dev decide what they prefer.
  • Finally, devs have to know to import FormattedExecutionResult from the graphql package. Should we re-export that ourselves and make it easier to use, e.g. StorefrontApiResult so devs can import that type directly from h-ui instead?
import type {StorefrontApiResult} from '@shopify/hydrogen-ui-alpha'

?

frehner avatar Sep 08 '22 23:09 frehner

Put up a PR for hydrogen ui.

frehner avatar Sep 12 '22 23:09 frehner

Finally, devs have to know to import FormattedExecutionResult from the graphql package. Should we re-export that ourselves and make it easier to use, e.g. StorefrontApiResult so devs can import that type directly from h-ui instead?

This would be nice. I actually find the naming FormattedExecutionResult quite generic. And if we want to encapsulate some of the Storefront API error quirks, we could through a StorefrontApiResult wrapper.

lordofthecactus avatar Sep 13 '22 08:09 lordofthecactus

Actually going to reopen so I don't forget to migrate this stuff to the hydrogen package too.

frehner avatar Sep 13 '22 15:09 frehner