[typescript] Utility types now considered when generating schemas (Issue #21317) #21414

DavidGrath · 2025-06-14T12:54:34Z

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 || exit
./bin/generate-samples.sh ./bin/configs/*.yaml || exit
./bin/utils/export_docs_generators.sh || exit
```
(For Windows users, please run the script in WSL)
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.x.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.

joscha · 2025-06-14T20:46:13Z

joscha · 2025-06-14T21:01:45Z

I am not following 100% what's going on in this pull request, but it seems like there is a lexer/parser for the typescript grammar being generated in order to find types and their generics? Whilst I can appreciate the lean solution, I don't think this is a future-safe, sustainable way to do this. Whilst being non-standard, it wouldn't automatically extend to future typescript grammars without manual update.

A few thoughts:
Can we use the TS parsing functionality itself - given this recently was ported to Go, could it be portable enough to be included here?
Alternatively, maybe one of the bigger AST generators like acorn has a WASM implementation that we can load?
Alternatively, is there a standard implementation of the lexer/parser part in this PR? Some widespread used Java package that has this functionality and is actively developed and we can lean on?
Alternatively, (this is already quite sad) is there a module that can load native JS in Java (Rhino comes to mind, but that is aeons old already) that we can use to load one of the AST generators and/or ts package itself inside java?
Alternatively, we can expect a dependency on node and call out to it at generation time to parse TS with its default tooling (this is probably the most future-proof but also the most overhead, unexpected solution)?

DavidGrath · 2025-06-15T17:53:30Z

Good Day, @joscha , thank you for your feedback,
These are my responses:

Can we use the TS parsing functionality itself - given this recently was ported to Go, could it be portable enough to be included here?

I'm not fully sure what you mean by "portable", but I'm also not sure what that "ported to Go" comment from the g4 file meant either, so maybe that's where the misunderstanding starts

Maybe one of the bigger AST generators like acorn has a WASM implementation that we can load?

I don't know how WASM works, so I can't really speak to this

Is there a standard implementation of the lexer/parser part in this PR? Some widespread used Java package that has this functionality and is actively developed and we can lean on?

I'm not sure if I can call it "standard", but I got the files from the Grammars repository, and the TypeScriptParser got its last update 7 months ago, on November 4 2024

Is there a module that can load native JS in Java (Rhino comes to mind, but that is aeons old already) that we can use to load one of the AST generators and/or ts package itself inside java?

I don't know much about Rhino, but from what I've looked up, I don't see anything about it supporting TypeScript

We can expect a dependency on node and call out to it at generation time to parse TS with its default tooling

I've thought about that, and then it also came to my mind that if we were to do similar for every language that this project supports, then it would be quite the amount of overhead

Overall, I get where you're coming from. The fact that I couldn't even write all my unit tests within this PR without running into a parsing bug proves that parsing - as it is currently may - not be that much of an enhancement. I could try rolling back these assertion classes and use regular String comparisons instead. Let me know if that's okay

joscha · 2025-06-15T19:55:58Z

Just a short note that I don't think the intent if this PR is bad, quite the opposite, I am just worried about the maintenance baggage that might come with it, especially because going forward we expect people making updates to the generator also to possibly deal with antlr, which can't be expected.
All I am saying is that I wish we could reuse the actual typescript parsing logic or a package which is less low level than the grammar for the language we're trying to support.
Given this is in tests and we're talking TS, maybe we can safely assume node and ecosystem to be there?
I know that the CLI package also has a nix profile, which alleviates a bunch of issues with dependency management - maybe that's an option here, too?
@macjohnny @wing328 WDYT?

macjohnny · 2025-06-17T08:52:28Z

...penapi-generator/src/test/java/org/openapitools/codegen/typescript/SharedTypeScriptTest.java

        }
    }
+
+    @Test(description = "Issue #21317")


instead of using a parser, i would suggest to just do sample file generation like https://github.com/OpenAPITools/openapi-generator/blob/master/bin/configs/typescript-fetch-allOf-nullable.yaml, resulting in https://github.com/OpenAPITools/openapi-generator/tree/master/samples/client/petstore/typescript-fetch/builds/allOf-nullable

Okay, thank you. So to be clear, would I do something like this?:

Create typescript-fetch-utility-types.yaml

Set inputSpec to the path for issue_21317.yaml

Build the sample and then test it from the samples directory by using .github/workflows/samples-typescript-typecheck.yaml as a guide, meaning I just run ./bin/ts-typecheck-all.sh in WSL

And also, should I remove everything that involves ANTLR+TypeScript for now - that is - the whole assertions package and the grammar files, or do I just remove their usage in the unit test specifically?

Create typescript-fetch-utility-types.yaml

Set inputSpec to the path for issue_21317.yaml

yes exactly

Build the sample and then test it from the samples directory by using .github/workflows/samples-typescript-typecheck.yaml as a guide, meaning I just run ./bin/ts-typecheck-all.sh in WSL

that wouldnt be needed here, reviewers would usually just inspect the generated samples. although if specific things should be tested, a typescript unit-test can be written as well.

And also, should I remove everything that involves ANTLR+TypeScript for now - that is - the whole assertions package and the grammar files

yes please, since its not needed

DavidGrath · 2025-06-18T09:11:57Z

@macjohnny , thank you for the review. Please do you think this properly addresses the original issue? @joscha , I've removed ANTLR by the advice of macjohnny. Do you think this fixes the main issue?

macjohnny · 2025-06-18T14:17:27Z

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

            return openAPIType;
        } else if (typeMapping.containsKey(openAPIType)) {
            type = typeMapping.get(openAPIType);
+            String typeWithoutGeneric = null;


please extract the typeWithoutGeneric calculation into a separate method to deduplicate the code

macjohnny · 2025-06-18T14:19:51Z

...penapi-generator/src/test/java/org/openapitools/codegen/typescript/SharedTypeScriptTest.java

+        TestUtils.assertFileContains(userModel.toPath(), "Record<string,unknown>");
+
+
+        File noRuntimeOutput = new File(output, "noruntime");


please split this into a separate test case

macjohnny · 2025-06-18T14:23:28Z

bin/configs/typescript-fetch-utility-types-without-runtime.yaml

@@ -0,0 +1,10 @@
+generatorName: typescript-fetch


thanks for adding the sample configs here, but after looking at the updated unit test, which covers the relevant code changes, i would argue the sample configs bin/configs/typescript-fetch-utility-types-without-runtime.yaml and bin/configs/typescript-fetch-utility-types.yaml and the corresponding samples should be removed, since:

the diff to other samples is minimal

the functionality is already covered in the unit test

there is no big change in the template

joscha · 2025-06-18T14:51:28Z

...penapi-generator/src/test/java/org/openapitools/codegen/typescript/SharedTypeScriptTest.java

+import org.antlr.v4.runtime.*;
+import org.antlr.v4.runtime.tree.ParseTree;
+import org.antlr.v4.runtime.tree.ParseTreeWalker;


Suggested change

import org.antlr.v4.runtime.*;

import org.antlr.v4.runtime.tree.ParseTree;

import org.antlr.v4.runtime.tree.ParseTreeWalker;

leftovers?

Oh, yes. Sorry. Thank you for that

joscha · 2025-06-18T14:53:18Z

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

                // Typescript reserved words
                "abstract", "await", "boolean", "break", "byte", "case", "catch", "char", "class", "const", "continue", "debugger", "default", "delete", "do", "double", "else", "enum", "export", "extends", "false", "final", "finally", "float", "for", "function", "goto", "if", "implements", "import", "in", "instanceof", "int", "interface", "let", "long", "native", "new", "null", "package", "private", "protected", "public", "return", "short", "static", "super", "switch", "synchronized", "this", "throw", "transient", "true", "try", "typeof", "var", "void", "volatile", "while", "with", "yield"));

+        defaultIncludes = new HashSet<>(Arrays.asList(


it's really unfortunate we maintain this list :( Also, what happens if I have a custom type with the same name that shadows any of these? My assumption is that the generated code won't be correct?

I believe toTypescriptTypeName will append "Model" for those types

joscha · 2025-06-18T14:53:36Z

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

-                "Set"
+                "Set",
+                //Utility types
+                "Awaited","Partial","Required","Readonly","Record","Pick","Omit","Exclude","Extract","NonNullable",


can we at least reuse the list from above here?

Okay, I'll make a local variable utilityTypes and use addAll

joscha · 2025-06-18T14:54:29Z

@joscha , I've removed ANTLR by the advice of macjohnny. Do you think this fixes the main issue?

yes, i think so :)

DavidGrath · 2025-06-19T08:02:44Z

Okay. Thank you both. I'll revert by the weekend or sooner

DavidGrath · 2025-06-21T18:55:46Z

@macjohnny @joscha , I've made the changes, but it seems I have to wait for some issues to be fixed from upstream as relates to meta-codegen

macjohnny

thanks!

macjohnny · 2025-06-23T07:30:23Z

@DavidGrath please update the docs and the samples and merge the latest master branch.

DavidGrath · 2025-06-25T04:15:54Z

@macjohnny Done

macjohnny · 2025-06-25T08:52:22Z

@DavidGrath can you please merge the most recent master branch and re-generate the samples and docs? looks like 5000+ files were changed because of some version string

DavidGrath · 2025-06-25T09:00:39Z

@macjohnny , please can you confirm? I should have already merged the 7.14.0 release with 600ad25 before I generated the docs

macjohnny · 2025-06-25T09:25:08Z

@DavidGrath yeah lets just try to merge the latest master, and re-generate the samples and docs, that should hopefully get rid of the unrelated version string changes

DavidGrath · 2025-06-25T10:54:27Z

@macjohnny , okay, I'll try and revert between now and Friday

DavidGrath · 2025-06-29T10:11:19Z

@macjohnny , sorry for the delay, I've merged and regenerated the 7.15-snapshot samples

DavidGrath added 4 commits June 12, 2025 17:04

Added failing test to fix Record renaming issue

a231ab6

Typescript utility types now taken into account when generating. Addr…

9adc829

…esses OpenAPITools#21317

Ran generation script

6dfc001

Remove print statements

f729421

DavidGrath changed the title ~~[typescript] Utility types now considered when generating schemas (Issue #21317)~~ [typescript] ANTLR test enhancement. Utility types now considered when generating schemas (Issue #21317) Jun 14, 2025

macjohnny reviewed Jun 17, 2025

View reviewed changes

DavidGrath added 3 commits June 17, 2025 20:28

Removed TS ANTLR. Added Utility Types Sample. Pending Regeneration

7007105

Generated samples for utility types

b8f5cdf

Samples fix

7826ba6

macjohnny reviewed Jun 18, 2025

View reviewed changes

macjohnny changed the title ~~[typescript] ANTLR test enhancement. Utility types now considered when generating schemas (Issue #21317)~~ [typescript] Utility types now considered when generating schemas (Issue #21317) Jun 18, 2025

macjohnny added the Client: TypeScript label Jun 18, 2025

joscha reviewed Jun 18, 2025

View reviewed changes

DavidGrath added 3 commits June 21, 2025 11:32

Removed TypeScript utility type samples. Other reviews applied

4a6cc5a

Merge branch 'master' into issue-21317

8f77456

Merge branch 'master' into issue-21317

600ad25

macjohnny approved these changes Jun 23, 2025

View reviewed changes

DavidGrath added 2 commits June 25, 2025 04:40

Updated samples

572dd9e

meta-codegen

a9fc23c

Merge branch 'master' into issue-21317

465e1c6

macjohnny merged commit 0e67c3a into OpenAPITools:master Jun 29, 2025
14 checks passed

DavidGrath mentioned this pull request Jul 28, 2025

[Bug][typescript-fetch] Fix missing close parenthesis in oneOf models. #21645

Merged

6 tasks

wing328 added this to the 7.15.0 milestone Aug 9, 2025

allejo mentioned this pull request Oct 29, 2025

[BUG][typescript-*] Incorrect behavior when using generics in typeMappings + top-level object schema #22251

Open

6 tasks

		TestUtils.assertFileContains(userModel.toPath(), "Record<string,unknown>");


		File noRuntimeOutput = new File(output, "noruntime");

	import org.antlr.v4.runtime.*;
	import org.antlr.v4.runtime.tree.ParseTree;
	import org.antlr.v4.runtime.tree.ParseTreeWalker;

Uh oh!

[typescript] Utility types now considered when generating schemas (Issue #21317) #21414

[typescript] Utility types now considered when generating schemas (Issue #21317) #21414

Uh oh!

Conversation

DavidGrath commented Jun 14, 2025

PR checklist

Uh oh!

joscha commented Jun 14, 2025

Uh oh!

joscha commented Jun 14, 2025

Uh oh!

DavidGrath commented Jun 15, 2025

Uh oh!

joscha commented Jun 15, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

DavidGrath commented Jun 18, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

joscha commented Jun 18, 2025

Uh oh!

DavidGrath commented Jun 19, 2025

Uh oh!

DavidGrath commented Jun 21, 2025

Uh oh!

macjohnny left a comment

Choose a reason for hiding this comment

Uh oh!

macjohnny commented Jun 23, 2025

Uh oh!

DavidGrath commented Jun 25, 2025

Uh oh!

macjohnny commented Jun 25, 2025

Uh oh!

DavidGrath commented Jun 25, 2025

Uh oh!

macjohnny commented Jun 25, 2025

Uh oh!

DavidGrath commented Jun 25, 2025

Uh oh!

DavidGrath commented Jun 29, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants