[typescript-node] Fix invalid type when using node@10 and ES5 #7133
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
👍 Thanks for opening this issue! The team will review the labels and make any necessary changes. |
|
@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) |
gasugesu
changed the title
gasugesu
force-pushed
the
feature/typescript-node/change_clientresponse_into_incommingmessage
branch
from
92d1c3d to
661653b
Compare
|
@gasugesu your proposal sounds reasonable to me. I think it is backwards compatible. if nobody has any objections, I think we can merge it. |
jimschubert
added a commit
that referenced
this pull request
Aug 12, 2020
* master: (27 commits) [WIP][python-exp] Force camelization of imports (#7186) Fixes #6942: Added ability to prepend a basePath to typescript-redux-query generators (#6943) [Typescript] Import path is invalid in windows. (#7175) Fix JaxRS Spec generator additional model types (#7180) [python{,-experimental}] Obey floating point timeouts provided to RESTClientObject.request(...) (#7154) [C#] Switch the spec to OAS v3 from v2 (#7176) [Javascript] Fixing the handling of non primitive types in paramToString (#7171) (#7172) [typescript-node] Fix invalid type when using node@10 and ES5 (#7133) Minor fix to github workflow badge [gradle] Enabling up-to-date checks and gradle caching for openapigenerator tasks (#6716) feat(csharp-netcore): Adding response headers to the ApiException (#7169) [ci] Verify supported JDK versions on master push (#7085) Issue #6830: Java server - Add getter to ApiException templates (#7150) update kotlin samples [Kotlin] Make ApiClient in jvm-retrofit2 be able to use own OkHttpClient (#6999) Sttp - wrap query params (#6884) Add a link to https://medium.com/@everisBrasil blog post (#7160) [C#][netcore] fix regular expression when it contains double quotes (#7147) remove duplicated cancellationToken in comment (#7148) update samples ...
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
ECMA proposal dose not determine Whether "
http.ClientResponse" or "http.Incomming" should be usedHello developers.
Thank you for maintaining the OpenAPI Generator!!
I have a suggestion for you today.
I have been using "typescript-node" recently, and I noticed a problem.
Even though I use node @ 10, es5, and Typescript 3.7 in my development, the correct type will not be generated unless I include the "--additional-properties supportsES6 = true" flag when generating the "typescript-node" code!
If this flag is not set, a "http.IncomingMessage" type should be created, but a "http: ClientResponse" type will be created.
So I did some research and found that the version of node, not the ECMA Proposal, should determine whether the "http.IncomingMessage" or "http: ClientResponse" type should be used.
As you know, the ECMA Proposal is just a proposal for a language specification and makes no mention of which proposals should be implemented in which node versions.
So I think this interface should not be determined by whether it is es6 or something else, but by which node version you are using.
Therefore, I would like to suggest the following.
Replace all "http: ClientResponse" with "http.IncomignMessage". Originally, "http: ClientResponse" is just an alias for "http.IncomignMessage" and you don't have to force yourself to use "http: ClientResponse".
If 1 is not possible for some reason, add a new flag indicating the version of your node. This will not confuse non-es6 users, such as those using es5.
By the way, the link below shows that "http: ClientResponse" is only an alias for "http.IncomignMessage" from node@v0.
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/node/v0/index.d.ts#L576
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/node/v6/base.d.ts#L877
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/node/v7/base.d.ts#L880
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/node/v8/base.d.ts#L1103
https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/types/node/v9/base.d.ts#L1185
PR checklist
./bin/generate-samples.shto update all Petstore samples related to your fix. 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*. For Windows users, please run the script in Git BASH.master