Add optional erasable syntax configuration to Typescript generator.

[brendandburns]

Typescript 5.8 added a new flag that optionally removes support for non-erasable syntax. This facilitates running Typescript directly in node 22 without transpiling. This PR adds a new configuration option to the Typescript generator to generate code that complies with erasable syntax only. It is off by default.

PR checklist

• [X] Read the contribution guidelines.
• [X] 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.
• [X] 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.
• [X] 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)
• [X] If your PR is targeting a particular programming language, @mention the technical committee members, so they are more likely to review the pull request.

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

[amakhrov]

Is there a reason not to ALWAYS generate erasable syntax, without extra configuration and complexity in the templates?

[brendandburns]

@amakhrov

Personally, I'm ok with that, but I made it optional because I wanted to minimize the potential impact to users of this generator.

[joscha]

I think having both options is quite the onus on future implementers and maintainers. You always have to think about both cases and what's supported might change over time as well as ecmascriot evolves, etc.

Given that we want to run future versions of this on native node, I personally would go with one of two options:

• Always produce erasable syntax
• Flick the switch to default yes for now, so all test fixtures are produced with erasable syntax and then we add node into CI to ensure we're compatible. Given that erasable syntax is always compatible with non-erasable, that seems to be the safer bet overall. Then, in the near future, I don't know, 7.13 or something, we remove the old code path.

Thoughts?

[brendandburns]

@joscha I'm fine with either option. I defer to the various typescript technical committee members opinion. Let me know the preferred solution and I will implement it in this PR.

[joscha]

@amakhrov @macjohnny thoughts?

[topce]

@brendandburns I like what you done: to be optional and by default working like before not sure all users target latest version of node... LGTM as it is.

[brendandburns]

There seem to be three different opinions here :)

Any ideas about how we should get to consensus? I'd like to get this PR merged so that we can start using it.

[joscha]

@brendandburns I like what you done: to be optional and by default working like before not sure all users target latest version of node...

Erasable syntax is a subset, right? So if we restrict the code to be always erasable we get node native support for free without impairment for anyone using more flexible transpilation?

[brendandburns]

It is a subset, however it does eliminate enum types in favor of the "object as const" pattern which shouldn't cause any breaking changes (and it didn't in my project) but I'm not 100% certain about all use cases.

[davidgamero]

the changes seem syntactically small for a gain in compatibility, making this option appealing:

@joscha

• Flick the switch to default yes for now, so all test fixtures are produced with erasable syntax and then we add node into CI to ensure we're compatible. Given that erasable syntax is always compatible with non-erasable, that seems to be the safer bet overall. Then, in the near future, I don't know, 7.13 or something, we remove the old code path.

[brendandburns]

I'm still looking for converged advice here. Can we merge as-is since it is a no-op for most users and then we could change the default (or even remove it entirely) in future releases?

[joscha]

I'm still looking for converged advice here. Can we merge as-is since it is a no-op for most users and then we could change the default (or even remove it entirely) in future releases?

Sorry, you're right. I still think we should make this default from now on, with a compat flag that people can switch on in projects where is causes issues (which I don't even think will happen). But I can live with it behind a opt-in flag as long as we add a CI step that ensures it will stay unbroken whilst almost no one is using it and/or aware of its existence.

[macjohnny]

@joscha lmk when this is ready to be merged

[joscha]

@joscha lmk when this is ready to be merged

I don't think this is just up to me? I made my point, I'd enable it by default and make it possible to disable until we've established that it is fine to keep enabled for good and we can get rid of the second code path. But I would love some more weigh-in from the TS committee, especially you for example, too?

[macjohnny]

@brendandburns sorry for the long delay and thanks for this feature! thank you all for the good arguments.

I would suggest the following to move this forward:

• remove all non-erasable code-paths (as @amakhrov suggested) except for the enum change (to keep backwards compatibility, to address @topce's concern)
  • this keeps the complexity increase of the template at a minimum
• switch on the erasable option by default (as @joscha and @davidgamero suggested)
  • people can switch it off if the enum-change causes trouble
• add a CI step that fails if its not erasable (as @joscha suggested)

makes sense? if there are no objections say within the next two weeks, @brendandburns would you want to implement that? we could also merge this PR as is, and do the above changes in a separate PR to speed things up.

[topce]

@brendandburns sorry for the long delay and thanks for this feature! thank you all for the good arguments.

I would suggest the following to move this forward:

• remove all non-erasable code-paths (as @amakhrov suggested) except for the enum change (to keep backwards compatibility, to address @topce's concern)

  • this keeps the complexity increase of the template at a minimum

• switch on the erasable option by default (as @joscha and @davidgamero suggested)

  • people can switch it off if the enum-change causes trouble

• add a CI step that fails if its not erasable (as @joscha suggested)

makes sense? if there are no objections say within the next two weeks, @brendandburns would you want to implement that? we could also merge this PR as is, and do the above changes in a separate PR to speed things up.

I like what @brendandburn done and LGTM as it is, to be merged! In TS they have syntactic sugar in constructor so it generate less code I prefer less code then more code. Also my philosophy is keep compatibility not to risk anything! But if all other think that it is better not to be optional I accept it. But not all our user use latest version of node... Deno and bun are more and more popular I think they are better option then node. But if it simplify maintenance I am OK to change it to be by default. Anyway great work @brendandburns Vox populi, vox Dei ;-)

[davidgamero]

momentum of discussion has decreased. i would be in favor of merging this PR as-is and following @macjohnny 's suggestion to pursue the remaining items in a future PR to continue the erasable default discussion later the current form of this PR is the least disruptive to users while leaving the door open to continue as @amakhrov and @joscha suggest

[EphraimHaber]

Any plans to merge this PR anytime soon?

[macjohnny]

@brendandburns can you try https://support.circleci.com/hc/en-us/articles/12504745606171-Why-am-I-seeing-the-Could-not-find-a-usable-config-yml-you-may-have-revoked-the-CircleCI-OAuth-app-message to fix the circle ci issue?

[davidgamero]

ive opened a copy of this PR with the circleCI and merge conflicts resolved at #21560

[macjohnny]

merged in https://github.com/OpenAPITools/openapi-generator/pull/21560