Improve "suggestions show path" for "node_modules"

[heroboy]

Search Terms

TypeScript suggestions show path
TypeScript can provide completions that also add an import statement. However, when there are multiple symbols with the same name, it is hard to pick the right completion. This release makes this simpler because paths of auto-import completions are shown with the label.

Suggestion

As description above. I think the path should be show as import form, not show path like 'node_modules/xxx/index' but 'xxx'
And sort it as the "show fixes" menu I post below.

Use Cases

Here is the current result

It is not very easy to choose which one to import Table, because the path is too long and very same. (Actually I want the 3rd one, it is 'antd')

---

Here is the "show fixes" menu

It is much more clear.

---

Here is the "show fixes" menu, when I already import something from 'antd'

Examples

no example.

Checklist

My suggestion meets these guidelines:

• This wouldn't be a breaking change in existing TypeScript/JavaScript code
• This wouldn't change the runtime behavior of existing JavaScript code
• This could be implemented without emitting different JS based on the types of the expressions
• This isn't a runtime feature (e.g. library functionality, non-ECMAScript syntax with JavaScript output, etc.)
• This feature would agree with the rest of TypeScript's Design Goals.

[andrewbranch]

That string was never meant to be shown to humans—at this stage, we don’t know that node_modules/antd/lib/index is resolvable as antd. Doing that takes a good bit more work than we’re able to do for every item in a completion list, which is why we can give you a definite answer in the codefix menu (because we only have to compute possible module specifiers for one symbol, not potentially hundreds). It looks like VS Code decided to show it here: microsoft/vscode#98228.

I think it would be an easy win to drop the leading node_modules/, because I do think this is better than showing nothing. Doing something more sophisticated is sort of tracked #36265, but it covers a lot more than just auto-imports.

But, TypeScript needs that string to contain node_modules/, so I think this is a VS Code issue. Assigning to @mjbvz for input on what to do with it after the holidays.

[typescript-bot]

This issue has been marked as 'External' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

[typescript-bot]

This issue has been marked as 'External' and has seen no recent activity. It has been automatically closed for house-keeping purposes.

[andrewbranch]

I’ve been experimenting with different ways we can do this while minimizing the interactive response time for completions. The options we have can be broadly divided into two buckets: ones where the response is complete and ones where it is incomplete.

Complete

A complete response means that, like today, we generate the full list of completions when they’re requested, that list is filtered down by the client, and the client never asks for an update to that list; it only asks for additional details for single items as they’re focused. The only way we can significantly improve (uncached) performance for complete responses is to limit the number of auto-imports that show module specifiers (most simply by limiting the number of auto-imports even included in the list).

Note: all response times reported are with completely cold caches. Subsequent requests, especially in the same file, are faster.

• Without changing how many auto-imports are offered or limiting how many get resolved module specifiers, the time penalty for the CompletionInfo command is +50–100%. (It’s highly dependent upon project structure.)
• By changing the filter to the same thing that import statement completions (and now Python auto-imports) use (first character typed must be the first character of the symbol name, then fuzzy match), the penalty drops to about +0–25%.
• By changing the filter to be a looser variation on the above (first character typed must be the first character of a “word” of the symbol, split into snake/camel case words), the penalty is about +13–50%.

These strategies are extremely easy to implement and minimize number of requests and message size of requests, which could be something to think about for remote scenarios, but is usually not one of our biggest considerations. They also feel the snappiest after the response comes back since additional typing only results in client-side filtering.

Incomplete

Incomplete responses, by contrast, result in the client re-requesting completions after more typing occurs. The idea is that we could resolve module specifiers only for the top N auto-imports, and if there are more without resolved module specifiers, we mark the response as incomplete. Then, when the user continues typing, we get another request. At this point, we have more identifier characters to filter by, and we can pull the previous response out of a short-term MRU cache, so we could fill in another N module specifiers for items that match the typed identifier until the response is fully filled in with module specifiers.

I tried a very quick, simple demo of this, and it seemed to work pretty well. The advantage of this general approach is that we would be able to offer all auto-imports that are offered today, and show module specifiers for them once enough characters have been typed that the list is sufficiently small, and we could do this for near-zero performance impact on the time it takes for the list to appear. The disadvantages are higher complexity, much higher cumulative TS Server traffic and response time, and the fact that someone simply scrolling the initial list with incomplete items would see an inconsistency between the ones at the top that have resolved module specifiers and the ones at the bottom that don’t.

[mjbvz]

Adding @jrieken since this is related to #36265 and the VS Code label2 api proposal

[andrewbranch]

I should also clarify that I outlined only the options that are available to us now with minimal or no API/client changes. The third way would be to request pages of label2 entries based on what’s visible in the list UI at a given time, so it would update both while typing and while scrolling. But based on the earlier discussion in #36265, my impression was that’s not feasible, which is why I’ve been looking into solutions that can be implemented on the TS side. We also want any solution to be doable in LSP.

[jrieken]

But based on the earlier discussion in #36265, my impression was that’s not feasible, which is why I’ve been looking into solutions that can be implemented on the TS side. We also want any solution to be doable in LSP.

Yes - we have currently no plans to lazy load labels of completion items.