Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[TypeScript] Rewritten TypeScript client generator supporting fetch & jquery #6341

Merged
merged 90 commits into from
Jun 15, 2020

Conversation

TiFu
Copy link
Contributor

@TiFu TiFu commented May 17, 2020

This PR introduces a new experimental generator for TypeScript. This generator is based on the rewrite started in #802 and separates the framework-independent code from the framework-specific code.
fixes #802

At this time, the generator only supports the fetch API and JQuery. We hope that finally merging this generator will accelerate the integration of additional frameworks like axios, etc. and further integration of additional features.

Credits:
@bodograumann Thank you a lot for all your help and all the work you put into this generator!

@core Team Members:
Could you please provide some feedback on changes to the core java classes in this PR?

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project before.
  • Run the shell script(s) under ./bin/ (or Windows batch scripts under.\bin\windows) to update Petstore samples related to your fix. This is important, as CI jobs will verify all generator outputs of your HEAD commit, and these must match the expectations made by your contribution. You only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the code or mustache templates for a language ({LANG}) (e.g. php, ruby, python, etc).
  • File the PR against the correct branch: master, 4.3.x, 5.0.x. Default: master.
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

CC TS Committee @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)

CC Core Members @wing328 (2015/07) @jimschubert (2016/05) @cbornet (2016/05) @ackintosh (2018/02) @jmini (2018/04) @etherealjoy (2019/06) @spacether (2020/05)

@TiFu
Copy link
Contributor Author

TiFu commented Jun 4, 2020

@macjohnny Thanks for the feedback!

@TiFu TiFu mentioned this pull request Jun 4, 2020
5 tasks
@macjohnny
Copy link
Member

@TiFu do you think we should merge it and see how it goes?

@TiFu
Copy link
Contributor Author

TiFu commented Jun 8, 2020

I'd be fine with that. @bodograumann 's inversify PR is still open but we can merge that later without any issues.

bodograumann and others added 2 commits June 13, 2020 21:26
* Add config option to enable InversifyJS

* Add pascal case lambda for mustache

* Generate a class for each auth method

* Add service identifiers and service binder helper

* Split Configuration into interface and factory

This way we don't need to import the factory everywhere to do
typechecking.

* Define minimal interface for ServerConfiguration

* Add annotations for inversify when enabled

* Always expose list of server configurations

* Add samples and defalt tests for useInversify

* Simplify sample generation script

* Fix: Add object_params arg description to help

* Fix: Properly enable inversify with bool property

* Build tests in pom instead of prepublish

Otherwise running `npm install`, when the build failed was impossible.

* Update dependencies for inversify tests

* Test basic api service resolution

* Remove Promise and Observable prefix from exports

* Fix, RxJS: Import Observable in object params api

* Add ioc service identifier for object param api

* Add hint about unimpeded development

* Simplify api service binder syntax

* Remove default tests for inversify

* Add wrapper for easy promise based http libraries

This wrapper allows defining and injecting http libraries that do not
need to know anything about observables, especially when useRxJS is not
enabled. I will employ this in the tests for InversifyJS.

Not sure if we should also use this wrapper internally.

* Add named injects for remaining auth parameters

* Directly inject promise services without RxJS

* Add tests for api service binder

* Add convenience method to bind all api services

* Fix: Rename inversify test artifact

* Run bin/utils/copy-to-website.sh
@bodograumann
Copy link
Contributor

@TiFu Thank you for merging my PR.
I think with the Merge master afterwards, you lost my change to CONTRIBUTING.md though.

I feel adding this notice is important, because until I found out how to regenerate the samples without recompiling the java packages, I lost many increments of 3min for compiling during development. It was especially annoying when I had to go back and forth between templates and samples multiple times until I go things right.

Cf. abe3aa7 and 41f5e3e.

@TiFu
Copy link
Contributor Author

TiFu commented Jun 14, 2020

Ah, sorry for that! I re-added the changes.

@TiFu
Copy link
Contributor Author

TiFu commented Jun 14, 2020

@macjohnny I think we are good and should be able to merge into master without problems. Do you want to do the merge or should I do it this Friday?

@rgomezp
Copy link

rgomezp commented Aug 26, 2021

Is there documentation for these changes? I can't find any.

@TiFu
Copy link
Contributor Author

TiFu commented Aug 26, 2021

The best you can probably find is in #802 - which includes 1 diagram [1] on how it works on a high-level. It's not 100% up-to-date but shows the general approach. Other than that, I think it's really down to just going through the mustache templates or one generated example (c.f. the usual folders and the tests we have for this generator).

Do you have any concrete questions about it?
Happy to write something up if helpful.

[1] https://gist.github.com/TiFu/44abab1bc3a430f7ecad5fed06416445

@rgomezp
Copy link

rgomezp commented Aug 26, 2021

Thanks @TiFu ,
It would be great to have some more info on the Configuration and ConfigurationParameters interfaces needed to set up the Default API.

@TiFu
Copy link
Contributor Author

TiFu commented Aug 29, 2021

@rgomezp See #10283 for an initial draft. Happy to get your feedback and understand a bit more in what form the documentation would be most helpful.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Rewrite of TypeScript Generators
6 participants