openapi-ts / openapi-typescript

Generate TypeScript types from OpenAPI 3 specs
https://openapi-ts.dev
MIT License
5.63k stars 455 forks source link

Deduplicate enums #1683

Open imranbarbhuiya opened 4 months ago

imranbarbhuiya commented 4 months ago

Description Hi, when using the --enum flag, the same enum gets generated multiple times if it's used multiple times, can we get a new flag or something to not generate duplicate enums when the keys and values of the enum match an existing enum?

Proposal A new flag that dedupes the enums will be useful. Currently, I'm patching the lib and keeping the stringified version of keys-values in a map and checking if it exists then returning instead of creating a new enum. I can create a PR to implement this feature first creating this issue to discuss how we can proceed with the naming and all

Checklist

My diff ```diff diff --git a/dist/lib/ts.js b/dist/lib/ts.js index 12edcb37af95c3f1f73fafac145d27aba44571b3..2af43b8a46e80c09c1cf38ea0fb8d23b23f47390 100644 --- a/dist/lib/ts.js +++ b/dist/lib/ts.js @@ -111,10 +111,15 @@ export function tsDedupe(types) { } return filteredTypes; } +const visited = new Map(); export function tsEnum(name, members, metadata, options) { let enumName = sanitizeMemberName(name); enumName = `${enumName[0].toUpperCase()}${enumName.substring(1)}`; - return ts.factory.createEnumDeclaration(options ? tsModifiers({ export: options.export ?? false }) : undefined, enumName, members.map((value, i) => tsEnumMember(value, metadata?.[i]))); + const hash = `${members.map((v, i) => `${metadata?.[i]?.name ?? String(v)}:${metadata?.[i]?.description ?? ""}`).join(",")}`; + if(visited.has(hash)) return visited.get(hash); + const data = ts.factory.createEnumDeclaration(options ? tsModifiers({ export: options.export ?? false }) : undefined, enumName, members.map((value, i) => tsEnumMember(value, metadata?.[i]))); + visited.set(hash, data); + return data; } export function tsArrayLiteralExpression(name, elementType, values, options) { let variableName = sanitizeMemberName(name); diff --git a/dist/transform/schema-object.js b/dist/transform/schema-object.js index 0ae8a4223850fd722e0e99db34e2c6e09cfe880e..ccfc6041488e78a5925fc15546db76d86bd83802 100644 --- a/dist/transform/schema-object.js +++ b/dist/transform/schema-object.js @@ -42,7 +42,7 @@ export function transformSchemaObjectWithComposition(schemaObject, options) { const enumType = tsEnum(enumName, schemaObject.enum, metadata, { export: true, }); - options.ctx.injectFooter.push(enumType); + if(!options.ctx.injectFooter.includes(enumType)) options.ctx.injectFooter.push(enumType); return ts.factory.createTypeReferenceNode(enumType.name); } const enumType = schemaObject.enum.map(tsLiteral); ```
Amixx commented 2 months ago

When is this planned? I need this feature, and am willing to try to implement it.

Amixx commented 2 months ago

@phk422 Thank you for implementing this feature! However, I found that it does not work - with this flag openapi-typescript command runs forever (and when the flag is removed it finished very quickly). Here is a minimal reproduction, please take a look: https://stackblitz.com/edit/vitejs-vite-8qzk9p?file=package.json Try running npm run success and npm run infinite-loading and you will see the issue.

imranbarbhuiya commented 2 months ago

The problem with the current implementation is that the name of the enum isn't stable. I initially provided the diff but didn't implement it due to this issue. the issue is if we add a new route that references an enum from the existing routes and the current route is alphabetically above the previous route, the enum name will change and it'll require a new change in our code to change the enum name. Can we add a non-standard prop like x-enum-name and use that for the enum name if it exists to solve this issue? or if something similar already exists then let me know

cc: @phk422 @drwpow

drwpow commented 2 months ago

Thanks for raising! In that case, I’ll keep this ticket open and just use this to track the bug now that the feature exists.

phk422 commented 2 months ago

@phk422 Thank you for implementing this feature! However, I found that it does not work - with this flag openapi-typescript command runs forever (and when the flag is removed it finished very quickly). Here is a minimal reproduction, please take a look: https://stackblitz.com/edit/vitejs-vite-8qzk9p?file=package.json Try running npm run success and npm run infinite-loading and you will see the issue.

This is indeed an issue. I suspect it's related to CLI argument parsing. I'll investigate if there's a way to resolve it. You can try placing other optional parameters at the end, like this:

openapi-typescript api/schema.yaml -o schema/schema.ts --enum --dedupe-enums

This should work correctly.