feat: narrow type based on status code

[DjordyKoert]

Changes

The various client methods (GET, POST, PUT e.t.c.) now have an additional status property.

This property allows users to narrow the type of their data/error based on their documented response status codes.

How to Review

How can a reviewer review your changes? What should be kept in mind for this review?

Checklist

• [x] Unit tests updated
• [x] docs/ updated (if necessary)
• [ ] pnpm run update:examples run (only applicable for openapi-typescript)

[changeset-bot[bot]]

🦋 Changeset detected

Latest commit: 30031bb45761e48adad87321bddb25c69d41d706

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 4 packages

Name	Type
openapi-typescript-helpers	Patch
openapi-react-query	Patch
openapi-fetch	Patch
swr-openapi	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

[drwpow]

I LOVE this! Please let us know if you need any help with the tests or the current direction. This would be amazing to land

[DjordyKoert]

@drwpow I think this is ready for a review now :)

[netlify[bot]]

❌ Deploy Preview for openapi-ts failed.

Name	Link
🔨 Latest commit	30031bb45761e48adad87321bddb25c69d41d706
🔍 Latest deploy log	https://app.netlify.com/sites/openapi-ts/deploys/67954b04cd491300089277d3

[benevbright]

Hey, thanks. This PR would fix https://github.com/openapi-ts/openapi-typescript/issues/1723 right?

[benevbright]

actually, https://github.com/openapi-ts/openapi-typescript/issues/1723 seems have been fixed by https://github.com/openapi-ts/openapi-typescript/pull/1937 already? @drwpow

[afarmerdev]

Thanks for the good work! May I ask what the blocker is on this? In most cases, openapi-react-query loses much of its usefulness without this feature, since status codes carry important semantic meaning in REST APIs.

In its current form, we can't even distinguish whether an error is a 400 or a 500 when using openapi-react-query. For example, we use 402 for validation errors, and we'd really like to have type-safe access to responses based on status codes.

[gzm0]

May I ask what the blocker is on this?

IIUC mainly the fixme in the test as pointed out by @drwpow . Basically, our understanding is that in the current form, this isn't backwards compatible.

IMHO we could discuss a breaking release, but then I'd like to look at:

• removing the error error return value
• potentially replacing the openapi-fetch specific return object with a native fetch response with narrower types (would imply the call sites have to call json()).

[edwardmp]

this is a much needed feature :) @gzm0 have you decided whether a breaking release is the way to go here?

[afarmerdev]

this is a much needed feature :) @gzm0 have you decided whether a breaking release is the way to go here?

We ended up using our own modified version of openapi-react-query which enriches the error type with responseStatus and responseBody. Unfortunately, responseBody is a union of all error body schemas rather than a specific one, but I believe this can be improved with some TypeScript judo.

@gzm0, if you're considering a breaking release, you might want to update the error schema as we did, instead of altering the idiomatic usage of react-query. Since this package handles HTTP calls, it makes perfect sense to include the status code and the corresponding error body object within the error type.

I am imaging a usage like this

  const { mutate, isError, error } = $api.useMutation(
    'post',
    '/v1/users',
    {}
  );

  if (error?.responseStatus === 400) {
    const { error_message } = error.responseBody;
    return <div>{error_message}</div>;
  }

  if (error?.responseStatus === 402) {
    // validation error
    const { validationErrors } = error.responseBody;
    return (
      <div>
        Please fill out the fields corrrectly{' '}
        {validationErrors.map((error) => {
          return (
            <div>
              {error.field}:{error.message}
            </div>
          );
        })}
      </div>
    );
  }

  if (isError) {
    //at this point it is either 500 or non-fetch related error
    error.cause && logger(error.cause); // log non-fetch errror
    return <div>Something went wrong</div>;
  }

And please don't go this path for discriminating fetch errors from non-fetch errors(which are very rare in our usage, perhaps should be handled centrally) I think ergonomics of testing error status right away without a step like this is better:

   if (error instanceof FetchError) { // please let's not do this
       if (error.responseStatus == 400)
          ...
    }

we can consider renaming error.responseStatus to error.statusCode and error.responseBody to error.body

[gzm0]

@edwardmp @afarmerdev thanks for keeping the ball rolling on this one.

Overall, I think I'm in favor of a breaking change to get this moving forward. I've tried to write down my thoughts below. LMK what you think.

Requirements

There are a couple of things I'd like us to look out for:

• Keep an easy way to distinguish (HTTP) success from error (i.e. an equivalent of Response.ok).
• Make sure the breaking change is not silent on usage sites during typechecking: i.e. usage sites should fail to typecheck and not just do something different.
• Nice to have: work towards data and error not being different.
• Nice to have: even the path to return a fetch response directly.

Potential solution

A way I think this could work (TS playground with example) is as follows:

• Replace the data and error fields with a new body field, use it to return the parsed response body (do not change parsing behavior for now).
• Add an ok field next to the existing status field.

I think this approach would satisfy 3 of the 4 requirements above: it does not satisfy "even the path to return a fetch response directly": body is the raw response. But maybe that's OK and we can deal with this in a future breaking change[^1].

[^1]: I realize splitting into multiple breaking changes is not ideal from a users POV, but given the limited resources, I think it makes sense to focus on the status improvement first. Of course, I'm more than happy to discuss other opinions.

Alternative

An alternative I have considered is to keep data and also use it to return errors. My main concern with that approach is that it might be difficult to debug on upgrade for users:

const { data, error } = await client.GET("/blogposts");

if (data) {
  // do stuff
} else {
  // handle error
}

If we re-use data both typechecking (and execution) will always take the "do stuff" branch. So whether or not typechecking fails will depend on what "do stuff" is.