feat(typescript-angular): add support for Angular V19

[thibaudsowa]

Changes

Add support for Angular 19
I have replicated PR #18916 for angular 19

Fix #20204

Technical committee

@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)

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 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.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]

Intent LGTM.

Two comments:

1. it doesn't currently generate a package.json anywhere, so we don't actually install the required angular version anywhere and check that the types work. Can you see a way to add that, so a generated sample would be picked up by https://github.com/OpenAPITools/openapi-generator/blob/master/.github/workflows/samples-typescript-typecheck.yaml and checked against an actual typescript version it is produced for and its typings.
2. It might be worth cleaning the if/else up in the Java code and use a properties file or similar to manage the atLeast, tsVersion, rxjsVersion, ngPackagrVersion and zoneJsVersion combinations?

[joscha]

1. it doesn't currently generate a package.json anywhere, so we don't actually install the required angular version anywhere and check that the types work. Can you see a way to add that, so a generated sample would be picked up by https://github.com/OpenAPITools/openapi-generator/blob/master/.github/workflows/samples-typescript-typecheck.yaml and checked against an actual typescript version it is produced for and its typings.

another option could be to include something in the mvn integration-test that gets executed for the addition.

[thibaudsowa]

Hi, I don't really understand the first point, could you develop the workflow of generation check?

[joscha]

Hi, I don't really understand the first point, could you develop the workflow of generation check?

So let's say some types change from Angluar 18 -> 19 that makes some Typescript code generated by the openapi generator invalid, then the typecheck CI step will pick that up and fail. It currently only runs on generated samples under samples/ and ONLY if there is a package.json:

openapi-generator/bin/ts-typecheck-all.sh

Lines 34 to 37 in 326f100

	if [[ ! -f "${root_dir}/${dir}/package.json" ]]; then
	# we can't really guarantee that all dependencies are there to do a typecheck...
	continue
	fi

For the current typescript-angular samples like the one you just added for v19, neither is true, so they are not typechecked, meaning that CI passes even though there might be type errors. That is, unless there is something inside mvn integration-test which I am unsure of as of this moment.

[thibaudsowa]

Ok thanks, I will try to do this.

For the second point, will it not be more flexible to add cli options for defining tsVersion, rxjsVersion, ngPackagrVersion (optional with fallback on ngVersion) and zoneJsVersion?
With that, when new angular version is released, no need to wait a new version of openapi-generator, you just have to change different versions according to angular compatibility matrix.

[joscha]

Ok thanks, I will try to do this.

Amazing!

For the second point, will it not be more flexible to add cli options for defining tsVersion, rxjsVersion, ngPackagrVersion (optional with fallback on ngVersion) and zoneJsVersion?
With that, when new angular version is released, no need to wait a new version of openapi-generator, you just have to change different versions according to angular compatibility matrix.

I actually think that makes a lot of sense, but I do not have insight on why it is the way it is.
Might just be historical.
One problem with not providing a "blessed" combination is, that it could lead to a lot of bugs that are really hard to fix and/or even compete with each other, so there is some merit in having a set of clearly defined and supported combinations. WDYT?

[thibaudsowa]

One problem with not providing a "blessed" combination is, that it could lead to a lot of bugs that are really hard to fix and/or even compete with each other, so there is some merit in having a set of clearly defined and supported combinations. WDYT?

I agree, but it could be cool to support both solution? Regular "clean" update with new release of openapi-generator or possibility to override for power user ...or impatient user ;)

I just pushed a new commit with the refactoring of Angular dependencies declaration + definition of new cli options.

What do you think?

[thibaudsowa]

Hey @joscha I just pushed one more commit to resolve your first point:

it doesn't currently generate a package.json anywhere, so we don't actually install the required angular version anywhere and check that the types work.

Could you review it and tell me if it is good for you?

Thanks :)

[joscha]

What do you think?

I think it's great, as it alleviates the need to make a pull request to this repo each time there's a new version, but as said before, it also removes the set of "blessed" versions. I saw you put them into a yaml file to keep them around, which is great, however, so maybe that's good enough? - @macjohnny WDYT?

Hey @joscha I just pushed one more commit to resolve your first point:

it doesn't currently generate a package.json anywhere, so we don't actually install the required angular version anywhere and check that the types work.

Could you review it and tell me if it is good for you?

I don't think that currently works. The code here:

openapi-generator/bin/ts-typecheck-all.sh

Line 29 in 26609e9

for dir in $(git ls-files samples | grep 'tsconfig.json$' | xargs -n1 dirname | sort -u); do

looks for samples, your added one is in the right spot ✅ (samples/client/petstore/typescript-angular-v19-provided-in-root/).
However, whilst the package.json you added is in here: samples/client/petstore/typescript-angular-v19-provided-in-root/package.json it doesn't seem to be one that is where there is the auto-generated code - the autogenerated code seems to be in samples/client/petstore/typescript-angular-v19-provided-in-root/builds/default/? So this part:

openapi-generator/bin/ts-typecheck-all.sh

Lines 30 to 32 in 26609e9

	if [[ ! -f "${root_dir}/${dir}/.openapi-generator-ignore" ]]; then
	# This is not a generated sample; skip it
	continue

will exit early.

Now I am not sure why the package.json is two directories up, maybe because of the integration etst being executed from there? But it does mean that the typecheck will not run for this sample.

A few ideas:

1. we can add another package.json to samples/client/petstore/typescript-angular-v19-provided-in-root/builds/default/
2. we can factor out

   openapi-generator/bin/ts-typecheck-all.sh

   Lines 40 to 42 in 26609e9

   	npm_install \
   	|| npm_install --force # --force because we have some incompatible peer-dependencies that can't be fixed
   	npm exec [email protected] --yes -- tsc --noEmit

   into a separate shell file and call it in CI/circle_parallel.sh (where the integration test is) for the samples/client/petstore/typescript-angular-v19-provided-in-root/
3. we can teach bin/ts-typecheck-all.sh about the angular (or even more generic, the integration test) samples when there is a package.json in folder/x and a .openapi-generator-ignore file in /x/builds/default.
4. we can not run the typecheck on these samples

Personal order of preference would be: 3,2,1,4 - interested in your thoughts?

[macjohnny]

i think we should keep the set of blessed versions. for the impatient, there is always the very easy way to write a postprocessing script.

[thibaudsowa]

@joscha Thanks for your comments! But I'm a bit lost with the package.json things. What is the purpose of that check?
The generated sample do not create package.json, so do I need to create it? If yes what dependencies do I need to add? Angular? OpenAPI?

What I did is generating an Angular app and importing open api sample to check if everything is fine. Which it is. Would it be more secure to check if the app compile and launch unit test for example?

@macjohnny In that PR there is the two alternatives: the "blessed versions" (refactored with clean config file) and the "fine tuned version" with cli options. Would you like that I remove that new extra options? Or the two possibilities can be complementary ?

[joscha]

@joscha Thanks for your comments! But I'm a bit lost with the package.json things. What is the purpose of that check?

It makes sure the generated Typescript compiles with the dependency versions specified and there are no type errors.

The generated sample do not create package.json, so do I need to create it? If yes what dependencies do I need to add? Angular? OpenAPI?

It's okay, let's go with 4) for now. Nothing to do, your sample is not worse or different than all the other angular samples.

What I did is generating an Angular app and importing open api sample to check if everything is fine. Which it is. Would it be more secure to check if the app compile and launch unit test for example?

Yes, the type check would necessarily be implicity part of that if it can be executed.

I am happy with the current state. If we improve it, we should really improve all thos samples, not only one anyway.

[macjohnny]

@thibaudsowa thanks for your contribution!