[typescript-angular] Fix inner enum reference in multi-map property type#22748
[typescript-angular] Fix inner enum reference in multi-map property type#22748macjohnny merged 5 commits intoOpenAPITools:masterfrom
Conversation
|
thanks for the pr cc @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @topce (2018/10) @akehir (2019/07) @petejohansonxo (2019/11) @amakhrov (2020/02) @davidgamero (2022/03) @mkusaka (2022/04) @joscha (2024/10) |
| } | ||
|
|
||
| @Override | ||
| public void postProcessModelProperty(CodegenModel model, CodegenProperty property) { |
There was a problem hiding this comment.
is this fix specific to the angular codegen, or could it be moved to the parent class (abstract typescript) or would it even be useful to fix it in the base class?
also, would this need to be fixed in the template instead of rewriting the data type here?
There was a problem hiding this comment.
@macjohnny There is a related #20877 which suggests that it could be done in the parent class. Not sure about the base class.
Honestly, I asked GitHub Copilot to fix this for me, I don't have any in-depth knowledge of the code to decide where the fix should land - parent class, base class, template? I expected that reviewers would point me to the right place if the current fix is sub-optimal.
There was a problem hiding this comment.
2 issues found across 4 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptFetchClientCodegen.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptFetchClientCodegen.java:805">
P2: Inner enum names can be double-prefixed for child properties when a model has a parent: processCodegenProperty already prefixes isInnerEnum, and the new inheritance loop prefixes isInnerEnum again on cm.allVars, which reuses the same property objects for child vars.</violation>
</file>
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractTypeScriptClientCodegen.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/AbstractTypeScriptClientCodegen.java:1113">
P2: Map-of-enum datatype replacement expects a trailing ">", but TypeScript map types are index signatures without ">", so simple maps won’t update the inner enum name.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
|
|
||
| if (Boolean.TRUE.equals(var.isEnum)) { | ||
| // Handle both direct enum properties and inner enums (maps/arrays of enums) | ||
| if (Boolean.TRUE.equals(var.isEnum) || Boolean.TRUE.equals(var.isInnerEnum)) { |
There was a problem hiding this comment.
P2: Inner enum names can be double-prefixed for child properties when a model has a parent: processCodegenProperty already prefixes isInnerEnum, and the new inheritance loop prefixes isInnerEnum again on cm.allVars, which reuses the same property objects for child vars.
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/TypeScriptFetchClientCodegen.java, line 805:
<comment>Inner enum names can be double-prefixed for child properties when a model has a parent: processCodegenProperty already prefixes isInnerEnum, and the new inheritance loop prefixes isInnerEnum again on cm.allVars, which reuses the same property objects for child vars.</comment>
<file context>
@@ -800,7 +801,8 @@ private ExtendedCodegenModel processCodeGenModel(ExtendedCodegenModel cm) {
- if (Boolean.TRUE.equals(var.isEnum)) {
+ // Handle both direct enum properties and inner enums (maps/arrays of enums)
+ if (Boolean.TRUE.equals(var.isEnum) || Boolean.TRUE.equals(var.isInnerEnum)) {
var.datatypeWithEnum = var.datatypeWithEnum
.replace(var.enumName, cm.classname + var.enumName);
</file context>
There was a problem hiding this comment.
The concern about double-prefixing is theoretical but not an actual bug in practice. My investigation shows:
Why there's no double-prefix bug:
-
For TypeScriptFetch:
cm.parentis always null because TypeScriptFetchClientCodegen does NOT setsupportsMultipleInheritance = true. The entireif (cm.parent != null)block is dead code. -
For generators that DO support inheritance (typescript-angular, etc.): When
supportsMultipleInheritance = true, thecm.allVarscollection contains COPIES of the properties, not references to the same object instances ascm.vars.
Therefore, modifying properties in cm.vars does not affect the (different) property objects in cm.allVars. Added commit 94499fa as a proof.
...erator/src/main/java/org/openapitools/codegen/languages/AbstractTypeScriptClientCodegen.java
Outdated
Show resolved
Hide resolved
This proves that following comment is not relevant: OpenAPITools#22748 (comment)
This implements comment OpenAPITools#22748 (comment)
Type annotation in a comment should match the actual field type.
There was a problem hiding this comment.
2 issues found across 41 files
Prompt for AI agents (all issues)
Check if these issues are valid — if so, understand the root cause of each and fix them.
<file name="modules/openapi-generator/src/test/java/org/openapitools/codegen/typescript/SharedTypeScriptTest.java">
<violation number="1" location="modules/openapi-generator/src/test/java/org/openapitools/codegen/typescript/SharedTypeScriptTest.java:286">
P2: New test references `src/test/resources/3_0/issue_22748_inherited_inner_enum.yaml`, but this spec file is not present in the repository, so the test will fail to load the input spec.</violation>
</file>
<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptFetchClientCodegen.java">
<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/languages/TypeScriptFetchClientCodegen.java:805">
P2: Enum/inner-enum properties in subclasses are prefixed twice: processCodegenProperty already prefixes cm.vars, and the new allVars loop repeats it, producing double-prefixed enum names (e.g., ChildChildStatus) and invalid generated types.</violation>
</file>
Reply with feedback, questions, or to request a fix. Tag @cubic-dev-ai to re-run a review.
Fixes #20877
Fixes #22747
This PR fixes an issue where TypeScript based code generation (Angular, axios, fetch, ...) produces invalid InnerEnum type references for map properties containing arrays of inline enums.
Solution:
Added
postProcessModelProperty()override inTypeScriptAngularClientCodegenthat:datatypeWithEnumwith the correctproperty.enumNamepostProcessModels()inAbstractTypeScriptClientCodegenadd the classname prefixTesting:
PR checklist
Commit all changed files.
This is important, as CI jobs will verify all generator outputs of your HEAD commit as it would merge with master.
These must match the expectations made by your contribution.
You may regenerate an individual generator by passing the relevant config(s) as an argument to the script, for example
./bin/generate-samples.sh bin/configs/java*.IMPORTANT: Do NOT purge/delete any folders/files (e.g. tests) when regenerating the samples as manually written tests may be removed.
master(upcoming7.x.0minor release - breaking changes with fallbacks),8.0.x(breaking changes without fallbacks)"fixes #123"present in the PR description)@TiFu
Summary by cubic
Fixes invalid inner enum references across TypeScript generators when map or array properties contain inline enums. Models now use the correct enum name and compile cleanly. Fixes #20877, #22747.
Written for commit 213ee3f. Summary will update on new commits.