Skip to content

Commit 17a9cc1

Browse files
committed
fix: make Readable/Writable marker resolution opt-in and fix readonly arrays
Readable<T> and Writable<T> were applied unconditionally to all openapi-fetch responses and request bodies, causing two regressions: 1. Readable<readonly string[]> fell through to the object branch, destructuring arrays into plain objects (fixes #2615) 2. The mapped type created new structural types that broke discriminated union narrowing (fixes #2620) Changes: - Add MaybeReadable<T, Markers> and MaybeWritable<T, Markers> helpers that resolve to identity when Markers=false (default) - Thread Markers type parameter through openapi-fetch type chain - Fix Readable/Writable to handle both mutable and readonly arrays - Update inline helper types in openapi-typescript codegen - Add regression tests for both issues with discriminated union schema
1 parent dff8655 commit 17a9cc1

File tree

10 files changed

+568
-77
lines changed

10 files changed

+568
-77
lines changed

.changeset/fix-readable-markers.md

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,25 @@
1+
---
2+
"openapi-typescript-helpers": patch
3+
"openapi-fetch": minor
4+
"openapi-typescript": patch
5+
---
6+
7+
Fix `Readable` and `Writable` types to correctly handle readonly arrays, and make marker resolution opt-in via `Markers` type parameter (default: `false`). This fixes issues where `Readable<readonly string[]>` would destructure the array into an object type (#2615), and where applying `Readable`/`Writable` unconditionally broke discriminated union narrowing (#2620).
8+
9+
### Breaking change for `--read-write-markers` users
10+
11+
`Readable`/`Writable` are no longer applied unconditionally to all responses and request bodies. If you use `--read-write-markers` with `openapi-fetch`, you now need to pass `true` as the third type parameter to opt in to marker resolution:
12+
13+
```ts
14+
// Before (0.17.x): markers resolved automatically
15+
const client = createClient<paths>();
16+
17+
// After: opt in explicitly
18+
import type { MediaType } from "openapi-typescript-helpers";
19+
// ↓ MediaType is required as a positional placeholder
20+
const client = createClient<paths, MediaType, true>();
21+
```
22+
23+
The second type parameter (`MediaType`) is the media type filter — it defaults to `MediaType` which accepts all content types. You must specify it explicitly to reach the third `Markers` parameter, since TypeScript does not support skipping positional type arguments.
24+
25+
Without `Markers = true`, `$Read`/`$Write` markers will be treated as raw wrapper types and not resolved. This is the intended default for users who do not use `--read-write-markers`.

packages/openapi-fetch/src/index.d.ts

Lines changed: 83 additions & 54 deletions
Original file line numberDiff line numberDiff line change
@@ -3,14 +3,14 @@ import type {
33
FilterKeys,
44
HttpMethod,
55
IsOperationRequestBodyOptional,
6+
MaybeReadable,
7+
MaybeWritable,
68
MediaType,
79
OperationRequestBodyContent,
810
PathsWithMethod,
9-
Readable,
1011
RequiredKeysOf,
1112
ResponseObjectMap,
1213
SuccessResponse,
13-
Writable,
1414
} from "openapi-typescript-helpers";
1515

1616
/** Options for each client instance */
@@ -98,31 +98,37 @@ export type ParamsOption<T> = T extends {
9898
: { params: T["parameters"] }
9999
: DefaultParamsOption;
100100

101-
// Writable<T> strips $Read markers (readOnly properties excluded from request body)
102-
export type RequestBodyOption<T> =
103-
Writable<OperationRequestBodyContent<T>> extends never
101+
// MaybeWritable<T> strips $Read markers when Markers is true (readOnly properties excluded from request body)
102+
export type RequestBodyOption<T, Markers extends boolean = false> =
103+
MaybeWritable<OperationRequestBodyContent<T>, Markers> extends never
104104
? { body?: never }
105105
: IsOperationRequestBodyOptional<T> extends true
106-
? { body?: Writable<OperationRequestBodyContent<T>> }
107-
: { body: Writable<OperationRequestBodyContent<T>> };
106+
? { body?: MaybeWritable<OperationRequestBodyContent<T>, Markers> }
107+
: { body: MaybeWritable<OperationRequestBodyContent<T>, Markers> };
108108

109-
export type FetchOptions<T> = RequestOptions<T> & Omit<RequestInit, "body" | "headers">;
109+
export type FetchOptions<T, Markers extends boolean = false> = RequestOptions<T, Markers> &
110+
Omit<RequestInit, "body" | "headers">;
110111

111-
// Readable<T> strips $Write markers (writeOnly properties excluded from response)
112-
export type FetchResponse<T extends Record<string | number, any>, Options, Media extends MediaType> =
112+
// MaybeReadable<T> strips $Write markers when Markers is true (writeOnly properties excluded from response)
113+
export type FetchResponse<
114+
T extends Record<string | number, any>,
115+
Options,
116+
Media extends MediaType,
117+
Markers extends boolean = false,
118+
> =
113119
| {
114-
data: ParseAsResponse<Readable<SuccessResponse<ResponseObjectMap<T>, Media>>, Options>;
120+
data: ParseAsResponse<MaybeReadable<SuccessResponse<ResponseObjectMap<T>, Media>, Markers>, Options>;
115121
error?: never;
116122
response: Response;
117123
}
118124
| {
119125
data?: never;
120-
error: Readable<ErrorResponse<ResponseObjectMap<T>, Media>>;
126+
error: MaybeReadable<ErrorResponse<ResponseObjectMap<T>, Media>, Markers>;
121127
response: Response;
122128
};
123129

124-
export type RequestOptions<T> = ParamsOption<T> &
125-
RequestBodyOption<T> & {
130+
export type RequestOptions<T, Markers extends boolean = false> = ParamsOption<T> &
131+
RequestBodyOption<T, Markers> & {
126132
baseUrl?: string;
127133
querySerializer?: QuerySerializer<T> | QuerySerializerOptions;
128134
bodySerializer?: BodySerializer<T>;
@@ -190,10 +196,10 @@ export type Middleware =
190196
};
191197

192198
/** This type helper makes the 2nd function param required if params/requestBody are required; otherwise, optional */
193-
export type MaybeOptionalInit<Params, Location extends keyof Params> =
194-
RequiredKeysOf<FetchOptions<FilterKeys<Params, Location>>> extends never
195-
? FetchOptions<FilterKeys<Params, Location>> | undefined
196-
: FetchOptions<FilterKeys<Params, Location>>;
199+
export type MaybeOptionalInit<Params, Location extends keyof Params, Markers extends boolean = false> =
200+
RequiredKeysOf<FetchOptions<FilterKeys<Params, Location>, Markers>> extends never
201+
? FetchOptions<FilterKeys<Params, Location>, Markers> | undefined
202+
: FetchOptions<FilterKeys<Params, Location>, Markers>;
197203

198204
// The final init param to accept.
199205
// - Determines if the param is optional or not.
@@ -206,79 +212,102 @@ export type ClientMethod<
206212
Paths extends Record<string, Record<HttpMethod, {}>>,
207213
Method extends HttpMethod,
208214
Media extends MediaType,
209-
> = <Path extends PathsWithMethod<Paths, Method>, Init extends MaybeOptionalInit<Paths[Path], Method>>(
215+
Markers extends boolean = false,
216+
> = <Path extends PathsWithMethod<Paths, Method>, Init extends MaybeOptionalInit<Paths[Path], Method, Markers>>(
210217
url: Path,
211218
...init: InitParam<Init>
212-
) => Promise<FetchResponse<Paths[Path][Method], Init, Media>>;
219+
) => Promise<FetchResponse<Paths[Path][Method], Init, Media, Markers>>;
213220

214-
export type ClientRequestMethod<Paths extends Record<string, Record<HttpMethod, {}>>, Media extends MediaType> = <
221+
export type ClientRequestMethod<
222+
Paths extends Record<string, Record<HttpMethod, {}>>,
223+
Media extends MediaType,
224+
Markers extends boolean = false,
225+
> = <
215226
Method extends HttpMethod,
216227
Path extends PathsWithMethod<Paths, Method>,
217-
Init extends MaybeOptionalInit<Paths[Path], Method>,
228+
Init extends MaybeOptionalInit<Paths[Path], Method, Markers>,
218229
>(
219230
method: Method,
220231
url: Path,
221232
...init: InitParam<Init>
222-
) => Promise<FetchResponse<Paths[Path][Method], Init, Media>>;
233+
) => Promise<FetchResponse<Paths[Path][Method], Init, Media, Markers>>;
223234

224-
export type ClientForPath<PathInfo extends Record<string | number, any>, Media extends MediaType> = {
225-
[Method in keyof PathInfo as Uppercase<string & Method>]: <Init extends MaybeOptionalInit<PathInfo, Method>>(
235+
export type ClientForPath<
236+
PathInfo extends Record<string | number, any>,
237+
Media extends MediaType,
238+
Markers extends boolean = false,
239+
> = {
240+
[Method in keyof PathInfo as Uppercase<string & Method>]: <Init extends MaybeOptionalInit<PathInfo, Method, Markers>>(
226241
...init: InitParam<Init>
227-
) => Promise<FetchResponse<PathInfo[Method], Init, Media>>;
242+
) => Promise<FetchResponse<PathInfo[Method], Init, Media, Markers>>;
228243
};
229244

230-
export interface Client<Paths extends {}, Media extends MediaType = MediaType> {
231-
request: ClientRequestMethod<Paths, Media>;
245+
export interface Client<Paths extends {}, Media extends MediaType = MediaType, Markers extends boolean = false> {
246+
request: ClientRequestMethod<Paths, Media, Markers>;
232247
/** Call a GET endpoint */
233-
GET: ClientMethod<Paths, "get", Media>;
248+
GET: ClientMethod<Paths, "get", Media, Markers>;
234249
/** Call a PUT endpoint */
235-
PUT: ClientMethod<Paths, "put", Media>;
250+
PUT: ClientMethod<Paths, "put", Media, Markers>;
236251
/** Call a POST endpoint */
237-
POST: ClientMethod<Paths, "post", Media>;
252+
POST: ClientMethod<Paths, "post", Media, Markers>;
238253
/** Call a DELETE endpoint */
239-
DELETE: ClientMethod<Paths, "delete", Media>;
254+
DELETE: ClientMethod<Paths, "delete", Media, Markers>;
240255
/** Call a OPTIONS endpoint */
241-
OPTIONS: ClientMethod<Paths, "options", Media>;
256+
OPTIONS: ClientMethod<Paths, "options", Media, Markers>;
242257
/** Call a HEAD endpoint */
243-
HEAD: ClientMethod<Paths, "head", Media>;
258+
HEAD: ClientMethod<Paths, "head", Media, Markers>;
244259
/** Call a PATCH endpoint */
245-
PATCH: ClientMethod<Paths, "patch", Media>;
260+
PATCH: ClientMethod<Paths, "patch", Media, Markers>;
246261
/** Call a TRACE endpoint */
247-
TRACE: ClientMethod<Paths, "trace", Media>;
262+
TRACE: ClientMethod<Paths, "trace", Media, Markers>;
248263
/** Register middleware */
249264
use(...middleware: Middleware[]): void;
250265
/** Unregister middleware */
251266
eject(...middleware: Middleware[]): void;
252267
}
253268

254-
export type ClientPathsWithMethod<CreatedClient extends Client<any, any>, Method extends HttpMethod> =
255-
CreatedClient extends Client<infer Paths, infer _Media> ? PathsWithMethod<Paths, Method> : never;
269+
export type ClientPathsWithMethod<CreatedClient extends Client<any, any, any>, Method extends HttpMethod> =
270+
CreatedClient extends Client<infer Paths, infer _Media, infer _Markers> ? PathsWithMethod<Paths, Method> : never;
256271

257272
export type MethodResponse<
258-
CreatedClient extends Client<any, any>,
273+
CreatedClient extends Client<any, any, any>,
259274
Method extends HttpMethod,
260275
Path extends ClientPathsWithMethod<CreatedClient, Method>,
261276
Options = {},
262277
> =
263-
CreatedClient extends Client<infer Paths extends { [key: string]: any }, infer Media extends MediaType>
264-
? NonNullable<FetchResponse<Paths[Path][Method], Options, Media>["data"]>
278+
CreatedClient extends Client<
279+
infer Paths extends { [key: string]: any },
280+
infer Media extends MediaType,
281+
infer Markers extends boolean
282+
>
283+
? NonNullable<FetchResponse<Paths[Path][Method], Options, Media, Markers>["data"]>
265284
: never;
266285

267-
export default function createClient<Paths extends {}, Media extends MediaType = MediaType>(
268-
clientOptions?: ClientOptions,
269-
): Client<Paths, Media>;
270-
271-
export type PathBasedClient<Paths extends Record<string | number, any>, Media extends MediaType = MediaType> = {
272-
[Path in keyof Paths]: ClientForPath<Paths[Path], Media>;
286+
export default function createClient<
287+
Paths extends {},
288+
Media extends MediaType = MediaType,
289+
Markers extends boolean = false,
290+
>(clientOptions?: ClientOptions): Client<Paths, Media, Markers>;
291+
292+
export type PathBasedClient<
293+
Paths extends Record<string | number, any>,
294+
Media extends MediaType = MediaType,
295+
Markers extends boolean = false,
296+
> = {
297+
[Path in keyof Paths]: ClientForPath<Paths[Path], Media, Markers>;
273298
};
274299

275-
export declare function wrapAsPathBasedClient<Paths extends {}, Media extends MediaType = MediaType>(
276-
client: Client<Paths, Media>,
277-
): PathBasedClient<Paths, Media>;
278-
279-
export declare function createPathBasedClient<Paths extends {}, Media extends MediaType = MediaType>(
280-
clientOptions?: ClientOptions,
281-
): PathBasedClient<Paths, Media>;
300+
export declare function wrapAsPathBasedClient<
301+
Paths extends {},
302+
Media extends MediaType = MediaType,
303+
Markers extends boolean = false,
304+
>(client: Client<Paths, Media, Markers>): PathBasedClient<Paths, Media, Markers>;
305+
306+
export declare function createPathBasedClient<
307+
Paths extends {},
308+
Media extends MediaType = MediaType,
309+
Markers extends boolean = false,
310+
>(clientOptions?: ClientOptions): PathBasedClient<Paths, Media, Markers>;
282311

283312
/** Serialize primitive params to string */
284313
export declare function serializePrimitiveParam(

packages/openapi-fetch/test/helpers.ts

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
import type { MediaType } from "openapi-typescript-helpers";
2-
import createClient from "../src/index.js";
2+
import createClient, { type ClientOptions } from "../src/index.js";
33

44
/**
55
* Create a client instance where all requests use a custom fetch implementation.
@@ -12,11 +12,11 @@ import createClient from "../src/index.js";
1212
* If you have too much going on in one handler, just make another instance. These are cheap.
1313
*/
1414
// Note: this isn’t called “createMockedClient” because ✨ nothing is mocked 🌈! It’s only calling the handler you pass in.
15-
export function createObservedClient<T extends {}, M extends MediaType = MediaType>(
16-
options?: Parameters<typeof createClient<T>>[0],
15+
export function createObservedClient<T extends {}, M extends MediaType = MediaType, Markers extends boolean = false>(
16+
options?: ClientOptions,
1717
onRequest: (input: Request) => Promise<Response> = async () => Response.json({ status: 200, message: "OK" }),
1818
) {
19-
return createClient<T, M>({
19+
return createClient<T, M, Markers>({
2020
...options,
2121
baseUrl: options?.baseUrl || "https://fake-api.example", // Node.js requires a domain for Request(). This restriction doesn’t exist in browsers, but we are using `e2e.test.ts` for that..
2222
fetch: (input) => onRequest(input),

0 commit comments

Comments
 (0)