drwpow
fix(enum-values): access optional prop child props
Changes
Fixes https://github.com/openapi-ts/openapi-typescript/issues/2138 and https://github.com/openapi-ts/openapi-typescript/issues/2140
How to Review
How can a reviewer review your changes? What should be kept in mind for this review?
Checklist
- [ ] Unit tests updated
- [ ]
docs/updated (if necessary) - [ ]
pnpm run update:examplesrun (only applicable for openapi-typescript)
Deploy request for openapi-ts pending review.
Visit the deploys page to approve it
| Name | Link |
|---|---|
| Latest commit | 3302fcc6ef33a8872055ffc5bfe8cf1da448fa9d |
![netlify[bot] avatar](https://avatars.githubusercontent.com/in/13473?v=4 "Published by a pub.dev verified publisher"){kind=small}
🦋 Changeset detected
Latest commit: 3302fcc6ef33a8872055ffc5bfe8cf1da448fa9d
The changes in this PR will be included in the next version bump.
This PR includes changesets to release 2 packages
| Name | Type |
|---|---|
| openapi-typescript | Patch |
| swr-openapi | Patch |
Not sure what this means? Click here to learn what changesets are.
Click here if you're a maintainer who wants to add another changeset to this PR
![changeset-bot[bot] avatar](https://avatars.githubusercontent.com/in/28616?v=4 "Published by a pub.dev verified publisher"){kind=small}
I've committed some further modifications to handle corner cases like the following that I've found in my codebase:
FormFieldBulkStoreRequest: {
data: {
composite_fields?: {
type: "TextSingleLine" | "TextMultiLine" | "Integer" | "Boolean" | "Date" | "Datetime" | "Reference" | "Asset";
}[] | null;
}[] | null;
};
I'm not for this approach.
Currently we export consts which reference into the existing types. This PR continues with that approach, layering on a utility type to deal with the complexity of keying into those existing types.
Instead, the consts themselves should represent the basis for the type, and the paths / operations / components structures should reference the types derived from those concrete values.
This is the approach I've already taken in https://github.com/openapi-ts/openapi-typescript/pull/2051, so I'll prioritize making those changes backwards-compatible with the 7.x branch, which should address both https://github.com/openapi-ts/openapi-typescript/issues/2138 and https://github.com/openapi-ts/openapi-typescript/issues/2140.
I definitely prefer your approach, I just aimed for the easiest and quickest solution.
Does #2051 fix #941? The enum could be used to compute the union types used in paths / operations / components from its keys.
I think #2051 doesn't address #941 as it expresses the concrete values as const array, and side-steps indexed access. However, the plan is to change the representation to a const object with symmetric keys + values, and one driver of that is the desire to enable the namespace-like access.
There's still open discussion about naming. One possibility is overloading the names, (eg; Prisma approach) Another is exposing the type and the concrete value with distinct names.
@duncanbeevers why don't we base everything on top of the enum?
export enum StatusEnum {
ACTIVE = 'active',
INACTIVE = 'inactive'
}
export type Status = `${StatusEnum}`
export const StatusValues: Status[] = Object.values(StatusEnum)
export interface components {
schemas: {
Status: Status
}
}
The approach is the same of your PR but you get #941 for free.
@duncanbeevers why don't we base everything on top of the enum?
That's a great suggestion for a way to move forward without breaking existing enum export. I'll take a look at what it will take to get that behavior into a 7.x release.
That said, I'm pushing towards eliminating non-erasable syntax in the generated types, and removing enums is an important task in that effort.
@duncanbeevers I've tried v8.x but unfortunately I've found a couple of issues: depending on the enum content/length it does or doesn't generate the array and thus neither the type nor the type predicate work.
openapi:
{
"name": "dimensions",
"in": "query",
"schema": {
"type": [
"array",
"null"
],
"items": {
"type": "string",
"enum": [
"product_code",
"product_name",
"product_color_code",
"product_color_name",
"product_category_code",
"product_category_name",
"product_type_code",
"product_type_name",
"product_style_code",
"product_style_name",
"supplier_erp_code",
"supplier_name",
"policy_code",
"law_code",
"law_name",
"is_active_policy"
]
}
}
},
{
"name": "metrics",
"in": "query",
"required": true,
"schema": {
"type": "array",
"items": {
"type": "string",
"enum": [
"po_items_requirements_total",
"po_items_requirements_total_mapping",
"po_items_requirements_total_tracking",
"product_requirements_total",
"product_total_requirements_mandatory",
"product_total_requirements_optional",
"product_requirements_mandatory_completed",
"product_requirements_optional_completed",
"product_requirements_total_completed",
"policy_total_orders",
"policy_total_orders_completed",
"policy_total_orders_uncompleted",
"total_product_with_policy"
]
},
"minItems": 1
}
}
v8:
export const pathsAnalyticsDataGetParametersQueryDimensionsValues = ["product_code", "product_name", "product_color_code", "product_color_name", "product_category_code", "product_category_name", "product_type_code", "product_type_name", "product_style_code", "product_style_name", "supplier_erp_code", "supplier_name", "policy_code", "law_code", "law_name", "is_active_policy"] as const;
export type pathsAnalyticsDataGetParametersQueryDimensions = (typeof pathsAnalyticsDataGetParametersQueryDimensionsValues)[number];
export const is_pathsAnalyticsDataGetParametersQueryDimensions = get_is<pathsAnalyticsDataGetParametersQueryDimensions>(pathsAnalyticsDataGetParametersQueryDimensionsValues);
export type pathsAnalyticsDataGetParametersQueryMetrics = (typeof pathsAnalyticsDataGetParametersQueryMetricsValues)[number];
export const is_pathsAnalyticsDataGetParametersQueryMetrics = get_is<pathsAnalyticsDataGetParametersQueryMetrics>(pathsAnalyticsDataGetParametersQueryMetricsValues);
dimensions generates an array, metrics doesn't.
If you remove an element from metrics it does as well.
If you keep the same number of elements but shorten them it works as well.
Also I've noticed that we're now generating tuples instead of arrays (not sure if that change was part of your PR) but that's broken as well:
openapi:
"anyOf": [
{
"type": "object",
"properties": {
"message": {
"type": "string",
"enum": [
"Bad request. (InvalidFilterException)"
]
},
"errors": {
"type": "object",
"properties": {
"filters": {
"type": "string"
}
},
"required": [
"filters"
]
}
},
"required": [
"message",
"errors"
]
},
{
"type": "object",
"properties": {
"message": {
"type": "string",
"enum": [
"Bad request. (InvalidDimensionException)"
]
},
"errors": {
"type": "object",
"properties": {
"dimensions": {
"type": "array",
"prefixItems": [
{
"type": "string"
}
],
"minItems": 1,
"maxItems": 1,
"additionalItems": false
}
},
"required": [
"dimensions"
]
}
},
"required": [
"message",
"errors"
]
},
v7:
"application/json": {
/** @enum {string} */
message: "Bad request. (InvalidFilterException)";
errors: {
filters: string;
};
} | {
/** @enum {string} */
message: "Bad request. (InvalidDimensionException)";
errors: {
dimensions: [
string
];
};
}
v8:
"application/json": {
/** @enum {string} */
message: pathsAnalyticsDataGetResponses400ContentApplicationJsonAnyOf0Message;
errors: {
filters: string;
};
} | {
/** @enum {string} */
message: pathsAnalyticsDataGetResponses400ContentApplicationJsonAnyOf1Message;
errors: {
dimensions: [
string,
...unknown[]
];
};
}
Setting both a minItems: 1 and maxItems: 1 should generate dimensions: [ string ], maybe dimensions: [ string, ...string[] ] if you don't want to obey max but surely not dimensions: [ string, ...unknown[] ]
@drwpow can we give this another try? I know that @duncanbeevers 's v8 is supposed to reverse the approach, but development stalled and there are currently many issues with that branch. On the other hand my pr builds on the already existing approach and refines it, while I also managed to throw in many bug fixes. We can always depart from the current approach in v8 which will be a new major version and would justify the potential breakage. We already use this in production and I've added a couple of tests as well: let me know how it looks and I'll add a changelog as well.
Hey just a quick note that I am also hitting issues with enum generation!
I have tested the proposed fixes against my API specification and the problem of not being able to reference optional fields goes away \o/.
But I have encountered a different problem! That is related to enum value generation as well.
In my case I am referencing enums that where inlined in the request body part of the api specification:
export const <GeneratedName>: ReadonlyArray<FlattenedDeepRequired<paths>["name"]["post"]["requestBody"]["application/json"]["variable"]> = […variants…];
The correct path would be:
export const <GeneratedName>: ReadonlyArray<FlattenedDeepRequired<paths>["name"]["post"]["requestBody"]["content"]["application/json"]["variable"]> = […variants…];
The content sub-path is missing. This is also an issue in the currently release version so. After manually adding the path I hit #2138, and thus found this PR. Since this already addresses two enum related bugs I think fixing the path here would be great.
If not I can create a separate issue for this problem as well.
Cheers, ju6ge
@ju6ge at this point this PR fixes at least 6 different enum related issues, so I might as well add the seventh :) Please share your openapi json so I can reproduce the issue and I will try to fix it when I have some spare time available (no more enums for today, I've seen too many of them in one day: https://github.com/astahmer/openapi-zod-client/pull/349)
@darkbasic
No Problem here is a minimal example to reproduce the buggy behavior: enum-bug-openapi.json
{
"openapi": "3.1.0",
"info": {
"title": "ts-bug-api-gen-example",
"description": "",
"license": {
"name": ""
},
"version": "0.1.0"
},
"paths": {
"/": {
"get": {
"tags": [
"example"
],
"operationId": "handler",
"requestBody": {
"content": {
"application/json": {
"schema": {
"type": "object",
"required": [
"selection"
],
"properties": {
"selection": {
"type": "string",
"enum": [
"A",
"B",
"C"
]
}
}
}
}
},
"required": true
},
"responses": {}
}
}
},
"components": {}
}
With the following command invocation: cli.js enum-bug-openapi.json -o gen.ts --enum-values
The resulting generated code looks like this, which will not compile because the content part of the path is missing:
/**
* This file was auto-generated by openapi-typescript.
* Do not make direct changes to the file.
*/
export interface paths {
"/": {
parameters: {
query?: never;
header?: never;
path?: never;
cookie?: never;
};
get: operations["handler"];
put?: never;
post?: never;
delete?: never;
options?: never;
head?: never;
patch?: never;
trace?: never;
};
}
export type webhooks = Record<string, never>;
export interface components {
schemas: never;
responses: never;
parameters: never;
requestBodies: never;
headers: never;
pathItems: never;
}
export type $defs = Record<string, never>;
export interface operations {
handler: {
parameters: {
query?: never;
header?: never;
path?: never;
cookie?: never;
};
requestBody: {
content: {
"application/json": {
/** @enum {string} */
selection: "A" | "B" | "C";
};
};
};
responses: never;
};
}
type FlattenedDeepRequired<T> = {
[K in keyof T]-?: FlattenedDeepRequired<T[K] extends unknown[] | undefined | null ? Extract<T[K], unknown[]>[number] : T[K]>;
};
type ReadonlyArray<T> = [
Exclude<T, undefined>
] extends [
unknown[]
] ? Readonly<Exclude<T, undefined>> : Readonly<Exclude<T, undefined>[]>;
export const pathsGetRequestBodyApplicationJsonSelectionValues: ReadonlyArray<FlattenedDeepRequired<paths>["/"]["get"]["requestBody"]["application/json"]["selection"]> = ["A", "B", "C"];
Thanks for your efforts for improving enum handling :tada:
I've rebased because there were some conflicts. All tests pass. Any kind of feedback from the maintainers would be appreciated.
Sorry for the delay in reviewing. We’d be happy to accept this PR! Please just add a patch changeset (see comment) and we can merge & release 🙂
