feat: Support generating erasable syntax enum-style types #1278
Conversation
|
There was a problem hiding this comment.
Pull Request Overview
Adds support for generating “erasable syntax” enums via a new CLI flag so consumers can opt into generating object‐literal enum types instead of unions or TS enums.
- Introduce
generateErasableSyntaxEnumsflag in both config declaration and defaults - Extend parser/formatter logic and EJS template to branch on the new flag
- Add fixture schema, snapshot test, and basic test to validate output
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| types/index.ts | Add generateErasableSyntaxEnums to GenerateApiConfiguration |
| src/configuration.ts | Add default generateErasableSyntaxEnums in CodeGenConfig |
| templates/base/enum-data-contract.ejs | Add EJS branch for erasable syntax enums |
| src/schema-parser/schema-formatters.ts | Handle generateErasableSyntaxEnums in content formatter |
| src/schema-parser/base-schema-parsers/enum.ts | Include generateErasableSyntaxEnums when choosing typeIdentifier |
| tests/spec/enumsErasableSyntax/schema.json | New Swagger schema covering various enum scenarios |
| tests/spec/enumsErasableSyntax/basic.test.ts | New Vitest integration test for erasable syntax enums |
| tests/spec/enumsErasableSyntax/snapshots/basic.test.ts.snap | Expected output snapshot for erasable syntax enums |
Comments suppressed due to low confidence (2)
src/configuration.ts:56
- The JSDoc comment for
generateErasableSyntaxEnumsis incomplete. Add a clear description, e.g./** CLI flag to generate erasable‐syntax enum types */.
/** CLI flag, */
tests/spec/enumsErasableSyntax/basic.test.ts:23
- import.meta.dirname is not defined in ESM modules; you should use fileURLToPath(import.meta.url) or a __dirname polyfill to locate the schema file.
input: path.resolve(import.meta.dirname, "schema.json"),
| @@ -5,6 +5,12 @@ const { name, $content } = contract; | |||
| %> | |||
| <% if (config.generateUnionEnums) { %> | |||
| export type <%~ name %> = <%~ _.map($content, ({ value }) => value).join(" | ") %> | |||
| <% } else if (config.generateErasableSyntaxEnums) { %> | |||
| export const <%~ name %>: { [key: string]: string } = { | |||
| <%~ _.map($content, ({key, value}) => `${key}: "${value}"`).join(",\n") %> | |||
There was a problem hiding this comment.
This wraps every enum value in quotes, turning numeric values into strings. You should render numeric literals without quotes (e.g., check typeof value or use separate formatting for numbers vs. strings) to preserve original types.
| <%~ _.map($content, ({key, value}) => `${key}: "${value}"`).join(",\n") %> | |
| <%~ _.map($content, ({key, value}) => | |
| `${key}: ${typeof value === "number" ? value : `"${value}"`}` | |
| ).join(",\n") %> |
Copilot uses AI. Check for mistakes.
There was a problem hiding this comment.
I think Copilot's catch here is worth looking at. Stringly typed enums might be a regression in some use cases.
Looking at the test cases you added though, seems to be working as intended? 🤔
|
@smorimoto After some additional testing, I'm absolutely certain this is not behaving the way I had originally assumed - what is the best way to understand the relationship between the schema formatters and the templates? Apologies for the interruption! |
|
This needs more time to cook - I'm going to close this for now and work on it before re-opening. |
First stab at supporting #1040 via a new CLI flag. It currently takes lower precedence than generateUnionEnums - so if both are set, unions will always be written out.
I'm not 100% confident I've fully grokked the intended structure of the added code, so please point out any deficiencies that have been added, and I'll try to correct them.
Cheers!