Skip to content

Use existing star import instead of adding new import #19628

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

Closed
mjbvz opened this issue Oct 31, 2017 · 6 comments
Closed

Use existing star import instead of adding new import #19628

mjbvz opened this issue Oct 31, 2017 · 6 comments
Labels
Fixed A PR has been merged for this issue Suggestion An idea for TypeScript VS Code Tracked There is a VS Code equivalent to this issue

Comments

@mjbvz
Copy link
Contributor

mjbvz commented Oct 31, 2017

From @chrmarti on October 31, 2017 16:58

  • VSCode Version: Code - Insiders 1.18.0-insider (81cca6c, 2017-10-31T06:24:27.011Z)
  • OS Version: Linux x64 3.13.0-108-generic
  • Extensions: none

Testing #37177

Steps to Reproduce:

  1. Use a global completion when a star import is already in the file:
import * as b from "./bar";

foo|
  1. Result:
import * as b from "./bar";
import { foo } from "./bar";

foo|
  1. It would make sense to reuse the existing import.

Copied from original issue: microsoft/vscode#37314

@mjbvz mjbvz self-assigned this Oct 31, 2017
@mjbvz mjbvz added VS Code Tracked There is a VS Code equivalent to this issue and removed javascript labels Oct 31, 2017
@mjbvz mjbvz removed their assignment Oct 31, 2017
@mhegazy mhegazy added the Suggestion An idea for TypeScript label Nov 1, 2017
@mhegazy mhegazy assigned ghost Nov 1, 2017
@mhegazy mhegazy added this to the TypeScript 2.7 milestone Nov 1, 2017
@grantila
Copy link

What if two modules share exported function names, should import order matter? Or should that automatically be an error? So, if I add an exported function in a file, all of a sudden other code can break and fail to compile? This magic will cause code to be much harder to follow, to know where magic functions come from. It will force people to use IDE:s to be at all productive.

How about (what probably most ES:ers and TS:ers have tried to do and failed):

import * as b, { foo } from "./bar";

I simply cannot understand why the above is not yet legal. It would solve your problem and introduce zero issues.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 21, 2017

How about (what probably most ES:ers and TS:ers have tried to do and failed):

We are not planning on adding any new syntax to modules that are not in the ECMAScript standard already. Please see design goals, specifically 6 and 8.

@grantila
Copy link

@mhegazy makes perfect sense, this surely is an upstream (ES) design flaw.

But then, changing how it is used (polluting the global scope with magic functions) shouldn't be done either, but you added this to the 2.7 milestone. Why? This is not aligned with point 6.

@mhegazy
Copy link
Contributor

mhegazy commented Nov 22, 2017

But then, changing how it is used (polluting the global scope with magic functions) shouldn't be done either, but you added this to the 2.7 milestone. Why? This is not aligned with point 6.

I am not sure i understand what you mean.. what changing the scopes, and what is polluting the global scope?

@grantila
Copy link

Well, I'm not sure what the | is supposed to mean in the examples, perhaps this is just some editor auto-completion thing. The title of this issue, and the examples, seem to suggest that one shouldn't have to explicitly import foo. It should be globally available anyway. That is pollution, and will cause the issues I described, e.g. with collisions etc.

@ghost
Copy link

ghost commented Dec 4, 2017

@grantila This issue is for an import fix, meaning the sample code in the original post would not compile and would need to be fixed by changing foo to b.foo. The behavior of the language won't change, but completions will have a different effect in the editor.

@ghost ghost closed this as completed in #20457 Dec 21, 2017
@ghost ghost added the Fixed A PR has been merged for this issue label Dec 21, 2017
@microsoft microsoft locked and limited conversation to collaborators Jun 21, 2018
This issue was closed.
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Fixed A PR has been merged for this issue Suggestion An idea for TypeScript VS Code Tracked There is a VS Code equivalent to this issue
Projects
None yet
Development

No branches or pull requests

3 participants