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-axios] switched to new URL API #6960

Merged
merged 2 commits into from
Jul 16, 2020

Conversation

sapphi-red
Copy link
Contributor

Switched to new URL API since Node.js legacy URL API is deprecated.

This will drop support Node.js below v6.0.0 (which was released Apr. 2016).

close #6859

PR checklist

  • Read the contribution guidelines.
  • If contributing template-only or documentation-only changes which will change sample output, build the project beforehand.
  • Run the shell script ./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/config/java*. For Windows users, please run the script in Git BASH.
  • File the PR against the correct branch: master
  • Copy the technical committee to review the pull request if your PR is targeting a particular programming language.

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

Copy link
Member

@macjohnny macjohnny left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@macjohnny
Copy link
Member

thanks for your contribution

@macjohnny macjohnny merged commit fce7488 into OpenAPITools:master Jul 16, 2020
@sapphi-red sapphi-red deleted the typescript-axios-new-url branch July 16, 2020 13:37
@wing328 wing328 added this to the 5.0.0 milestone Aug 9, 2020
@sapphi-red
Copy link
Contributor Author

@macjohnny
I found that this PR drops not only below v6 but below v10.
I overlooked the document that URL is not a global object until v10.
Since Node.js v7 to v9 is already EOL, I think this is not a serious problem.

@macjohnny
Copy link
Member

Thanks for the update

@breezewish
Copy link

breezewish commented Sep 16, 2020

This PR introduces a bug that const query = new URLSearchParams(localVarUrlObj.search); will produce a constant re-define syntax error when there is already a API parameter called query (in the function arguments).

@macjohnny
Copy link
Member

@breeswish this could be fixed by renaming query to queryParameters, as queryParameters is in the list of the reserved words:

reservedWords.addAll(Arrays.asList(
// local variable names used in API methods (endpoints)
"varLocalPath", "queryParameters", "headerParams", "formParams", "useFormData", "varLocalDeferred",
"requestOptions",
// 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"));

would you like to fix this?

@sapphi-red
Copy link
Contributor Author

I have made a PR #7475.

@baurine
Copy link

baurine commented Dec 28, 2020

@sapphi-red
Copy link
Contributor Author

The latest version is 5.0.0 and that PR is included in 5.0.0 or 5.0.0-beta.3.

@baurine
Copy link

baurine commented Dec 28, 2020

The latest version is 5.0.0 and that PR is included in 5.0.0 or 5.0.0-beta.3.

ok, thanks! let me try it later.

@baurine
Copy link

baurine commented Dec 31, 2020

hi @sapphi-red , sorry I didn't find the responding version in the https://www.npmjs.com/package/@openapitools/openapi-generator-cli , did version 5.0.0 and 5.0.0-beta.3 release to npm, or I should install 2.1.16?

My current version is "^1.0.15-4.3.1". After running yarn upgrade @openapitools/openapi-generator-cli, it is upgraded to "1.0.15-5.0.0-beta"

"@openapitools/openapi-generator-cli@^1.0.15-4.3.1":
-  version "1.0.15-4.3.1"
-  resolved "https://registry.yarnpkg.com/@openapitools/openapi-generator-cli/-/openapi-generator-cli-1.0.15-4.3.1.tgz#25ef943eba3c82e82379b30f858bb05ed97dae0b"
-  integrity sha512-U+sanspDmeBElVNjYHQ4U7BbSEJUQzjNKmiTzXpcEw/r93sgxmzS2Sew5t+Zj6kyN1YTvjhRjJikNcW9/bmTKA==
+  version "1.0.15-5.0.0-beta"
+  resolved "https://registry.yarnpkg.com/@openapitools/openapi-generator-cli/-/openapi-generator-cli-1.0.15-5.0.0-beta.tgz#01b4805308d10762655158f446ebc54032ca33cc"
+  integrity sha512-4Sbl/o2XbArryKnvsAtrK4UwXe0nTGnHErkYlMuIZYWBIZdEq8EeHHHHcVLlH1K3LZVP+uu18If51DcLtqVW4A==

@sapphi-red
Copy link
Contributor Author

@baurine You need to install 2.x.x for using 5.0.0-beta.2 and later. yarn upgrade won't bump versions which does not match with pattern written in package.json, so you need to run yarn upgrade --latest @openapitools/openapi-generator-cli. After installing it, set the version in config (https://www.npmjs.com/package/@openapitools/openapi-generator-cli#configuration).

@baurine
Copy link

baurine commented Jan 4, 2021

hi @sapphi-red , it works after upgrading to 2.1.16, thank you!

I just a little didn't understand, the old version just depends on ONE npm package while the new version depends on a ton of npm packages (likes nestjs, nuxtjs ...), isn't it just a wrapper of the jar? why does it need other npm packages?

image

image

@sapphi-red
Copy link
Contributor Author

I do not know about that, maybe creating an issue at the repository below would be better.
https:/OpenAPITools/openapi-generator-cli

bradymholt added a commit to ynab/ynab-sdk-js that referenced this pull request Jan 19, 2023
As part of this upgrade I had to update the Swagger template to avoid
trying to delete a non-optional property.  I referenced this PR:
OpenAPITools/openapi-generator#6960
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.

[REQ] [typescript-axios] switching to new URL API
5 participants