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 suggestions in TS 2.6 : wrong strategy for import paths #19694

Closed
cyrilletuzi opened this issue Nov 2, 2017 · 13 comments
Closed
Labels
Bug A bug in TypeScript Domain: Completion Lists The issue relates to showing completion lists in an editor Domain: Quick Fixes Editor-provided fixes, often called code actions. Fixed A PR has been merged for this issue VS Code Tracked There is a VS Code equivalent to this issue

Comments

@cyrilletuzi
Copy link

TypeScript Version: 2.7.0-dev.20171102
VSCode Version: Insiders, same date

Code

In an Angular project, start typing any Angular code, like :

@Inp

and select the auto-import suggestion.

Expected behavior:

Should import top level export :

import { Input } from '@angular/core';

Actual behavior:

Imports deep level export :

import { Input } from '@angular/core/src/metadata/directives';

It was handled fine when just one auto-import was suggested (see #19417).

Also, there was the same issue with the Import quick fix (#15223), but which was resolved a long time ago.

@mhegazy @mjbvz

@ghost
Copy link

ghost commented Nov 2, 2017

Currently we dedupe by always pointing you to the original rather than a re-export.
We could be smarter by comparing all re-exports of the same symbol and choosing the one with the smallest path. We should probably do that in completion details rather than entries to save work, meaning the completion identifier would still use the source of the original export.

@ghost ghost self-assigned this Nov 2, 2017
@ghost ghost added the Suggestion An idea for TypeScript label Nov 2, 2017
@mhegazy mhegazy added Bug A bug in TypeScript and removed Suggestion An idea for TypeScript labels Nov 2, 2017
@mhegazy mhegazy added this to the TypeScript 2.7 milestone Nov 2, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Nov 2, 2017

We should always get the top most file, and not the source. usually the source is buried deep inside the module guts.

@mhegazy mhegazy added Domain: Completion Lists The issue relates to showing completion lists in an editor Domain: Quick Fixes Editor-provided fixes, often called code actions. labels Nov 2, 2017
@cyrilletuzi
Copy link
Author

Can't we just use the same code from the import quick fix ?

@mhegazy
Copy link
Contributor

mhegazy commented Nov 10, 2017

@mhegazy mhegazy added the VS Code Tracked There is a VS Code equivalent to this issue label Nov 10, 2017
@mhegazy
Copy link
Contributor

mhegazy commented Nov 17, 2017

Should be fixed by #20049

@nazarkryp
Copy link

@mhegazy
it is not fixed yet.
image

@ghost
Copy link

ghost commented Dec 4, 2017

@nazarkryp When I use typescript@next and type ElementRe the completion is from @angular/core. Could you double check that your editor is using typescript 2.7.0-dev.20171203?

@TedDriggs
Copy link

TedDriggs commented Feb 7, 2018

I still reproduce this on typescript 2.7.1. My folder structure is:

components
    Textarea
        index.ts
        Autosizing.tsx
        Textarea.tsx

Textarea.tsx has export default class Textarea, and then index.ts imports that and re-exports it as index.ts's default:

import AutosizingTextarea from './Autosizing';
import Textarea, { TextareaProps } from './Textarea';

export { Textarea as default, TextareaProps, AutosizingTextarea };

When I type Textarea elsewhere in the project, the suggested import path is components/Textarea/Textarea, but based on this I'd expect it to suggest components/Textarea. Is our project configured wrong to take advantage of this fix?

@ghost
Copy link

ghost commented Feb 7, 2018

@TedDriggs That's interesting, I was able to reproduce that, then noticed that I didn't have "jsx" enabled in my tsconfig.json, and after enabling that I correctly got completions from index. Could you check if that is the problem? If not, could you get a complete reproducible example with tsconfig.json included?

@TedDriggs
Copy link

@andy-ms I had a repro but when I restarted and updated VS Code it started working properly. I've got Code configured to use our project's version of TS rather than VS Code's own and we were on 2.7.1 already, but maybe the editor's autocomplete didn't know about that?

@ghost
Copy link

ghost commented Feb 8, 2018

Well, the tsserver used for autocomplete is the same as the tsserver used for everything else. But glad to hear this is working now. Possibly there was some problem with the config not being picked up.

@kumarharsh
Copy link

kumarharsh commented Mar 23, 2018

I have a JS project with jsconfig.json file which reads as so:

{
  "compilerOptions": {
    "target": "ES6",
    "module": "es2015",
    "baseUrl": "./src"
  },
  "exclude": ["node_modules", "scripts", "build"]
}

Clone this MCVE project: https://github.com/kumarharsh/ts-autoimport-bug, go to the Landing.js file and try completing the update call:

image

The autoimport item inserted is wrong - the actual import should be utils/update, but the inserted one is utils/fetch - I checked the fetch file, and there doesn't even seem to be an update word anywhere in the whole file or in the file it imports (node_modules\isomorphic-fetch\fetch-npm-node.js). So, where is the update autoimport coming from?

Also, although it doesn't happen in the MCVE project I posted above, in my real project this happens (all files in question are the same as in the MCVE):

  • Autoimport is shown as update (imported from) utils/fetch
  • Press Enter
  • Autoimport inserted as `update from 'utils/fetch'.
  • But, if I have this line already in my file: import { createSelector } from 'utils/reselect';, and I do the three steps, it inserts: import update, { createSelector } from 'utils/reselect';

See reproduction:
bug1
bug2

@TedDriggs
Copy link

@andy-ms is it possible that the fix for preferring shortest path would cause the quick-fix as you're typing to prefer ../../Interface over components/Interface? Our team avoids using relative paths, but I've noticed that lately VS Code has been suggesting relatives unless I finish typing and then press Ctrl-. to see all options.

@microsoft microsoft locked and limited conversation to collaborators Jul 31, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Bug A bug in TypeScript Domain: Completion Lists The issue relates to showing completion lists in an editor Domain: Quick Fixes Editor-provided fixes, often called code actions. Fixed A PR has been merged for this issue VS Code Tracked There is a VS Code equivalent to this issue
Projects
None yet
Development

No branches or pull requests

5 participants