Skip to content

Conversation

Kauabunga
Copy link
Contributor

@Kauabunga Kauabunga commented Oct 1, 2025

Changes

This PR introduces request level middleware options enabling the same client level middleware feature to be applied per request. This fills the missing feature of the docs and the issue raised here #2005

How to Review

  1. Ensure the new fetch options are typed correctly
  2. Ensure the middleware priority makes sense (client > request)
  3. Unit test coverage is appropriate. Tests focus on request and client middleware options. Most of the middleware logic is already covered.

Checklist

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

@Kauabunga Kauabunga requested a review from a team as a code owner October 1, 2025 19:56
@Kauabunga Kauabunga requested a review from gzm0 October 1, 2025 19:56
Copy link

changeset-bot bot commented Oct 1, 2025

🦋 Changeset detected

Latest commit: 7f5ad65

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

This PR includes changesets to release 3 packages
Name Type
openapi-fetch Minor
openapi-react-query Major
swr-openapi Major

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

Copy link

netlify bot commented Oct 1, 2025

Deploy Preview for openapi-ts failed.

Name Link
🔨 Latest commit 7f5ad65
🔍 Latest deploy log https://app.netlify.com/projects/openapi-ts/deploys/68dfcb4221d7970008d0260b

@Kauabunga Kauabunga force-pushed the feat/enable-per-request-middleware branch from cd199fb to bd35e6b Compare October 1, 2025 19:58
Copy link
Contributor

@gzm0 gzm0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice! Thanks for taking the time to do this. In terms of functionality this looks good.

I've left some detailed comments mostly regarding readability.

@gzm0 gzm0 added the openapi-fetch Relevant to the openapi-fetch library label Oct 2, 2025
@gzm0
Copy link
Contributor

gzm0 commented Oct 2, 2025

This can be merged, but there seems to be an issue with Netlify. I do not know how that subsystem works (and I don't have time to look at it right now). I have asked for help on the maintainer chat.

Copy link
Contributor

@gzm0 gzm0 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apologies, I have to walk back on this one.

It seems that middleware is really a more appropriate plural of middleware. Not middlewares:

https://english.stackexchange.com/a/257139

So... The original parameter name as it was in the docs and how you have built it first was correct. Could you revert to that? (the internal variable names are less important, we can always change them later, but we're bound to the interface).

Sorry 🙈 (on the upside, we can merge w/o netlfiy after the revert).

@Kauabunga Kauabunga force-pushed the feat/enable-per-request-middleware branch from 979d7f3 to 7f5ad65 Compare October 3, 2025 13:10
@Kauabunga
Copy link
Contributor Author

No worries! Have rolled it back to using the original doc name middleware while keeping the test and variable name changes from ya feedback in.

Appreciate the quick reviews from you :)

@Kauabunga Kauabunga requested a review from gzm0 October 3, 2025 13:14
@gzm0 gzm0 merged commit 8f96eb5 into openapi-ts:main Oct 3, 2025
7 checks passed
@openapi-ts-bot openapi-ts-bot mentioned this pull request Oct 3, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
openapi-fetch Relevant to the openapi-fetch library
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants