[TS][Axios] To fix conflict params name 'url' #2921
Conversation
Sometime we have api params named url that will conflict with imported module url named url.
|
@vellengs thanks for the PR. Can you please also run |
|
cc @TiFu (2017/07) @taxpon (2017/07) @sebastianhaas (2017/07) @kenisteward (2017/07) @Vrolijkx (2017/09) @macjohnny (2018/01) @nicokoenig (2018/09) @topce (2018/10) |
| @@ -3,7 +3,7 @@ | |||
| /// <reference path="{{apiRelativeToRoot}}custom.d.ts" /> | |||
| {{>licenseInfo}} | |||
|
|
|||
| import * as url from 'url'; | |||
| import * as $url from 'url'; | |||
There was a problem hiding this comment.
instead of renaming the parameter, I think it is better to add url to the reserved keywords. See e.g.
There was a problem hiding this comment.
@macjohnny yup, that's one way to do it.
My little suggestion is to prefix it (like what's done in the PR) in this case as "url" may be a popular choice for naming variables/classes.
Reserved keywords are usually language specified such as return in PHP that can must be prefixed/suffixed.
There was a problem hiding this comment.
but someone might also use url$ as a parameter name. I think it is a good idea to add all global symbols to the reserved keywords to avoid any naming conflicts.
There was a problem hiding this comment.
I think the dollar sign and other symbols will be removed in toVarName or toParamName as part of the sanitization.
what about using the "localVar" prefix approach?
localVar may not sound right in this case so what about "globalImportUrl"?
There was a problem hiding this comment.
"globalImportUrl" seems fine.
There was a problem hiding this comment.
@vellengs please go ahead with this approach to see if it works for you (I think it should).
There was a problem hiding this comment.
@wing328 can you build the shell
./bin/typescript-axios-petstore-all.sh.
I have promblem with Marven.
There was a problem hiding this comment.
Can you use maven wrapper instead?
./mvnw clean install
There was a problem hiding this comment.
@naelrashdeen I changed two part of 'api.mustache' and 'apiInner.mustache'
why the build samples file still have
import * as url from 'url';
any cache of shell build?
|
I've updated the samples. Let's see how the CI tests go. |
wing328
changed the title
|
@vellengs thanks again for your contribution, which has been included in the v4.0.1 release (https://twitter.com/oas_generator/status/1135534738658062336) |
|
Is 'url' mentioned above https://www.npmjs.com/package/url as there seems no other url package in npm library? And if so, when building my project using "npm run build" which actually refers to "vue-cli-service build", such error occurs. |
Sometime we have api params named url that will conflict with imported module url named url.
PR checklist
./bin/to update Petstore sample so that CIs can verify the change. (For instance, only need to run./bin/{LANG}-petstore.sh,./bin/openapi3/{LANG}-petstore.shif updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in.\bin\windows\. If contributing template-only or documentation-only changes which will change sample output, be sure to build the project first.master,4.1.x,5.0.x. Default:master.Description of the PR
(details of the change, additional tests that have been done, reference to the issue for tracking, etc)