Skip to content

fix(47134): Inconsistent QuickInfo on type-only imported symbols #47138

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

Merged
merged 1 commit into from
Dec 14, 2021

Conversation

lowr
Copy link
Contributor

@lowr lowr commented Dec 13, 2021

Fixes #47134

I think type-only (re)exports should behave the same so included the fix for that, but I can revert it if it's not desirable.

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Dec 13, 2021
Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for looking into this! However, I think it probably makes the most sense to make the opposite change—I would slightly prefer for these to show all meanings they encompass, not just their type meaning. I suspect this utility function should just lose its special casing of type-only imports and everything would work fine.

I think I originally wrote this in the first iteration of type-only imports, which shipped with a beta version of TS but was then rewritten before the RC. In that version, type-only imports were actually a special kind of alias that filtered the meaning of their target down to just types. So, you couldn’t reference the value meaning of a type-only import even if it was in a non-emitting position, like

import type { SomeClass } from "./mod";
type SomeClassConstructor = typeof SomeClass;

In the beta version, this was an error, but there was quickly strong demand to be able to write this, and it made sense, so I switched over to what we have today. I’m pretty sure this special quick-info handling code is a relic of the original version, where the type meaning really was the only accessible meaning. I had totally forgotten this code existed until this PR. 😁

@lowr
Copy link
Contributor Author

lowr commented Dec 14, 2021

Thanks for your detailed explanation! Actually I didn't know that you can reference the value meaning of a type-only imports/exports until a few hours ago but came to think that it should be the other way around as you suggested when I figured it out. I've updated the bug report and the commit!

Copy link
Member

@andrewbranch andrewbranch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks! 🌟

@andrewbranch andrewbranch merged commit 7e0e867 into microsoft:main Dec 14, 2021
mprobst pushed a commit to mprobst/TypeScript that referenced this pull request Jan 10, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
For Backlog Bug PRs that fix a backlog bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Inconsistent QuickInfo on type-only imported symbols
3 participants