-
-
Notifications
You must be signed in to change notification settings - Fork 6.6k
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
[TS][Inversify] Rename map to GlobalImportOperators #3020
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this does not seem correct to me. I expect that this does not work
@macjohnny How can we make it work? :) What's the issue? (I've not tested it locally) |
@wing328 I don't understand the reason why |
@@ -8,7 +8,7 @@ import { Observable } from "rxjs/Observable"; | |||
import { Observable } from "rxjs"; | |||
{{/useRxJS6}} | |||
|
|||
import { map } from "rxjs/operators"; | |||
import { GlobalImportOperators } from "rxjs/operators"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
"rxjs/operators"
does not export a GlobalImportOperators
symbol
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah I got it....
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so we need something like
import * as GlobalImportOperators from "rxjs/operators";
and then call it by
GlobalImportOperators.map
(or something like that) ?
@wing328 I vote against renaming imported symbols and for making them reserved keywords, since I consider it a design flaw to simply insert arbitrary (user-defined) variable names into a code template. When simply renaming imports, there will always be a naming conflict, although someties less likely. See also |
Agreed the naming conflicts can still occur but should be less likely. I'll close this PR as no one has complained about naming conflicts yet. We'll revisit this when there are more demands from the users to name their variables or models as "map" or other reserved words. |
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.sh
if 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
Rename map to GlobalImportOperators so that
map
doesn't need to be a reserved word.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)