fix(typescript-axios): add useErasableSyntax support for erasableSyntaxOnly compatibility#23247
fix(typescript-axios): add useErasableSyntax support for erasableSyntaxOnly compatibility#23247thobed wants to merge 4 commits intoOpenAPITools:masterfrom
Conversation
There was a problem hiding this comment.
2 issues found across 3 files
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="modules/openapi-generator/src/test/java/org/openapitools/codegen/typescript/axios/TypeScriptAxiosClientCodegenTest.java">
<violation number="1" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/typescript/axios/TypeScriptAxiosClientCodegenTest.java:213">
P2: The new erasable-syntax test does not assert that the `axios` parameter property is absent, leaving a regression gap for the feature it intends to protect.</violation>
</file>
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptAxiosClientCodegen.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptAxiosClientCodegen.java:186">
P2: `useErasableSyntax` is not reconciled with `stringEnums`, allowing generation of `export enum` despite claiming erasable-syntax compatibility.</violation>
</file>
Since this is your first cubic review, here's how it works:
- cubic automatically reviews your code and comments on bugs and improvements
- Teach cubic by replying to its comments. cubic learns from your replies and gets better over time
- Add one-off context when rerunning by tagging
@cubic-dev-aiwith guidance or docs links (includingllms.txt) - Ask questions if you need clarification on any suggestion
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
...rc/test/java/org/openapitools/codegen/typescript/axios/TypeScriptAxiosClientCodegenTest.java
Show resolved
Hide resolved
|
|
||
| if (additionalProperties.containsKey(USE_ERASABLE_SYNTAX)) { | ||
| this.useErasableSyntax = Boolean.parseBoolean(additionalProperties.get(USE_ERASABLE_SYNTAX).toString()); | ||
| additionalProperties.put(USE_ERASABLE_SYNTAX, this.useErasableSyntax); |
There was a problem hiding this comment.
P2: useErasableSyntax is not reconciled with stringEnums, allowing generation of export enum despite claiming erasable-syntax compatibility.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptAxiosClientCodegen.java, line 186:
<comment>`useErasableSyntax` is not reconciled with `stringEnums`, allowing generation of `export enum` despite claiming erasable-syntax compatibility.</comment>
<file context>
@@ -177,6 +181,11 @@ public void processOpts() {
+ if (additionalProperties.containsKey(USE_ERASABLE_SYNTAX)) {
+ this.useErasableSyntax = Boolean.parseBoolean(additionalProperties.get(USE_ERASABLE_SYNTAX).toString());
+ additionalProperties.put(USE_ERASABLE_SYNTAX, this.useErasableSyntax);
+ }
+
</file context>
| additionalProperties.put(USE_ERASABLE_SYNTAX, this.useErasableSyntax); | |
| additionalProperties.put(USE_ERASABLE_SYNTAX, this.useErasableSyntax); | |
| if (Boolean.TRUE.equals(this.useErasableSyntax) && Boolean.TRUE.equals(this.stringEnums)) { | |
| this.stringEnums = false; | |
| additionalProperties.put("stringEnums", false); | |
| } |
There was a problem hiding this comment.
Addressed in 0acb15c — added a runtime warning when both useErasableSyntax and stringEnums are enabled. The warning explains that enum declarations are not erasable syntax and recommends disabling stringEnums (the default generates erasable-compatible as const objects).
chose a warning over a hard error to avoid breaking existing configs — users may have valid reasons to enable both (e.g. migrating incrementally).
|
Thanks for the review! Finding #1 (missing axios assertion): Fixed in 740a777 — added Finding #2 (stringEnums + useErasableSyntax): Looking into this now. When |
…axOnly compatibility Add useErasableSyntax option to the typescript-axios generator so that generated base.ts avoids TypeScript parameter properties (protected/public in constructor params), which are incompatible with TypeScript 5.8's erasableSyntaxOnly flag. Closes OpenAPITools#22540
Adds assertFileNotContains check for the axios parameter property in erasable syntax mode to close the regression gap.
…e both enabled TypeScript enum declarations are not erasable syntax and will fail with erasableSyntaxOnly. Log a warning guiding users to disable stringEnums (the default generates erasable-compatible const objects).
The project enforces that Logger fields must not be static to avoid unnecessary memory consumption, since generators are used once per program lifetime (see PR OpenAPITools#8799).
Summary
Fixes #22540 — [BUG] Typescript-axios code generated is not compatible with erasableSyntaxOnly
TypeScript 5.8 introduced the
erasableSyntaxOnlycompiler option, which disallows parameter properties (protected/publicin constructor parameters). Thetypescript-axiosgenerator produces code inbase.tsthat uses parameter properties in bothBaseAPIandRequiredErrorconstructors, making the generated code incompatible with this flag.This PR adds a new
useErasableSyntaxgenerator option that, when enabled, replaces parameter properties with explicit field declarations and constructor assignments.Problem
The generated
base.tscontains parameter properties like:These fail compilation when
erasableSyntaxOnly: trueis set intsconfig.json(TypeScript 5.8+).Changes
1. Java Generator (
TypeScriptAxiosClientCodegen.java)useErasableSyntaxCLI option (boolean, defaults tofalse)additionalProperties2. Mustache Template (
baseApi.mustache){{#useErasableSyntax}}/{{^useErasableSyntax}}BaseAPIdeclaresbasePathandaxiosas explicit protected fields, assigns them in the constructor body.RequiredErrordeclaresfieldas a public field, assigns it in the constructor body.3. Tests (
TypeScriptAxiosClientCodegenTest.java)testUseErasableSyntaxConfig()test that generates code with bothuseErasableSyntax: trueanduseErasableSyntax: falseFiles Changed
Usage
Or via CLI:
Test Plan
./mvnw clean install -DskipTestsTypeScriptAxiosClientCodegenTest.testUseErasableSyntaxConfigpassesuseErasableSyntax=truecompiles witherasableSyntaxOnly: trueSummary by cubic
Adds a
useErasableSyntaxoption to thetypescript-axiosgenerator sobase.tscompiles with TypeScript 5.8erasableSyntaxOnly. Warns when used withstringEnums; tests verify parameter properties (including axios) are removed. Fixes #22540.New Features
useErasableSyntax(defaultfalse).BaseAPIandRequiredErroruse explicit fields and constructor assignments; disabled keeps existing output.stringEnums.--additional-properties=useErasableSyntax=trueor configadditionalProperties.Refactors
Written for commit 30f7c22. Summary will update on new commits.