From b893c44f4290917f24c2ef7cda106c540df9cb3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Ji=C5=99=C3=AD=20Stani=C5=A1evsk=C3=BD?= Date: Fri, 30 Aug 2024 18:19:58 +0200 Subject: [PATCH] Do not set content-type on body-less requests (#1826) * Do not set content-type on body-less requests * Add docs * Remove commented code * Implement CR suggestions * Add change-set * Fix change-set * Post-rebase fix --- .changeset/chilled-forks-rush.md | 5 + docs/openapi-fetch/api.md | 11 ++ packages/openapi-fetch/src/index.js | 28 +++-- packages/openapi-fetch/test/index.test.ts | 144 ++++++++++++++++++++-- 4 files changed, 167 insertions(+), 21 deletions(-) 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..a530bb853 --- /dev/null +++ b/.changeset/chilled-forks-rush.md @@ -0,0 +1,5 @@ +--- +"openapi-fetch": minor +--- + +Do not set content-type on body-less requests diff --git a/docs/openapi-fetch/api.md b/docs/openapi-fetch/api.md index 3a120cb38..bc4539f59 100644 --- a/docs/openapi-fetch/api.md +++ b/docs/openapi-fetch/api.md @@ -181,6 +181,17 @@ 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. + +::: + ## 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: 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..b484d4fdb 100644 --- a/packages/openapi-fetch/test/index.test.ts +++ b/packages/openapi-fetch/test/index.test.ts @@ -4,6 +4,7 @@ import createClient, { type BodySerializer, type FetchOptions, type MethodResponse, + type HeadersOptions, type Middleware, type MiddlewareCallbackParams, type QuerySerializerOptions, @@ -11,7 +12,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 +820,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 +846,6 @@ describe("client", () => { expect(getRequest().headers).toEqual( new Headers({ "Cache-Control": "no-cache", - "Content-Type": "application/json", }), ); }); @@ -894,6 +889,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 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, + // 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"); + }, + ); + + 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) {