Fix duplicate enum/type alias exports in types-plugin #52
Conversation
jgclement91
requested review from
XavierBolducMeilleur,
gdrag14 and
romanzhyn-workleap
There was a problem hiding this comment.
Pull request overview
This PR fixes a bug where the types-plugin was generating duplicate exports when using enum: true configuration with openapi-typescript. The plugin now correctly detects existing enum declarations and excludes them from type alias generation.
Key Changes:
- Added enum detection logic using TypeScript's AST API to identify existing enum declarations
- Implemented filtering to prevent duplicate type alias generation for enums
- Added comprehensive E2E test with test data and snapshot validation
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
packages/create-schemas/src/plugins/types-plugin.ts |
Added logic to collect enum declaration names from AST and filter them out when generating type aliases |
packages/create-schemas/tests/e2e.test.ts |
Added E2E test to verify enums generate without duplicate type aliases and non-enum schemas still receive aliases |
packages/create-schemas/tests/data/enums.yaml |
Created test OpenAPI spec with enum and non-enum schemas |
packages/create-schemas/tests/__snapshots__/e2e.test.ts.snap |
Added snapshot showing expected output with enums but no duplicate type aliases |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| test( | ||
| "enums.yaml / enum generation without duplicate type aliases", | ||
| async ({ expect, onTestFinished }) => { | ||
| const tempFolder = await createTemporaryFolder({ onTestFinished }); | ||
|
|
||
| const configFile = ` | ||
| export default { | ||
| openApiTsOptions: { | ||
| enum: true | ||
| } | ||
| }; | ||
| `; | ||
|
|
||
| await writeFile(join(tempFolder, "create-schemas.config.ts"), configFile); | ||
|
|
||
| const result = await runCompiledBin({ | ||
| source: join(dataFolder, "enums.yaml"), | ||
| outdir: join(tempFolder, "dist"), | ||
| cwd: tempFolder | ||
| }); | ||
|
|
||
| const typesFile = result.find(file => file.filename === openapiTypeScriptFilename); | ||
| assert(typesFile); | ||
|
|
||
| // Verify enums are generated | ||
| expect(typesFile.code).toContain("export enum Status"); | ||
| expect(typesFile.code).toContain("export enum Priority"); | ||
|
|
||
| // Verify NO duplicate type aliases for enums | ||
| const statusTypeAliasRegex = /export type Status = components\["schemas"\]\["Status"\];/; | ||
| const priorityTypeAliasRegex = /export type Priority = components\["schemas"\]\["Priority"\];/; | ||
| expect(typesFile.code).not.toMatch(statusTypeAliasRegex); | ||
| expect(typesFile.code).not.toMatch(priorityTypeAliasRegex); | ||
|
|
||
| // Verify non-enum types still get type aliases | ||
| expect(typesFile.code).toContain('export type Task = components["schemas"]["Task"];'); | ||
|
|
||
| expect(typesFile.code).toMatchSnapshot(); | ||
| }, | ||
| timeout | ||
| ); |
There was a problem hiding this comment.
The E2E test only covers enum names that are already valid TypeScript identifiers (Status, Priority). Consider adding a test case with enum schema names that contain special characters (e.g., user-status, my.org.Status) to ensure the toSafeName transformation correctly matches between enum declarations and the filtering logic. This would help verify that the enum name transformation in openapi-typescript aligns with the toSafeName function used in the filter.
| // Collect names of all enum declarations to avoid generating duplicate type aliases | ||
| const existingEnums = new Set( | ||
| ast | ||
| .filter(node => ts.isEnumDeclaration(node)) | ||
| .map(node => (node as ts.EnumDeclaration).name.text) | ||
| ); |
There was a problem hiding this comment.
The new enum filtering logic should have a unit test in tests/plugins.test.ts to supplement the E2E test. Consider adding a test case that passes code with enum declarations to typesPlugin and verifies that type aliases are only generated for non-enum schemas.
Example test structure:
test("typesPlugin with enums", async ({ expect }) => {
const plugin = typesPlugin();
const result = await plugin.transform({
config: await resolveConfig({ input: "openapi.json" }),
id: openapiTypeScriptId,
code: `export interface components {
schemas: {
Status: string;
Task: { name: string };
}
}
export enum Status {
Active = "active",
Inactive = "inactive"
}`,
filename: openapiTypeScriptFilename,
emitFile: () => void 0
});
expect(result.code).toContain('export type Task = components["schemas"]["Task"]');
expect(result.code).not.toContain('export type Status = components["schemas"]["Status"]');
});
|
I need to close this one and open a new one with a fixed branch name as the Jira validation fails |
When using the enum: true configuration option with openapi-typescript, the types-plugin was generating duplicate exports:
Description of changes
Breaking changes
No breaking change
Additional checks