Middleware #1521

drwpow · 2024-01-28T21:57:12Z

Changes

Addresses #1122 by adding Middleware support. This is an RFC so feedback is welcome! This will be used for testing & feedback before stable release.

You can download the beta version at the @next tag:

npm i openapi-fetch@next

Dev Notes

Please read the docs first to learn about the API. The following are more “meta” notes helpful for early reviewers:

The API was suggested by @denisw 🎉. I found it to be easy-to-use and clear in typing.
- I pulled inspiration from tRPC links, Hono middleware, Next.js middleware, and Axios interceptors, but ultimately landed on a semi-unique API that’s a good blend of idiomatic and performant/lightweight.
- A lot of middleware complexity stems from server streaming, which this library isn’t concerned with (as the 99% usecase is JSON, which is not streamable).
- A simple middleware approach is not only easy-to-use; it’s also performant and lightweight (the final implementation only added a couple dozen lines to the codebase, and no performance impact).
There’s still no next() callback.
- In server middleware, this is necessary when streaming, or wanting to return a response to the user earlier but have some server work to do (logging, etc.).
- But in fetch() land, and in my early testing, I found it to be a little overcomplicated since JSON responses (which this relies on) doesn’t utilize streaming. And to start I wanted to prefer a simpler middleware structure over a complex one (i.e. not encourage over-engineering clientside server setups)
The .use(middleware) seemed to be a ubiquitous way to register middlewares
- Originally, there was a createClient({ middlewares: […] }) array, which doesn’t have any advantages, and only limits how/when middlewares are loaded.
- The less-commonly-used .eject() was inspired by Axios interceptors. It may or may not get use, but was essentially free. So why not?

How to Review

📚 Read the docs
Test out the latest version at openapi-fetch@next.

Checklist

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

changeset-bot · 2024-01-28T21:57:15Z

🦋 Changeset detected

Latest commit: fc3a468

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

This PR includes changesets to release 1 package

Name	Type
openapi-fetch	Minor

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

cloudflare-workers-and-pages · 2024-01-28T22:07:30Z

Deploying with Cloudflare Pages

Latest commit:	`fc3a468`
Status:	✅ Deploy successful!
Preview URL:	https://7779179f.openapi-ts.pages.dev
Branch Preview URL:	https://middleware.openapi-ts.pages.dev

View logs

drwpow · 2024-01-28T22:28:54Z

packages/openapi-fetch/README.md


 const {
  data, // only present if 2XX response
  error, // only present if 4XX or 5XX response
-} = await GET("/blogposts/{post_id}", {
+} = await client.GET("/blogposts/{post_id}", {


Extremely in-the-weeds: when writing the docs for middleware, I found const { use } = createClient() to be an awkward syntax (it it just me or does a floating use() just seem weird?). Renaming it (const { use: useMiddleware }) also felt awkward. client.use(…) felt like the most natural syntax, and matched other libraries.

As a result, it felt a little weird in other places to both keep client around and destructure it, so I opted to simplify and just keep client around in all places. Really, there’s no recommended syntax, and people can do what they want. I’m just trying to write the docs in the most “copy-pasteable” way 🤷

drwpow · 2024-01-28T22:30:37Z

packages/openapi-fetch/examples/react-query/src/lib/api/index.ts

 import type { paths } from "./v1";

+const throwOnError: Middleware = {


People have asked for this: a more automatic way to interface with React Query. Hopefully this is sufficient!

I feel this may be generally useful enough to deserve a call out in the middleware docs (even if just cross linking to the example). It may get missed otherwise

drwpow · 2024-01-28T22:32:23Z

docs/openapi-fetch/examples.md

@@ -4,130 +4,28 @@ title: openapi-fetch Examples

 # Examples

-## Authentication


With middleware, these awkward, one-off workarounds can go away. Now it can be handled more-automatically (also this entire section was rewritten, and moved to the middleware page).

drwpow · 2024-01-28T22:38:36Z

packages/openapi-fetch/src/index.js

@@ -81,26 +116,17 @@ export default function createClient(clientOptions) {
    if (response.ok) {
      // if "stream", skip parsing entirely
      if (parseAs === "stream") {
-        // fix for bun: bun consumes response.body, therefore clone before accessing
-        // TODO: test this?
-        return { data: response.clone().body, response };


I removed a lot of the .clone()-ing that was happening internally. I profiled this, and this can be a pretty big bottleneck, in addition to a waste of memory for large responses.

Instead, I’d rather users learn about safe vs unsafe body access rather than shield them from this. openapi-fetch will consume the body by calling .json() automatically (because 99% of the time that’s what users want). And if users need to read the raw body, then they’re doing something advanced and should know to .clone().

This related Bun issue here (#1486) I think is a mixture of a potential bug in Bun (which may be patched by now) along with user error. In either case, will investigate separately rather than cause potential performance bottlenecks for everyone.

packages/openapi-fetch/test/index.test.ts

packages/openapi-fetch/test/v7-beta.test.ts

docs/openapi-fetch/api.md

docs/openapi-fetch/middleware-auth.md

Noahkoole · 2024-01-30T14:23:56Z

Found another issue with type safety. #1525

drwpow · 2024-02-15T17:29:30Z

docs/openapi-fetch/index.md


 | Library                    | Size (min) | “GET” request              |
 | :------------------------- | ---------: | :------------------------- |
-| openapi-fetch              |     `4 kB` | `278k` ops/s (fastest)     |
+| openapi-fetch              |     `5 kB` | `278k` ops/s (fastest)     |


🤷 sometimes the bytes do things

drwpow · 2024-02-15T17:31:13Z

docs/openapi-fetch/testing.md

+
+## Mocking Requests
+
+To test requests, the `fetch` option can be supplied with any spy function like `vi.fn()` (Vitest) or `jest.fn()` (Jest).


🤔 I’ll probably rewrite this to recommend MSW as I’ve had some good success with it, but will probably wait until I refactor this codebase to use it for testing. Treat this as more of a placeholder.

drwpow · 2024-02-15T17:38:34Z

packages/openapi-fetch/test/index.test.ts

          });

          // expect post_id to be encoded properly
-          expect(fetchMocker.mock.calls[0][0]).toBe(`/blogposts/${post_id}`);
+          expect(fetchMocker.mock.calls[0][0].url).toBe(
+            `/blogposts/post?id%20=%20🥴`,


Side-effect of using new Request()—we’re not encoding anything manually. Seems OK? Still tests UTF-8 sticks

drwpow mentioned this pull request Jan 28, 2024

Some sort of middleware support #1122

Closed

1 task

drwpow force-pushed the middleware branch from ce5f868 to 584dcfc Compare January 28, 2024 22:07

drwpow force-pushed the middleware branch 2 times, most recently from 77f2787 to f6bcb23 Compare January 28, 2024 22:23

drwpow commented Jan 28, 2024

View reviewed changes

github-advanced-security bot found potential problems Jan 28, 2024

View reviewed changes

packages/openapi-fetch/test/index.test.ts Dismissed Show resolved Hide resolved

packages/openapi-fetch/test/v7-beta.test.ts Dismissed Show dismissed Hide dismissed

djMax reviewed Jan 29, 2024

View reviewed changes

docs/openapi-fetch/api.md Outdated Show resolved Hide resolved

djMax reviewed Jan 29, 2024

View reviewed changes

docs/openapi-fetch/api.md Outdated Show resolved Hide resolved

djMax reviewed Jan 29, 2024

View reviewed changes

docs/openapi-fetch/middleware-auth.md Outdated Show resolved Hide resolved

djMax reviewed Jan 29, 2024

View reviewed changes

docs/openapi-fetch/middleware-auth.md Outdated Show resolved Hide resolved

drwpow force-pushed the middleware branch from 8a8df6e to cdfd6fb Compare February 14, 2024 02:51

drwpow mentioned this pull request Feb 14, 2024

Query parameter serialization for objects is not the openapi default #1533

Closed

1 task

drwpow force-pushed the middleware branch 2 times, most recently from 3f45d46 to 831e0b0 Compare February 15, 2024 03:41

drwpow added 7 commits February 15, 2024 09:39

WIP

b174dd6

Middleware 2nd pass

fd44bd2

Middleware 3rd pass

f98b87b

Fix tests

6b576a8

Add changeset note on cloning

2551e4b

Docs fixes

7a61431

Add throwing error example to middleware docs

676066a

drwpow force-pushed the middleware branch 3 times, most recently from f8e3b5e to db0ada7 Compare February 15, 2024 17:28

drwpow commented Feb 15, 2024

View reviewed changes

drwpow force-pushed the middleware branch from db0ada7 to 47ac497 Compare February 15, 2024 17:30

drwpow commented Feb 15, 2024

View reviewed changes

Fix rebase

fc3a468

drwpow force-pushed the middleware branch from 47ac497 to fc3a468 Compare February 15, 2024 17:39

drwpow merged commit c61c472 into main Feb 15, 2024
8 checks passed

drwpow deleted the middleware branch February 15, 2024 17:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Middleware #1521

Middleware #1521

drwpow commented Jan 28, 2024 •

edited

Loading

changeset-bot bot commented Jan 28, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Jan 28, 2024 •

edited

Loading

drwpow Jan 28, 2024

drwpow Jan 28, 2024

WickyNilliams Jan 28, 2024

drwpow Jan 28, 2024 •

edited

Loading

drwpow Jan 28, 2024

drwpow Jan 29, 2024

Noahkoole commented Jan 30, 2024

drwpow Feb 15, 2024

drwpow Feb 15, 2024

drwpow Feb 15, 2024

		import type { paths } from "./v1";

		const throwOnError: Middleware = {

		@@ -4,130 +4,28 @@ title: openapi-fetch Examples

		# Examples

		## Authentication


		## Mocking Requests

		To test requests, the `fetch` option can be supplied with any spy function like `vi.fn()` (Vitest) or `jest.fn()` (Jest).

Middleware #1521

Middleware #1521

Conversation

drwpow commented Jan 28, 2024 • edited Loading

Changes

Dev Notes

How to Review

Checklist

changeset-bot bot commented Jan 28, 2024 • edited Loading

🦋 Changeset detected

cloudflare-workers-and-pages bot commented Jan 28, 2024 • edited Loading

Deploying with Cloudflare Pages

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drwpow Jan 28, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Noahkoole commented Jan 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

drwpow commented Jan 28, 2024 •

edited

Loading

changeset-bot bot commented Jan 28, 2024 •

edited

Loading

cloudflare-workers-and-pages bot commented Jan 28, 2024 •

edited

Loading

drwpow Jan 28, 2024 •

edited

Loading