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

Auto-import quickfix : wrong strategy for import paths #15223

Closed
cyrilletuzi opened this issue Apr 17, 2017 · 2 comments · Fixed by #17536
Closed

Auto-import quickfix : wrong strategy for import paths #15223

cyrilletuzi opened this issue Apr 17, 2017 · 2 comments · Fixed by #17536
Assignees
Labels
Bug A bug in TypeScript Domain: Quick Fixes Editor-provided fixes, often called code actions. Fixed A PR has been merged for this issue

Comments

@cyrilletuzi
Copy link

TypeScript Version: 2.2.1

Actual behavior:
With the new auto-import quick fix in TS2.2 (usable in VS Code), the suggested paths for missing imports are behaving strangely.

If I already have something imported from one package, let's say :
import { Component } from '@angular/core';
then the auto-import quick fix for next missing imports from the same package will suggest the good solution :
"Add OnInit from existing import declaration from @angular/core"
resulting in :
import { Component, OnInit } from '@angular/core';

But if nothing is yet imported from one package, the suggested auto-imports from quick fix will be :
"Import OnInit from @angular/core/src/metadata"
"Import OnInit from @angular/core/src/core"
"Import OnInit from @angular/core/public_api"
"Import OnInit from @angular/core/core"
resulting for example in :
import { OnInit } from '@angular/core/core';
which is wrong.

Expected behavior:
As it's a common pattern for a package to do a main entry file (index.ts), re-exporting all the public API, and for TypeScript to have a consistent behavior, the auto-import quick fix should suggest the top-level path only :
"Import OnInit from @angular/core"
instead of low-level paths.

@mhegazy
Copy link
Contributor

mhegazy commented Apr 18, 2017

This is a bit tough, if you already have this in your file we defer to it; if not, we are trying to find the exported name in all modules that export it. we want to avoid IO as much as possible, so we do no load the packge.json, and that is what redirects @angular/core to @angular/core/core.d.ts.

To do this efficiently we need to keep a cache of package.json mappings to files, and do a reveres lookup to get the package name as a suggestions instead of the file.

@rsxdalv
Copy link

rsxdalv commented May 30, 2017

I'm not really happy with myself, but I ended up being more productive with a hotkey assigned regexp that tries to fix the import path, extending it for missed cases, and glancing to see if it acted correctly.

Matching pattern: /(import {.+} from ".+)\/(src|typings)[\/\w]*"/
Replacement out of the match (slightly simplifies the replacement syntax as it doesn't use captured groups): /\/(src|typings)[\/\w]*"/ --> "\""

For VSCode, I made this barebone extension: , which exports a command "Fix imports" that can be bound to keys. However, it requires manual installation (copy to ~/.vscode/extensions/<git clone into ./typescript-auto-import-fixer/>) as it's not published.
I know this is a hacked up solution, but I found manual editing to be slow and error prone.

Edit: published on the vscode marketplace: ext install quickfiximportsfix

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Domain: Quick Fixes Editor-provided fixes, often called code actions. Fixed A PR has been merged for this issue
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants