From 06163a2030eaf8d0579f624d86481e1205aef396 Mon Sep 17 00:00:00 2001 From: Djordy Koert Date: Fri, 25 Oct 2024 19:33:36 +0200 Subject: [PATCH] fix(openapi-fetch): Return union of possible responses (data & error) (#1937) * Get all possible responses as a union * Add tests * lint fix * Add tests for standalone types * lint fix * changeset * Fix passing more keys (OkStatus) returning unknown * add test for non-existent media type * Update invalid path test * lint-fix * better @ts-expect-error scoping --- .changeset/small-jokes-wait.md | 6 + packages/openapi-fetch/src/index.d.ts | 6 +- .../test/common/response.test.ts | 6 +- .../never-response/never-response.test.ts | 143 +++++++++ .../schemas/never-response.d.ts | 142 +++++++++ .../schemas/never-response.yaml | 78 +++++ packages/openapi-fetch/test/types.test.ts | 283 ++++++++++++++++++ .../openapi-typescript-helpers/index.d.ts | 46 ++- 8 files changed, 692 insertions(+), 18 deletions(-) create mode 100644 .changeset/small-jokes-wait.md create mode 100644 packages/openapi-fetch/test/never-response/never-response.test.ts create mode 100644 packages/openapi-fetch/test/never-response/schemas/never-response.d.ts create mode 100644 packages/openapi-fetch/test/never-response/schemas/never-response.yaml create mode 100644 packages/openapi-fetch/test/types.test.ts diff --git a/.changeset/small-jokes-wait.md b/.changeset/small-jokes-wait.md new file mode 100644 index 00000000..cfcff9af --- /dev/null +++ b/.changeset/small-jokes-wait.md @@ -0,0 +1,6 @@ +--- +"openapi-typescript-helpers": patch +"openapi-fetch": patch +--- + +client data & error now return a union of possible types diff --git a/packages/openapi-fetch/src/index.d.ts b/packages/openapi-fetch/src/index.d.ts index 4941612f..a7b26a1b 100644 --- a/packages/openapi-fetch/src/index.d.ts +++ b/packages/openapi-fetch/src/index.d.ts @@ -98,7 +98,7 @@ export type RequestBodyOption = OperationRequestBodyContent extends never export type FetchOptions = RequestOptions & Omit; -export type FetchResponse = +export type FetchResponse, Options, Media extends MediaType> = | { data: ParseAsResponse, Media>, Options>; error?: never; @@ -187,7 +187,7 @@ export type ClientMethod< ...init: InitParam ) => Promise>; -export type ClientForPath = { +export type ClientForPath, Media extends MediaType> = { [Method in keyof PathInfo as Uppercase]: >( ...init: InitParam ) => Promise>; @@ -234,7 +234,7 @@ export default function createClient; -export type PathBasedClient = { +export type PathBasedClient, Media extends MediaType = MediaType> = { [Path in keyof Paths]: ClientForPath; }; diff --git a/packages/openapi-fetch/test/common/response.test.ts b/packages/openapi-fetch/test/common/response.test.ts index 50b8e85a..7a3aacf3 100644 --- a/packages/openapi-fetch/test/common/response.test.ts +++ b/packages/openapi-fetch/test/common/response.test.ts @@ -49,8 +49,8 @@ describe("response", () => { {}, ); - assertType(result.data); - // @ts-expect-error: FIXME when #1723 is resolved; this shouldn’t throw an error + //@ts-expect-error impossible to determine data type for invalid path + assertType(result.data); assertType(result.error); }); @@ -74,7 +74,7 @@ describe("response", () => { } else { expectTypeOf(result.data).toBeUndefined(); expectTypeOf(result.error).extract<{ code: number }>().toEqualTypeOf<{ code: number; message: string }>(); - expectTypeOf(result.error).exclude<{ code: number }>().toEqualTypeOf(); + expectTypeOf(result.error).exclude<{ code: number }>().toEqualTypeOf(undefined); } }); diff --git a/packages/openapi-fetch/test/never-response/never-response.test.ts b/packages/openapi-fetch/test/never-response/never-response.test.ts new file mode 100644 index 00000000..bca47fae --- /dev/null +++ b/packages/openapi-fetch/test/never-response/never-response.test.ts @@ -0,0 +1,143 @@ +import { assertType, describe, expect, test } from "vitest"; +import { createObservedClient } from "../helpers.js"; +import type { components, paths } from "./schemas/never-response.js"; + +describe("GET", () => { + test("sends correct method", async () => { + let method = ""; + const client = createObservedClient({}, async (req) => { + method = req.method; + return Response.json({}); + }); + await client.GET("/posts"); + expect(method).toBe("GET"); + }); + + test("sends correct options, returns success", async () => { + const mockData = { + id: 123, + title: "My Post", + }; + + let actualPathname = ""; + const client = createObservedClient({}, async (req) => { + actualPathname = new URL(req.url).pathname; + return Response.json(mockData); + }); + + const { data, error, response } = await client.GET("/posts/{id}", { + params: { path: { id: 123 } }, + }); + + assertType(data); + + // assert correct URL was called + expect(actualPathname).toBe("/posts/123"); + + // assert correct data was returned + expect(data).toEqual(mockData); + expect(response.status).toBe(200); + + // assert error is empty + expect(error).toBeUndefined(); + }); + + test("sends correct options, returns undefined on 204", async () => { + let actualPathname = ""; + const client = createObservedClient({}, async (req) => { + actualPathname = new URL(req.url).pathname; + return new Response(null, { status: 204 }); + }); + + const { data, error, response } = await client.GET("/posts/{id}", { + params: { path: { id: 123 } }, + }); + + assertType(data); + + // assert correct URL was called + expect(actualPathname).toBe("/posts/123"); + + // assert 204 to be transformed to empty object + expect(data).toEqual({}); + expect(response.status).toBe(204); + + // assert error is empty + expect(error).toBeUndefined(); + }); + + test("sends correct options, returns error", async () => { + const mockError = { code: 404, message: "Post not found" }; + + let method = ""; + let actualPathname = ""; + const client = createObservedClient({}, async (req) => { + method = req.method; + actualPathname = new URL(req.url).pathname; + return Response.json(mockError, { status: 404 }); + }); + + const { data, error, response } = await client.GET("/posts/{id}", { + params: { path: { id: 123 } }, + }); + + assertType(error); + + // assert correct URL was called + expect(actualPathname).toBe("/posts/123"); + + // assert correct method was called + expect(method).toBe("GET"); + + // assert correct error was returned + expect(error).toEqual(mockError); + expect(response.status).toBe(404); + + // assert data is empty + expect(data).toBeUndefined(); + }); + + test("handles array-type responses", async () => { + const client = createObservedClient({}, async () => Response.json([])); + + const { data } = await client.GET("/posts", { params: {} }); + if (!data) { + throw new Error("data empty"); + } + + // assert array type (and only array type) was inferred + expect(data.length).toBe(0); + }); + + test("handles empty-array-type 204 response", async () => { + let method = ""; + let actualPathname = ""; + const client = createObservedClient({}, async (req) => { + method = req.method; + actualPathname = new URL(req.url).pathname; + return new Response(null, { status: 204 }); + }); + + const { data } = await client.GET("/posts", { params: {} }); + + assertType(data); + + // assert correct URL was called + expect(actualPathname).toBe("/posts"); + + // assert correct method was called + expect(method).toBe("GET"); + + // assert 204 to be transformed to empty object + expect(data).toEqual({}); + }); + + test("gracefully handles invalid JSON for errors", async () => { + const client = createObservedClient({}, async () => new Response("Unauthorized", { status: 401 })); + + const { data, error } = await client.GET("/posts"); + + expect(data).toBeUndefined(); + expect(error).toBe("Unauthorized"); + }); +}); diff --git a/packages/openapi-fetch/test/never-response/schemas/never-response.d.ts b/packages/openapi-fetch/test/never-response/schemas/never-response.d.ts new file mode 100644 index 00000000..d3d7f5b7 --- /dev/null +++ b/packages/openapi-fetch/test/never-response/schemas/never-response.d.ts @@ -0,0 +1,142 @@ +/** + * This file was auto-generated by openapi-typescript. + * Do not make direct changes to the file. + */ + +export interface paths { + "/posts": { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + get: { + parameters: { + query?: never; + header?: never; + path?: never; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["Post"][]; + }; + }; + /** @description No posts found, but it's OK */ + 204: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": unknown[]; + }; + }; + /** @description Unexpected error */ + default: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["Error"]; + }; + }; + }; + }; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; + "/posts/{id}": { + parameters: { + query?: never; + header?: never; + path: { + id: number; + }; + cookie?: never; + }; + get: { + parameters: { + query?: never; + header?: never; + path: { + id: number; + }; + cookie?: never; + }; + requestBody?: never; + responses: { + /** @description OK */ + 200: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["Post"]; + }; + }; + /** @description No post found, but it's OK */ + 204: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + /** @description A weird error happened */ + 500: { + headers: { + [name: string]: unknown; + }; + content?: never; + }; + /** @description Unexpected error */ + default: { + headers: { + [name: string]: unknown; + }; + content: { + "application/json": components["schemas"]["Error"]; + }; + }; + }; + }; + put?: never; + post?: never; + delete?: never; + options?: never; + head?: never; + patch?: never; + trace?: never; + }; +} +export type webhooks = Record; +export interface components { + schemas: { + Error: { + code: number; + message: string; + }; + Post: { + id: number; + title: string; + }; + }; + responses: never; + parameters: never; + requestBodies: never; + headers: never; + pathItems: never; +} +export type $defs = Record; +export type operations = Record; diff --git a/packages/openapi-fetch/test/never-response/schemas/never-response.yaml b/packages/openapi-fetch/test/never-response/schemas/never-response.yaml new file mode 100644 index 00000000..45f4bce5 --- /dev/null +++ b/packages/openapi-fetch/test/never-response/schemas/never-response.yaml @@ -0,0 +1,78 @@ +openapi: 3.1.0 +info: + title: openapi-fetch + version: "1.0" +paths: + /posts: + get: + responses: + 200: + description: OK + operationId: getPosts + content: + application/json: + schema: + type: array + items: + $ref: "#/components/schemas/Post" + 204: + description: No posts found, but it's OK + content: + application/json: + schema: + type: array + items: {} + default: + description: Unexpected error + content: + application/json: + schema: + $ref: "#/components/schemas/Error" + /posts/{id}: + parameters: + - name: id + in: path + required: true + schema: + type: integer + get: + responses: + 200: + description: OK + operationId: getPost + content: + application/json: + schema: + $ref: "#/components/schemas/Post" + 204: + description: No post found, but it's OK + 500: + description: A weird error happened + default: + description: Unexpected error + content: + application/json: + schema: + $ref: "#/components/schemas/Error" +components: + schemas: + Error: + type: object + required: + - code + - message + properties: + code: + type: integer + message: + type: string + Post: + type: object + required: + - id + - title + properties: + id: + type: integer + title: + type: string diff --git a/packages/openapi-fetch/test/types.test.ts b/packages/openapi-fetch/test/types.test.ts new file mode 100644 index 00000000..a44e437e --- /dev/null +++ b/packages/openapi-fetch/test/types.test.ts @@ -0,0 +1,283 @@ +import { assertType, describe, test } from "vitest"; +import type { ErrorResponse, GetResponseContent, OkStatus, SuccessResponse } from "openapi-typescript-helpers"; + +describe("types", () => { + describe("GetResponseContent", () => { + describe("MixedResponses", () => { + interface MixedResponses { + 200: { + content: { + "application/json": { data: `200 application/json` }; + "text/plain": `200 text/plain`; + }; + }; + 204: { content: never }; + 206: { content: { "text/plain": `206 text/plain` } }; + 404: { content: { "text/plain": `404 text/plain` } }; + 500: { content: { "application/json": { error: `500 application/json` } } }; + } + + test("returns all possible responses", () => { + type Response = GetResponseContent; + assertType({ data: "200 application/json" }); + assertType({ + // @ts-expect-error It picks literal over string + data: "200 but different string", + }); + assertType("200 text/plain"); + assertType("206 text/plain"); + assertType("404 text/plain"); + assertType({ error: "500 application/json" }); + assertType( + // @ts-expect-error 204 never does not become undefined + undefined, + ); + }); + + test("returns correct type for 200 with literal", () => { + type Response = GetResponseContent; + assertType({ data: "200 application/json" }); + assertType("200 text/plain"); + assertType( + // @ts-expect-error + "206 text/plain", + ); + assertType( + // @ts-expect-error + "404 text/plain", + ); + assertType({ + // @ts-expect-error + error: "500 application/json", + }); + }); + + test("returns correct type for 200 with json-like literal", () => { + type Response = GetResponseContent; + assertType({ data: "200 application/json" }); + assertType( + // @ts-expect-error + "200 text/plain", + ); + assertType( + // @ts-expect-error + "206 text/plain", + ); + assertType( + // @ts-expect-error + "404 text/plain", + ); + assertType({ + // @ts-expect-error + error: "500 application/json", + }); + }); + + test("returns correct type for 200 with application/json", () => { + type Response = GetResponseContent; + assertType({ data: "200 application/json" }); + assertType( + // @ts-expect-error + "200 text/plain", + ); + assertType( + // @ts-expect-error + "206 text/plain", + ); + assertType( + // @ts-expect-error + "404 text/plain", + ); + assertType({ + // @ts-expect-error + error: "500 application/json", + }); + }); + + test("returns 200 & 500 responses", () => { + type Response = GetResponseContent; + assertType({ data: "200 application/json" }); + assertType("200 text/plain"); + assertType( + // @ts-expect-error + "206 text/plain", + ); + assertType( + // @ts-expect-error + "404 text/plain", + ); + assertType({ error: "500 application/json" }); + }); + + test("returns all OK responses", () => { + type Response = GetResponseContent< + MixedResponses, + `${string}/${string}`, + // @ts-expect-error: Type 'OkStatus' does not satisfy the constraint 'keyof MixedResponses'. Can safely ignore this error. + OkStatus + >; + assertType({ data: "200 application/json" }); + assertType("200 text/plain"); + assertType("206 text/plain"); + assertType( + // @ts-expect-error + "404 text/plain", + ); + assertType({ + // @ts-expect-error + error: "500 application/json", + }); + }); + + test("non existent media type", () => { + type Response = GetResponseContent; + assertType({ + // @ts-expect-error + data: "200 application/json", + }); + assertType({ + // @ts-expect-error + data: "200 but different string", + }); + assertType( + // @ts-expect-error + "200 text/plain", + ); + assertType( + // @ts-expect-error + "206 text/plain", + ); + assertType( + // @ts-expect-error + "404 text/plain", + ); + + assertType({ + // @ts-expect-error + error: "500 application/json", + }); + assertType( + // @ts-expect-error 204 never does not become undefined + undefined, + ); + }); + }); + + test("picks undefined over never", () => { + interface Responses { + 200: { content: { "application/json": { data: `Yup` } } }; + 204: { content?: never }; + } + + type Response = GetResponseContent; + assertType({ data: "Yup" }); + assertType(undefined); + }); + }); + + describe("SuccessResponse", () => { + interface Responses { + 200: { + content: { + "application/json": { data: `200 application/json` }; + "text/plain": `200 text/plain`; + }; + }; + 204: { content: never }; + 206: { content: { "text/plain": `206 text/plain` } }; + 404: { content: { "text/plain": `404 text/plain` } }; + 500: { content: { "application/json": { error: `500 application/json` } } }; + } + + test("returns all 2XX responses", () => { + type Response = SuccessResponse; + assertType({ data: "200 application/json" }); + assertType("200 text/plain"); + assertType("206 text/plain"); + assertType( + // @ts-expect-error + "404 text/plain", + ); + assertType({ + // @ts-expect-error + error: "500 application/json", + }); + }); + + test("returns all 2XX responses, only application/json", () => { + type Response = SuccessResponse; + assertType({ data: "200 application/json" }); + assertType( + // @ts-expect-error + "200 text/plain", + ); + assertType( + // @ts-expect-error + "206 text/plain", + ); + assertType( + // @ts-expect-error + "404 text/plain", + ); + // @ts-expect-error + assertType({ error: "500 application/json" }); + }); + }); + + describe("ErrorResponse", () => { + interface Responses { + 200: { + content: { + "application/json": { data: `200 application/json` }; + "text/plain": `200 text/plain`; + }; + }; + 204: { content: never }; + 206: { content: { "text/plain": `206 text/plain` } }; + 404: { content: { "text/plain": `404 text/plain` } }; + 500: { content: { "application/json": { error: `500 application/json` } } }; + default: { content: { "application/json": { error: `default application/json` } } }; + } + + test("returns all 5XX and 4xx responses", () => { + type Response = ErrorResponse; + assertType({ + // @ts-expect-error + data: "200 application/json", + }); + assertType( + // @ts-expect-error + "200 text/plain", + ); + assertType( + // @ts-expect-error + "206 text/plain", + ); + assertType("404 text/plain"); + assertType({ error: "500 application/json" }); + assertType({ error: "default application/json" }); + }); + + test("returns all 5XX and 4xx responses, only application/json", () => { + type Response = ErrorResponse; + assertType({ + // @ts-expect-error + data: "200 application/json", + }); + assertType( + // @ts-expect-error + "200 text/plain", + ); + assertType( + // @ts-expect-error + "206 text/plain", + ); + assertType( + // @ts-expect-error + "404 text/plain", + ); + assertType({ error: "500 application/json" }); + assertType({ error: "default application/json" }); + }); + }); +}); diff --git a/packages/openapi-typescript-helpers/index.d.ts b/packages/openapi-typescript-helpers/index.d.ts index c6cf5cbd..1715c07c 100644 --- a/packages/openapi-typescript-helpers/index.d.ts +++ b/packages/openapi-typescript-helpers/index.d.ts @@ -116,25 +116,47 @@ export type OperationRequestBodyContent = FilterKeys>, MediaType> | undefined : FilterKeys, MediaType>; -/** Return first 2XX response from a Response Object Map */ -export type SuccessResponse = FilterKeys< - ResponseContent>, - Media ->; +/** Return all 2XX responses from a Response Object Map */ +export type SuccessResponse< + T extends Record, + Media extends MediaType = MediaType, +> = GetResponseContent; + +type GetResponseContent< + T extends Record, + Media extends MediaType = MediaType, + ResponseCode extends keyof T = keyof T, +> = ResponseCode extends keyof T + ? { + [K in ResponseCode]: T[K]["content"] extends Record + ? FilterKeys extends never + ? T[K]["content"] + : FilterKeys + : K extends keyof T + ? T[K]["content"] + : never; + }[ResponseCode] + : never; /** - * Return first 5XX or 4XX response (in that order) from a Response Object Map + * Return all 5XX and 4XX responses (in that order) from a Response Object Map */ -export type ErrorResponse = FilterKeys< - ResponseContent>, - Media ->; +export type ErrorResponse< + T extends Record, + Media extends MediaType = MediaType, +> = GetResponseContent; /** Return first JSON-like 2XX response from a path + HTTP method */ -export type SuccessResponseJSON = SuccessResponse, `${string}/json`>; +export type SuccessResponseJSON> = SuccessResponse< + ResponseObjectMap, + `${string}/json` +>; /** Return first JSON-like 5XX or 4XX response from a path + HTTP method */ -export type ErrorResponseJSON = ErrorResponse, `${string}/json`>; +export type ErrorResponseJSON> = ErrorResponse< + ResponseObjectMap, + `${string}/json` +>; /** Return JSON-like request body from a path + HTTP method */ export type RequestBodyJSON = JSONLike, "content">>;