[typescript] Add parameter to enrich maps with propertyNames and required properties #19513

Sqh3rd · 2024-09-02T11:30:02Z

Add additional parameter for the abstract typescript codegen to consider defined propertyNames and required properties, when generating maps/dictionaries.

Implements #19433

Mention TS-People:
(@TiFu @taxpon @sebastianhaas @kenisteward @Vrolijkx @macjohnny @topce @akehir @petejohansonxo @amakhrov @davidgamero @mkusaka)

PR checklist

Read the contribution guidelines.
Pull Request title clearly describes the work in the pull request and Pull Request description provides details about how to validate the work. Missing information here may result in delayed response from the community.
Run the following to build the project and update samples:
```
./mvnw clean package 
./bin/generate-samples.sh ./bin/configs/*.yaml
./bin/utils/export_docs_generators.sh
```
(For Windows users, please run the script in Git BASH)
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.
File the PR against the correct branch: master (upcoming 7.6.0 minor release - breaking changes with fallbacks), 8.0.x (breaking changes without fallbacks)
If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

macjohnny

thanks for your contribution!

macjohnny · 2024-11-25T17:04:52Z

samples/client/echo_api/typescript-axios/build/.openapi-generator/VERSION

@@ -1 +1 @@
-7.11.0-SNAPSHOT
+7.9.0-SNAPSHOT


can you please rebase on the latest master an re-generate the samples?

macjohnny · 2024-11-25T17:05:28Z

...generator/src/test/java/org/openapitools/codegen/typescript/TypeScriptClientCodegenTest.java

+        codegen.additionalProperties().put("enrichedMaps", Boolean.TRUE);
+        codegen.processOpts();
+        parentSchema.propertyNames(new StringSchema());
+        Assert.assertEquals(codegen.getTypeDeclaration(parentSchema), "{ [key: string]: string; x: string; y: string; }");


how about adding newlines between the properties in the generated code?

macjohnny · 2024-11-25T17:06:56Z

cc @joscha

joscha · 2024-11-25T22:40:58Z

...erator/src/main/java/org/openapitools/codegen/languages/AbstractTypeScriptClientCodegen.java

+                .orElse("");
+
+        if (!enrichedMaps) {
+            return "{ [key: string]: " + innerDataType + suffixOrEmpty + "; }";


should we use Record here?

joscha · 2024-11-25T22:42:46Z

...erator/src/main/java/org/openapitools/codegen/languages/AbstractTypeScriptClientCodegen.java

+        // Property names are assumed to be optional, so if we have any we need to make sure that the type of the index
+        // signature includes undefined
+        String undefinedSuffix = !propertyNames.isEmpty() && !suffixOrEmpty.contains("undefined") ? " | undefined" : "";
+        StringBuilder sb = new StringBuilder()
+                .append("{ [key: string]: ")
+                .append(innerDataType)
+                .append(suffixOrEmpty)
+                .append(undefinedSuffix)
+                .append(";");
+
+        requiredProperties.forEach(appendProperty(sb, innerDataType + suffixOrEmpty, true));
+        propertyNames.forEach(appendProperty(sb, innerDataType + suffixOrEmpty, false));
+
+        return sb.append(" }").toString();
+    }
+
+    private Consumer<String> appendProperty(StringBuilder sb, String type, boolean required) {
+        return (String propertyName) -> sb.append(" ")
+                .append(propertyName)
+                .append(required ? "" : "?")
+                .append(": ")
+                .append(type)
+                .append(";");
+    }


juts thinking out loud here - is there a cleaner way to describe this? Like for example it's own sub-template? The string concat is quite extensive here - isn't this better expressed in a template?

I'll try to extract it into it's own sub-template. I should have some time end of january, start of february to hopefully finish the changes

fravl · 2026-01-13T13:21:05Z

@Sqh3rd are you still working on this feature?

Sqh3rd · 2026-01-13T13:31:36Z

@Sqh3rd are you still working on this feature?

will take it up again a bit later today and try to finish it by the end of the week :)

Sqh3rd · 2026-01-20T19:13:10Z

So after taking way longer than I thought it would, I'm not sure about the original approach of using a parameter to define this behavior.

In order to make the changes work neatly with the templates, it's necessary to add propertyNames in IJsonSchemaValidationProperties, additionally I added the nameOnlyVars list to be able to get them in the template. (I'd really appreciate some feedback about that, not sure if the solution fits well into the codebase)

Having a parameter that either sets or clears the corresponding vars seems very intrusive, so I'm not sure whether that makes sense to do. I think that just adding this behavior is not breaking backwards compatibility, as the properties will be optional, so I think it's fine.

cubic-dev-ai

2 issues found across 7 files (changes from recent commits).

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/CodegenResponse.java">

<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenResponse.java:94">
P2: New field propertyNames is not included in equals/hashCode, so responses differing only in propertyNames are treated as equal.</violation>
</file>

<file name="modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenParameter.java">

<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenParameter.java:64">
P2: New field `propertyNames` is not propagated: copy(), equals(), and hashCode() ignore it, so cloned/equal parameters drop property-name constraints.</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
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.}

modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenResponse.java

modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenParameter.java

…ertyNames enum

cubic-dev-ai

1 issue found across 2 files (changes from recent commits).

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/CodegenParameter.java">

<violation number="1" location="modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenParameter.java:299">
P2: propertyNames added to equals/hashCode is not copied in copy(), so cloned parameters drop map key schema metadata</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
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.}

modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenParameter.java

fravl · 2026-01-21T15:35:16Z

@TiFu @taxpon @sebastianhaas @kenisteward @Vrolijkx @macjohnny @topce @akehir @petejohansonxo @amakhrov @davidgamero @mkusaka @joscha

Tagging again, cause the PR is quite old. Could somebody from the technical committee please take a look once more at @Sqh3rd's work?

Sqh3rd changed the title ~~Feature#19433~~ [Enhancement] [typescript] Add parameter to enrich maps with propertyNames and required properties Sep 2, 2024

Sqh3rd changed the title ~~[Enhancement] [typescript] Add parameter to enrich maps with propertyNames and required properties~~ [typescript] Add parameter to enrich maps with propertyNames and required properties Sep 2, 2024

Sqh3rd force-pushed the feature#19433 branch from 268d734 to e801c0d Compare November 22, 2024 13:52

macjohnny reviewed Nov 25, 2024

View reviewed changes

macjohnny closed this Nov 25, 2024

macjohnny reopened this Nov 25, 2024

joscha reviewed Nov 25, 2024

View reviewed changes

Sqh3rd closed this Jan 18, 2026

Sqh3rd force-pushed the feature#19433 branch from e801c0d to 848aecc Compare January 18, 2026 17:55

Add propertyNames and nameOnlyVars to Model and Property

f26d0fe

Sqh3rd reopened this Jan 20, 2026

cubic-dev-ai bot reviewed Jan 20, 2026

View reviewed changes

modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenResponse.java Show resolved Hide resolved

modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenParameter.java Show resolved Hide resolved

Sqh3rd added 2 commits January 20, 2026 20:58

Ensure generation of properties that only have additionalProps + prop…

d2a721e

…ertyNames enum

Appease the AI Overlords

c373269

cubic-dev-ai bot reviewed Jan 20, 2026

View reviewed changes

modules/openapi-generator/src/main/java/org/openapitools/codegen/CodegenParameter.java Show resolved Hide resolved

Sqh3rd force-pushed the feature#19433 branch from f1529b1 to c373269 Compare January 20, 2026 21:19

Fix tests and generate samples

9ebb72a

		@@ -1 +1 @@
		7.11.0-SNAPSHOT
		7.9.0-SNAPSHOT

Uh oh!

[typescript] Add parameter to enrich maps with propertyNames and required properties #19513

Are you sure you want to change the base?

[typescript] Add parameter to enrich maps with propertyNames and required properties #19513

Conversation

Sqh3rd commented Sep 2, 2024

PR checklist

Uh oh!

macjohnny left a comment

Choose a reason for hiding this comment

Uh oh!

macjohnny Nov 25, 2024

Choose a reason for hiding this comment

Uh oh!

macjohnny Nov 25, 2024

Choose a reason for hiding this comment

Uh oh!

macjohnny commented Nov 25, 2024

Uh oh!

joscha Nov 25, 2024

Choose a reason for hiding this comment

Uh oh!

joscha Nov 25, 2024

Choose a reason for hiding this comment

Uh oh!

Sqh3rd Jan 22, 2025

Choose a reason for hiding this comment

Uh oh!

fravl commented Jan 13, 2026

Uh oh!

Sqh3rd commented Jan 13, 2026

Uh oh!

Sqh3rd commented Jan 20, 2026

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

cubic-dev-ai bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

fravl commented Jan 21, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants