Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix type discrimination for non-overlapping content types #1610

Merged
merged 3 commits into from
Apr 18, 2024

Conversation

illright
Copy link
Contributor

@illright illright commented Apr 7, 2024

Changes

Fixes #1609

How to Review

I've added a new kind of test — Vitest type test, to verify the correctness of this solution.

Checklist

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

@illright illright changed the title Illright/patch 77478 Fix type discrimination for non-overlapping content types Apr 7, 2024
Copy link

changeset-bot bot commented Apr 7, 2024

🦋 Changeset detected

Latest commit: 7f75fa9

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

This PR includes changesets to release 2 packages
Name Type
openapi-typescript-helpers Patch
openapi-fetch 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
Copy link
Owner

drwpow commented Apr 8, 2024

This is great, thank you!

If you could fix the linting error, and add a patch changeset (see comment), I’d be happy to merge. The tests pass locally for me; there seems to be an issue in CI right now that’s unrelated to your PR.

@JrSchild
Copy link

JrSchild commented Apr 10, 2024

Hi @illright. Does this also fix the issue where both data and result can not infer correctly to their respective types after an if check?

const dataRes = await GET('/');

if (dataRes.data !== undefined) {
  // dataRes.data is never 
  console.log(dataRes.data);
}

@illright
Copy link
Contributor Author

@JrSchild hard to say, I would need to see your openapi schema as well. Feel free to check out this branch and try it out for yourself

@illright
Copy link
Contributor Author

@drwpow is it ready to go?

@drwpow
Copy link
Owner

drwpow commented Apr 18, 2024

Ah thanks for the poke! Looks great and will merge; CI tests are currently experiencing an issue that I’ll fix this week.

@drwpow drwpow merged commit cc8073b into drwpow:main Apr 18, 2024
2 of 6 checks passed
@drwpow
Copy link
Owner

drwpow commented Apr 23, 2024

Released in the latest version of openapi-fetch. Thanks again!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Type discrimination between data and error doesn't work
3 participants