From 80e2b9350de16f277b11510b856de8b24faa4aa8 Mon Sep 17 00:00:00 2001 From: jirist Date: Wed, 7 Aug 2024 12:39:33 +0200 Subject: [PATCH 1/7] Do not set content-type on body-less requests --- packages/openapi-fetch/src/index.js | 28 +++-- packages/openapi-fetch/test/index.test.ts | 145 ++++++++++++++++++++-- 2 files changed, 152 insertions(+), 21 deletions(-) diff --git a/packages/openapi-fetch/src/index.js b/packages/openapi-fetch/src/index.js index 36248c52f..cc9952ec6 100644 --- a/packages/openapi-fetch/src/index.js +++ b/packages/openapi-fetch/src/index.js @@ -1,8 +1,4 @@ // settings & const -const DEFAULT_HEADERS = { - "Content-Type": "application/json", -}; - const PATH_PARAM_RE = /\{[^{}]+\}/g; /** Add custom parameters to Request object */ @@ -41,7 +37,6 @@ export default function createClient(clientOptions) { ...baseOptions } = { ...clientOptions }; baseUrl = removeTrailingSlash(baseUrl); - baseHeaders = mergeHeaders(DEFAULT_HEADERS, baseHeaders); const middlewares = []; /** @@ -58,6 +53,7 @@ export default function createClient(clientOptions) { parseAs = "json", querySerializer: requestQuerySerializer, bodySerializer = globalBodySerializer ?? defaultBodySerializer, + body, ...init } = fetchOptions || {}; if (localBaseUrl) { @@ -78,19 +74,25 @@ export default function createClient(clientOptions) { }); } + const serializedBody = body === undefined ? undefined : bodySerializer(body); + + const defaultHeaders = + // with no body, we should not to set Content-Type + serializedBody === undefined || + // if serialized body is FormData; browser will correctly set Content-Type & boundary expression + serializedBody instanceof FormData + ? {} + : { + "Content-Type": "application/json", + }; + const requestInit = { redirect: "follow", ...baseOptions, ...init, - headers: mergeHeaders(baseHeaders, headers, params.header), + body: serializedBody, + headers: mergeHeaders(defaultHeaders, baseHeaders, headers, params.header), }; - if (requestInit.body !== undefined) { - requestInit.body = bodySerializer(requestInit.body); - // remove `Content-Type` if serialized body is FormData; browser will correctly set Content-Type & boundary expression - if (requestInit.body instanceof FormData) { - requestInit.headers.delete("Content-Type"); - } - } let id; let options; diff --git a/packages/openapi-fetch/test/index.test.ts b/packages/openapi-fetch/test/index.test.ts index 79362d072..1e093fa07 100644 --- a/packages/openapi-fetch/test/index.test.ts +++ b/packages/openapi-fetch/test/index.test.ts @@ -4,6 +4,8 @@ import createClient, { type BodySerializer, type FetchOptions, type MethodResponse, + type FetchOptions, + type HeadersOptions, type Middleware, type MiddlewareCallbackParams, type QuerySerializerOptions, @@ -11,7 +13,7 @@ import createClient, { type PathBasedClient, createPathBasedClient, } from "../src/index.js"; -import { server, baseUrl, useMockRequestHandler, toAbsoluteURL } from "./fixtures/mock-server.js"; +import { baseUrl, server, toAbsoluteURL, useMockRequestHandler } from "./fixtures/mock-server.js"; import type { paths } from "./fixtures/api.js"; beforeAll(() => { @@ -819,12 +821,7 @@ describe("client", () => { await client.GET("/self"); // assert default headers were passed - expect(getRequest().headers).toEqual( - new Headers({ - ...headers, // assert new header got passed - "Content-Type": "application/json", // probably doesn’t need to get tested, but this was simpler than writing lots of code to ignore these - }), - ); + expect(getRequest().headers).toEqual(new Headers(headers)); }); it("can be overridden", async () => { @@ -850,7 +847,6 @@ describe("client", () => { expect(getRequest().headers).toEqual( new Headers({ "Cache-Control": "no-cache", - "Content-Type": "application/json", }), ); }); @@ -894,6 +890,139 @@ describe("client", () => { }); }); + describe("content-type", () => { + const BODY_ACCEPTING_METHODS = [["PUT"], ["POST"], ["DELETE"], ["OPTIONS"], ["PATCH"]] as const; + const ALL_METHODS = [...BODY_ACCEPTING_METHODS, ["GET"], ["HEAD"]] as const; + + const fireRequestAndGetContentType = async (options: { + defaultHeaders?: HeadersOptions; + method: (typeof ALL_METHODS)[number][number]; + fetchOptions: FetchOptions; + }) => { + const client = createClient({ baseUrl, headers: options.defaultHeaders }); + const { getRequest } = useMockRequestHandler({ + baseUrl, + method: "all", + path: "/blogposts-optional", + status: 200, + }); + await client[options.method]("/blogposts-optional", options.fetchOptions as any); + + const request = getRequest(); + return request.headers.get("content-type"); + }; + + it.each(ALL_METHODS)("no content-type for body-less requests - %s", async (method) => { + const contentType = await fireRequestAndGetContentType({ + method, + fetchOptions: {}, + }); + + expect(contentType).toBe(null); + }); + + it.each(ALL_METHODS)("no content-type for `undefined` body requests - %s", async (method) => { + const contentType = await fireRequestAndGetContentType({ + method, + fetchOptions: { + body: undefined, + }, + }); + + expect(contentType).toBe(null); + }); + + const BODIES = [{ prop: "a" }, {}, "", "str", null, false, 0, 1, new Date("2024-08-07T09:52:00.836Z")] as const; + // const BODIES = ["str"] as const; + const METHOD_BODY_COMBINATIONS = BODY_ACCEPTING_METHODS.flatMap(([method]) => + BODIES.map((body) => [method, body] as const), + ); + + it.each(METHOD_BODY_COMBINATIONS)( + "implicit default content-type for body-full requests - %s, %j", + async (method, body) => { + const contentType = await fireRequestAndGetContentType({ + method, + fetchOptions: { + body, + }, + }); + + expect(contentType).toBe("application/json"); + }, + ); + + it.each(METHOD_BODY_COMBINATIONS)( + "provided default content-type for body-full requests - %s, %j", + async (method, body) => { + const contentType = await fireRequestAndGetContentType({ + defaultHeaders: { + "content-type": "application/my-json", + }, + method, + fetchOptions: { + body, + }, + }); + + expect(contentType).toBe("application/my-json"); + }, + ); + + it.each(METHOD_BODY_COMBINATIONS)( + "native-fetch default content-type for body-full requests, when default is suppressed - %s, %j", + async (method, body) => { + const contentType = await fireRequestAndGetContentType({ + defaultHeaders: { + "content-type": null, + }, + method, + fetchOptions: { + body, + }, + }); + // the fetch implementation won't allow sending a body without content-type, + // so it invents one up and sends it, hopefully this will be consistent across + // local environments and won't make the tests flaky + expect(contentType).toBe("text/plain;charset=UTF-8"); + }, + ); + + it.each(METHOD_BODY_COMBINATIONS)( + "specified content-type for body-full requests - %s, %j", + async (method, body) => { + const contentType = await fireRequestAndGetContentType({ + method, + fetchOptions: { + body, + headers: { + "content-type": "application/my-json", + }, + }, + }); + + expect(contentType).toBe("application/my-json"); + }, + ); + + it.each(METHOD_BODY_COMBINATIONS)( + "specified content-type for body-full requests, even when default is suppressed - %s, %j", + async (method, body) => { + const contentType = await fireRequestAndGetContentType({ + method, + fetchOptions: { + body, + headers: { + "content-type": "application/my-json", + }, + }, + }); + + expect(contentType).toBe("application/my-json"); + }, + ); + }); + describe("fetch", () => { it("createClient", async () => { function createCustomFetch(data: any) { From 880b28817c5c521fbd052dccec6a8756d8f5351c Mon Sep 17 00:00:00 2001 From: jirist Date: Wed, 7 Aug 2024 12:51:57 +0200 Subject: [PATCH 2/7] Add docs --- docs/openapi-fetch/api.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/docs/openapi-fetch/api.md b/docs/openapi-fetch/api.md index 3a120cb38..d564d76fc 100644 --- a/docs/openapi-fetch/api.md +++ b/docs/openapi-fetch/api.md @@ -181,6 +181,18 @@ const { data, error } = await client.PUT("/submit", { }); ``` +::: tip + +For convenience, `openapi-fetch` sets `Content-Type` to `application/json` automatically +for any request that provides value for the `body` parameter. When the `bodySerializer` returns an instance of `FormData`, +`Content-Type` is omitted, allowing the browser to set it automatically with the correct message part boundary. + +You can also set `Content-Type` manually through `headers` object either in the fetch options, +or when instantiating the client. Setting `Content-Type` to `null` will omit the header, however the native fetch API +will likely set it to `text/plain` in this case. + +::: + ## Path serialization openapi-fetch supports path serialization as [outlined in the 3.1 spec](https://swagger.io/docs/specification/serialization/#path). This happens automatically, based on the specific format in your OpenAPI schema: From 7e78435ae1e2a5ad2c7c2e7b42f9dfb2dfa58acb Mon Sep 17 00:00:00 2001 From: jirist Date: Wed, 7 Aug 2024 13:00:36 +0200 Subject: [PATCH 3/7] Remove commented code --- packages/openapi-fetch/test/index.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/openapi-fetch/test/index.test.ts b/packages/openapi-fetch/test/index.test.ts index 1e093fa07..7a104b38d 100644 --- a/packages/openapi-fetch/test/index.test.ts +++ b/packages/openapi-fetch/test/index.test.ts @@ -933,7 +933,6 @@ describe("client", () => { }); const BODIES = [{ prop: "a" }, {}, "", "str", null, false, 0, 1, new Date("2024-08-07T09:52:00.836Z")] as const; - // const BODIES = ["str"] as const; const METHOD_BODY_COMBINATIONS = BODY_ACCEPTING_METHODS.flatMap(([method]) => BODIES.map((body) => [method, body] as const), ); From 15e89bfe5779ee38211e3773480d8e5ed8dfd168 Mon Sep 17 00:00:00 2001 From: jirist Date: Wed, 28 Aug 2024 10:46:57 +0200 Subject: [PATCH 4/7] Implement CR suggestions --- docs/openapi-fetch/api.md | 3 +-- packages/openapi-fetch/test/index.test.ts | 7 ++++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/docs/openapi-fetch/api.md b/docs/openapi-fetch/api.md index d564d76fc..bc4539f59 100644 --- a/docs/openapi-fetch/api.md +++ b/docs/openapi-fetch/api.md @@ -188,8 +188,7 @@ for any request that provides value for the `body` parameter. When the `bodySeri `Content-Type` is omitted, allowing the browser to set it automatically with the correct message part boundary. You can also set `Content-Type` manually through `headers` object either in the fetch options, -or when instantiating the client. Setting `Content-Type` to `null` will omit the header, however the native fetch API -will likely set it to `text/plain` in this case. +or when instantiating the client. ::: diff --git a/packages/openapi-fetch/test/index.test.ts b/packages/openapi-fetch/test/index.test.ts index 7a104b38d..eae14e6b1 100644 --- a/packages/openapi-fetch/test/index.test.ts +++ b/packages/openapi-fetch/test/index.test.ts @@ -981,9 +981,10 @@ describe("client", () => { }, }); // the fetch implementation won't allow sending a body without content-type, - // so it invents one up and sends it, hopefully this will be consistent across - // local environments and won't make the tests flaky - expect(contentType).toBe("text/plain;charset=UTF-8"); + // and it defaults to `text/plain;charset=UTF-8`, however the actual default value + // is irrelevant and might be flaky across different fetch implementations + // for us, it's important that it's not `application/json` + expect(contentType).not.toBe("application/json"); }, ); From fdeecfecceb00e61f58d61b6f023c03d623d6399 Mon Sep 17 00:00:00 2001 From: jirist Date: Fri, 30 Aug 2024 09:51:41 +0200 Subject: [PATCH 5/7] Add change-set --- .changeset/chilled-forks-rush.md | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 .changeset/chilled-forks-rush.md diff --git a/.changeset/chilled-forks-rush.md b/.changeset/chilled-forks-rush.md new file mode 100644 index 000000000..47cfcf167 --- /dev/null +++ b/.changeset/chilled-forks-rush.md @@ -0,0 +1,5 @@ +--- +"openapi-typescript": minor +--- + +Do not set content-type on body-less requests From ef8a69ea9bce1a8804b8eb6b9df5280dcfc7ce88 Mon Sep 17 00:00:00 2001 From: jirist Date: Fri, 30 Aug 2024 15:59:17 +0200 Subject: [PATCH 6/7] Fix change-set --- .changeset/chilled-forks-rush.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.changeset/chilled-forks-rush.md b/.changeset/chilled-forks-rush.md index 47cfcf167..a530bb853 100644 --- a/.changeset/chilled-forks-rush.md +++ b/.changeset/chilled-forks-rush.md @@ -1,5 +1,5 @@ --- -"openapi-typescript": minor +"openapi-fetch": minor --- Do not set content-type on body-less requests From 1812affcb1dc3618a98d03633edb3d19bc448d57 Mon Sep 17 00:00:00 2001 From: jirist Date: Fri, 30 Aug 2024 15:59:28 +0200 Subject: [PATCH 7/7] Post-rebase fix --- packages/openapi-fetch/test/index.test.ts | 1 - 1 file changed, 1 deletion(-) diff --git a/packages/openapi-fetch/test/index.test.ts b/packages/openapi-fetch/test/index.test.ts index eae14e6b1..b484d4fdb 100644 --- a/packages/openapi-fetch/test/index.test.ts +++ b/packages/openapi-fetch/test/index.test.ts @@ -4,7 +4,6 @@ import createClient, { type BodySerializer, type FetchOptions, type MethodResponse, - type FetchOptions, type HeadersOptions, type Middleware, type MiddlewareCallbackParams,