Go-to-definition on interfaces shadowed by namespaces opens a selection dialog instead of going to the interface, when used as one.

[rattrayalex]

Does this issue occur when all extensions are disabled?: Yes

• VS Code Version: 1.81.1
• OS Version: 13.4.1 (c)

Steps to Reproduce:

1. npm install openai
2. import OpenAI from 'openai'; type Foo = OpenAI.Chat.Completion
3. cmd-click on "Completion"
4. observe a dialog pop up, offering you the interface Completion or the namespace Completion

Desired behavior:

Jump directly to the definition of the interface Completion, since that is how it's being used there.

Context:

Libraries like stripe and openai use the pattern of shadowing interfaces with namespaces when describing large JSON objects generated from their OpenAPI specs.

This was a tradeoff made because the alternatives are worse: you'd otherwise have to generate long, Java-like names (eg; ChatCompletionChoiceMessageFunctionCall), try to come up with minimal names (which could easily clash with future objects, causing backwards-compatibility issues), move all sub-objects into other files (which'd quickly get unreadable), or define all objects inline (which makes it difficult-to-impossible to directly reference object types whose parents are nullable/optional, members of unions, etc).

[andrewbranch]

This is the case for all merged symbols; simple example: https://www.typescriptlang.org/play?#code/C4TwDgpgBAYg9nKBeKA7ArgWwEYQE4DcAUEQMZyoDOwsCyUA3gL7FHxwFQD0XUAPAFoBUAOaJgiACYQAZlAAW+CESA

I feel like whether it’s desirable is kind of a toss-up on a case-by-case basis. Sometimes, types and values are declared separately but are intended to represent one conceptual thing, as when making an enum-like or class-like construct from separate parts. Other times, the meanings are intended to be distinct. IMO it’s better to show too much than too little, but it would be nice if there were a way to deemphasize some definitions in the UI somehow.

[rattrayalex]

Hey Andrew, thanks for taking a look! I agree it'd be ideal to improve the experience in the case of many merged symbols, which can get tricky depending on the situation.

One way to improve the DX here might be to always go to the symbol as it's being used in that context.

For example, in the case of the example you linked, Foo; is being used as a variable, not a type, so I'd expect the cmd+click to go to the const Foo declaration.

Then, if the user isn't satisfied with that and was hoping to see something else, they will probably cmd+click again, this time on const Foo =, at which point opening a selector would be fantastic.

This kind of "double-click to disambiguate intent" was something I used when overriding cmd+F in the Stripe API docs – if you go to stripe.com/docs/api and press cmd+F, you'll first get our built-in search function that can help you find content in the api docs that is currently collapsed. But, if that's not what you wanted and you really wanted the browser cmd+f, you can just press it again and get it. This turned out to be great, because the "no that's not what I wanted!" reaction is often to just do the thing again.

I think this change – go to the type if it's used as a type, go to the namespace if it's used as a namespace, etc, and then if you cmd+click on a symbol declaration, show other declarations of that symbol in a selector – would make for a much better experience in many situations.

Plus, of course, if somebody wants to cmd+click to the other thing, they can add a usage of it and cmd+click to that, as with anything else.

Thoughts?

[andrewbranch]

I think that might be fine. In my experience, it’s hard to predict how much this kind of change will impact people. Hence “awaiting more feedback.” If the feedback is positive I think we could try it.

[rattrayalex]

Hey, this keeps coming up, and I'd be interested in funding someone to put up a PR with this behavior to try out and see if you like it. Would that be welcome?

[andrewbranch]

Putting up an experimental PR is always fine, with the caveat that having the work done doesn’t make the suggestion more likely to be accepted; there’s a risk of doing the work and having it go unmerged. But if you’re ok with that, go ahead! I can create an experimental playground / npm build from it.

[PierceLanternStudios]

Hey! Sorry to revive an old thread, but I now have an experimental PR out for this issue. Here is the PR: #62813.

I'm not sure if this comment constitutes "referencing" the PR in an issue as is requested by the TS bot, but if not happy to create a separate issue for it if needed.

Thanks so much for your time, and happy Thanksgiving!