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

[Suggestion] Display the name of type alias in quick-info consistently #13095

Closed
HerringtonDarkholme opened this issue Dec 21, 2016 · 4 comments
Labels
Domain: Quick Info e.g. hover text, tool-tips, and tooltips. Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds

Comments

@HerringtonDarkholme
Copy link
Contributor

HerringtonDarkholme commented Dec 21, 2016

I have tried my best to search existing issues. Pardon me if it is still duplicated.

Currently quick-info will always display the name of an interface no matter if it is imported from other modules or declared in the same file. This greatly shortens quick-info message length.

Type alias behaves partly like interface: if type alias is declared in a non-module file, quick-info displays its alias name.

type Alias<K> = {
  property1: K
} | {
  property2: number
}

var a: Alias<string> // popup as Alias<string>

Online example

However, an import statement will make type alias displayed as its type literal expansion.

import 'test'
type Alias<K> = {
  property1: K
} | {
  property2: number
}

var a: Alias<string> // displayed as `{property1: string}|{property2: number}`

example

When used with mapped types, type alias' full expansion soon becomes very lengthy. For my extreme case, one type can have 7500+ lines of code after expansion.

After mapped types have more adoption, displaying type alias, in my speculation, might be a real world usability issue. It might even strangulate language service if the type is too large to show. For example, #12904 is using mapped type for MongoDB API. While all the typing looks proper and valid to me, the quick-info for MongoFilter<Person> is quite daunting. Also, compiler error reports the alias name, not expansion.

It would be fine if TypeScript can show alias name in quick-info consistently and concisely.

@HerringtonDarkholme HerringtonDarkholme changed the title [Suggestion] Display the name of type alias in quick-info [Suggestion] Display the name of type alias in quick-info consistently Dec 21, 2016
@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript In Discussion Not yet reached consensus Suggestion An idea for TypeScript and removed Bug A bug in TypeScript labels Dec 21, 2016
@RyanCavanaugh
Copy link
Member

This is a bit tricky due to implementation details.

Internally types are interned, and aliases are eagerly resolved, so a resolved definition of Alias<K> in foo.ts is indistinguishable from an identical declaration Alias2<K> in bar.ts.

When a type alias exists in the global scope, we store the first-seen alias with the type itself. This is a good heuristic (namely, aliases from lib.d.ts always "win" over user definitions of the same type). But we don't store the names of module-declared aliases because the same type alias might have different local names in different modules.

We could potentially keep a per-module type->alias lookup.

@RyanCavanaugh RyanCavanaugh added Bug A bug in TypeScript and removed In Discussion Not yet reached consensus Suggestion An idea for TypeScript labels Jan 10, 2017
@RyanCavanaugh
Copy link
Member

I was wrong about how the alias storage works - we don't care if it's global or not. Instead the problem is that the type-to-string code checks that the alias is reachable from the global scope, rather than the scope of the quickinfo request. It may be an issue of just using the correct reachability analysis depending on the QI invocation location. Investigate as Bug and see what happens

@kpdonn
Copy link
Contributor

kpdonn commented Apr 24, 2018

Heads up I submitted a PR that would fix this at #23638.

@DanielRosenwasser DanielRosenwasser added the Domain: Quick Info e.g. hover text, tool-tips, and tooltips. label May 24, 2018
@RyanCavanaugh RyanCavanaugh added the Too Complex An issue which adding support for may be too complex for the value it adds label Aug 15, 2018
@RyanCavanaugh
Copy link
Member

Nice to have but there is a lot of implementation complexity behind this. Still open to targeted PRs but I don't think we can make any specific promises here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Domain: Quick Info e.g. hover text, tool-tips, and tooltips. Suggestion An idea for TypeScript Too Complex An issue which adding support for may be too complex for the value it adds
Projects
None yet
Development

No branches or pull requests

5 participants