Provide full import statement text from auto-import suggestions #31658
Comments
Can you please share some example code or a small example project Also, when you are typing out the import, is the entire line just |
I have a something.js - in that file I export the const something in index.js I try to import { some -> and I was hoping that autocomplete will kick in and complete the line. That doesn't happen. If I go down and type something -> intellisense suggest something -> I hit it and the import on the top of the file is inserted... And another funny thing is that - if I use flowjs whenever I define a type and intelliSense suggest it -> instead of importing on the top, the import happens inline? I guess this is another issue, but just wanted to mention it in case it is related. |
This is a feature request. The requested behavior is to support auto imports while typing out the import itself, such as: import { ab| Could show a completion for an auto import for import { abc } './abc' |
Ok. That's standard with intelliJ so I expected it with vscode as well. Sorry then. the completion should be
Since we are talking about import features. |
Agree with @compojoom |
Steps to Reproduce:
TS Server traceIt does work if i just type it out in the document without being inside of import braces. Looks like the difference is a
Typing inside importTyping inside document@RyanCavanaugh does the above help? |
@mjbvz comment above |
Spoke with @andrewbranch about this a little bit. We think this can do a lot to alleviate the ECMAScript import order syntax that a lot of users are unhappy with. Here's some rules for imports that I'm currently thinking.
Most of the same logic applies except for named exports, which I think we would not want to power this feature for. One open question is how this works when you already have existing identifiers in a named import list - do you try to filter the possible module specifiers down? It's not strictly necessary since it's probably an edge case, but it would be a nice UX improvement to try that, and if no modules are found, fall back to module specifiers based on the current identifier. Curious to hear feedback. We're also open to pull requests, though it's probably not a good first issue. |
This is great, i agree with the logic you have come up with for section 3.
I don't follow, isn't this logic already covered in 1? |
Well, potentially not |
The only scenario I can think of is if you start listing modules without providing the path.
Im interested to know how you would do this without a path. How do you know what to filter down to? Do you base this off the modules that are already in the braces? Do you then use these heuristics to both:
|
Today with auto-imports, we add an entry for every module that exports something with the appropriate name. That gets served up in the completion list. In that proposal, you could start with that logic, then do a post-pass to look at what other identifiers are in the braces, and ask "does this module export all of these other identifiers as well?" If you end up with more than 0, those are your suggestions; if you end up with none, fall back to the original list for the current identifier. |
For my understanding. What do these entries look like? Is this a list of filenames or paths for every file (module) that exports something? Or some inverted index of every module and the file it links back to.
When you say more than 0, are you referring to modules (files) that export those identifiers or the identifiers themselves? I’m assuming you’re referring to identifiers left from this file that matches as you’re within the braces so the path doesn’t matter at this point. Assuming the above is correct there’s also enough information to autocomplete the |
Because React has no default export. You can’t write |
As you said Looking at |
It's kind of like being proactive about people going down a mental garden path. A similar experience is using speech-to-text and realizing that you've started dictating a half-formed thought that isn't consistent with what you actually wanted to ask. I don't know if we need to add it right away, but I do actually like the idea. |
Right, this is the question.
Same way we know to offer it when you type |
I started prototyping this, and quickly realized that it might take a little bit of work on the editor side to do it properly. There were two possible approaches using the protocol API that exists today:
|
I figured out exactly what VS Code is doing. Using the example where you want to go from
It would solve the problem if we could make VS Code apply the second edit after the caret position instead of moving the caret to the right with the edit. However, there’s another problem: VS Code may not re-request details with every keystroke, so we can also end up in a position where we calculate the edits to be in the exact right place for the partially-typed identifier So, I don’t think this feature is possible without a new editor API to support it. And if we add something to the completions API, I’m not sure what implications that has for the LSP side of things on VS. |
@andrewbranch it sounds like this should be broken off as a separate feature (but still something that should be pursued). From what i understand #31658 (comment) is a nice to have. Is #31658 (comment) still possible to do without VSCode changes? (I would assume it is as we already do this sort of intellisense/editing in other parts of the file) |
No, every possible version of this whole feature would be affected by the problems I've outlined. It all needs editor work to be done well. |
Well, I should clarify: we could make it work only for named imports, and only for people who have brace completion turned on. We can go from |
I’m sure it’s on by default, I’ve never switched it on so I’m assuming almost everyone has this on. (I can only speak for VSCode)
When I type import { I get the closing } brace automatically, so then I start typing the names import, this alone would be a big improvement over nothing at all. I’m a sample set of one but for me it seems worth doing just for that.
Are you referring to the cursor? If so I would expect it to be on the end of the line after the insert so the latter.
Yeah I agree what was showed in the thread is better. But if that’s not possible offering autocomplete between the braces is still a big upgrade compared to what exists today, and would be a shame to throw that out. |
@uniqueiniquity pointed me toward the upcoming (3.16) LSP spec that can handle this nicely, through the combination of two mechanisms:
Together, these two features solve all of the problems I mentioned earlier. Implementing the former in typescript-langague-features for VS Code should be trivial, and it should either Just Work™ or also be trivial for VS, since VS is already speaking LSP to its TypeScript support layer. Unfortunately, I couldn’t find anything resembling the latter in |
Note that this is not completely true. Resolve lets you lazily evaluate a lot; however, most platforms will not allow you to lazily evaluate the text edit because it impacts how a completion item is evaluated at display time. For instance in the spec acknowledges this as:
|
@NTaylorMullen I thought that was the point of the new |
Notably, VS does allow for this, and should be specifying as such in the client capabilities. |
The full text of the discussion is
I read “all other properties” to mean “all properties not specified by |
Wasn't sure if VS did but I know that VSCode didn't not that long ago
I should have been more specific. Yes the
From my understanding it influences when to show/dismiss the completion list based off of the potential "replacable" completion span. I don't 100% recall though to be honest. |
Looking at the typescript-language-features source code for VS Code, I don’t see why it would be a problem to set |
I'd also imagine in VSCode that |
Indeed, vscode only supports resolving detail, documentation, and additionalTextEdits. https://github.com/microsoft/vscode-languageserver-node/blob/f1bc8dba5b8192ce8730aaeb05ce823cfff8c9b5/client/src/common/client.ts#L1514 |
This might be where the confusion lies—TypeScript does not speak LSP to VS Code. As far as I can tell, I could make any imperative change in here in conjunction with a TS Server protocol change. |
Don't you wire your "custom" LSP messages up via VSCode's hooks though? Aka their completion resolve hook? If that's the case it'd have the same restrictions. |
You’re right, I didn’t see this comment: |
Only a couple possible ways forward here. It seems like we have to calculate the full edit for all completions we offer up front, which adds a little bit of overhead¹ for module specifier resolution on every item. Options are
¹ Or currently, a lot of bit for pnpm users, a la #40584 |
Can we be concrete about the specific overhead of each item? From the top level scope, we already provide auto import completions, so what sort of work has to be done per module to generate the full statement? |
I think this might be reasonable here too. |
When auto import completions are not cached, the details request for a single item is usually an order of magnitude faster than the completion list request. Obviously there are huge variables that could affect this relationship, but that’s typical based on old notes of mine. Some portion of the details request is spent redoing work that was done in the completion list request, and the rest is spent on module specifier resolution and building display parts and code actions—so if we tried to do all that work in the completion list request, we’d be able to skip the former part. But let’s still call the details work an order of magnitude faster than the full list work—200ms for the list and 20ms for the details might be a ballpark guess for a small program. This order of magnitude relationship means the total response time for the list request could easily double if we tried to do the details work for 10 auto imports. My gut feeling is that analysis is coming out overly pessimistic for typical cases, but 10 auto imports is not very many. One of the things that can make module specifier resolution more complicated is symlinks, which play a big role in lerna, yarn workspaces, and pnpm. I don’t want to write a feature that appears miserably slow for power users with big multi-project workspaces, even if it is purely adding functionality where none exists now. I’m open to a smarter filter only if the experience feels really seamless. But part of my hesitation is that this is a non-trivial amount of work that would largely not be reusable if the limitation of not being able to lazily resolve the main edit were removed. Conversely, because normal auto-imports are lazily resolved now, this would be a pretty small feature that could reuse a lot of existing code if that were an option for these kinds of completions. So trying to resolve everything eagerly feels risky, because it’s a decent amount of work and there’s likely to be a performance/experience tradeoff. |
Edit from @DanielRosenwasser
Given
or
The suggestion is to look for auto-imports to and to fully complete the paths with
Steps to Reproduce:
2.In another file I do import { Button... and I expect auto-complete to display Buttons and hitting enter to complete the path.
But intelliSense just shows the text Buttons (with abc in front of it) and hitting enter doesn't add the path to the import.
If I try to directly import the component from a function - then auto-import correctly adds the Component to the top of the file with an import {Button} from 'correct_path'
Does this issue occur when all extensions are disabled?: Yes
The text was updated successfully, but these errors were encountered: