Fix 0.10.0 errors #1719
Fix 0.10.0 errors #1719
Conversation
Deploying openapi-ts with
|
| Latest commit: |
c3558ba
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://0308b2c3.openapi-ts.pages.dev |
| Branch Preview URL: | https://fix-fetch.openapi-ts.pages.dev |
| id = randomID(); | ||
|
|
||
| // middleware (request) | ||
| options = Object.freeze({ |
There was a problem hiding this comment.
Micro-optimization: don’t run certain codepaths if no middleware loaded
| /** | ||
| * Test 1: GET /api/v1/get | ||
| */ | ||
| async function testGet() { |
There was a problem hiding this comment.
New tests that get loaded in a true browser environment. Not comprehensive, but we at least have:
- GET request (no body)
- POST request (with body)
- POST request with multi-part/form data
| @@ -54,15 +54,17 @@ | |||
| "build:cjs": "esbuild --bundle src/index.js --format=cjs --outfile=dist/cjs/index.cjs && cp dist/index.d.ts dist/cjs/index.d.cts", | |||
| "format": "biome format . --write", | |||
| "lint": "biome check .", | |||
| "generate-types": "openapi-typescript test/fixtures/api.yaml -o test/fixtures/api.d.ts", | |||
| "generate-types": "openapi-typescript -c test/redocly.yaml", | |||
There was a problem hiding this comment.
Use fancy new 7.x ability to generate multiple schemas with one config file
| } | ||
| } | ||
| } | ||
|
|
||
| // fetch! | ||
| let response = await fetch(request.url, request); | ||
| let response = await fetch(request); |
There was a problem hiding this comment.
This is the central error fix—Request can’t be substituted for RequestInit in every environment. Trying to hack around this results in even nastier code when you consider different browsers implement different stages of RequestInit and experimental APIs under-the-hood. This library shouldn’t take on any of that burden, and instead use a single Request instance for input which is universal and spec-compliant.
| @@ -15,7 +15,7 @@ export interface ClientOptions extends Omit<RequestInit, "headers"> { | |||
| /** set the common root URL for all API requests */ | |||
| baseUrl?: string; | |||
| /** custom fetch (defaults to globalThis.fetch) */ | |||
| fetch?: (input: string, init?: RequestInit) => Promise<Response>; | |||
| fetch?: (input: Request) => Promise<Response>; | |||
There was a problem hiding this comment.
This is technically a breaking change, but since it was a breaking change from 0.9.0 I’m just going to pretend this was an accidental regression and never happened 🙈
| @@ -12,14 +12,18 @@ | |||
| "lint": "pnpm run -r --parallel --aggregate-output lint", | |||
| "format": "pnpm run -r --parallel --aggregate-output format", | |||
| "test": "pnpm run -r --parallel --aggregate-output test", | |||
| "test-e2e": "pnpm run -r --parallel --aggregate-output test-e2e", | |||
There was a problem hiding this comment.
"test". We don’t need to run browser tests multiple times in different OSs and different Node versions (Playwright is the environment). We only need to run this ONCE in a single environment (Ubuntu, Node latest because that’s the cheapest/fastest).
Changes
Fixes #1715 and #1712. Adds Playwright tests to catch client behavior that doesn’t exist in Node’s
fetch()implementation.How to Review
Checklist
docs/updated (if necessary)pnpm run update:examplesrun (only applicable for openapi-typescript)