GV

GeezerVison

The World Wide Web, restored to 800 pixels of glory.

Home|About|Gaming cabinet|Archives|How it works|Starter sites

On this site

  • Sign in
  • ActionsCI/CD and automation
  • CodespacesCloud development environments
  • All Features
  • Documentation
  • Training
  • Certifications
  • Build with Copilot
  • Customize Copilot
  • Integrations
  • Trending repos
  • GitHub Sponsors
  • Developer articles
  • Enterprises

This view

  • Original page
  • New address

GeezerVison is showing github.com — Fix getTypeAtLocation for `as const` to not issue a diagnostic by weswigham · Pull Request #36741 · microsoft/TypeScript

Conversation

@weswigham

@weswigham Wesley Wigham (weswigham) commented Feb 11, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member

Fixes #34913

checkAssertion in the normal codepath avoids calling into getTypeFromTypeNode on the const in as const expressions, however the checker getTypeAtLocation API may invoke it directly. Previously, this'd cause as resolution error, as we'd resolve an (entirely unused) variable named const and return its' type. With this change, we instead return the type of the expression associated with the const (which should be more useful to API consumers, I hope, and prevents any spurious errors from being added).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions
Wesley Wigham (weswigham) added 3 commits February 11, 2020 14:11
@weswigham
Fix getTypeAtLocation for as const to not issue a diagnostic
015a472
@weswigham
use existing helpers for checks
6feb349
@weswigham
Fix lint
8bcd4f4
@weswigham
Wesley Wigham (weswigham) requested review from Ryan Cavanaugh (RyanCavanaugh), Andrew Casey (amcasey), Nathan Shively-Sanders (sandersn) and Sheetal Nandi (sheetalkamat) February 11, 2020 22:32
sheetalkamat
Sheetal Nandi (sheetalkamat) reviewed Feb 11, 2020
View reviewed changes
Comment thread src/compiler/checker.ts
function getTypeFromTypeReference(node: TypeReferenceType): Type {
const links = getNodeLinks(node);
if (!links.resolvedType) {
// handle LS queries on the `const` in `x as const` by resolving to the type of `x`

@sheetalkamat Sheetal Nandi (sheetalkamat) Feb 11, 2020

Copy link
Copy Markdown
Member

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Instead of this shouldn't getTypeOfNode just check if it is const of as const expression should forward it to correct node instead?

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

All reactions

@weswigham Wesley Wigham (weswigham) Feb 11, 2020 •
edited
Loading

Uh oh!

There was an error while loading. Please reload this page.

Copy link
Copy Markdown
Member Author

There was a problem hiding this comment.

Choose a reason for hiding this comment

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

Ehhhhhh, it could, but I'd rather push the handling to here, to prevent us from ever attempting to resolve the const in as const as a real type reference, however we may get there in the future (plus, by doing it here the result is cached).

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

sandersn reacted with thumbs up emoji
All reactions
  • 👍 1 reaction
@weswigham
Merge branch 'master' into fix-ls-gettypeatlocation-for-as-const
a85b27c
sandersn
Nathan Shively-Sanders (sandersn) approved these changes Feb 12, 2020
View reviewed changes
@weswigham
Wesley Wigham (weswigham) merged commit 2b64731 into microsoft:master Feb 12, 2020
@weswigham
Wesley Wigham (weswigham) deleted the fix-ls-gettypeatlocation-for-as-const branch February 12, 2020 21:43
@amcasey

Andrew Casey (amcasey) commented Feb 12, 2020

Copy link
Copy Markdown
Member

Is this related to #36490? I believe that PR was also about handling LS requests in larger spans (i.e. not just on identifiers).

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@weswigham

Wesley Wigham (weswigham) commented Feb 12, 2020

Copy link
Copy Markdown
Member Author

Not as far as I know - this is just about fixing us giving a bogus result (and adding a diagnostic) for the type of const in x as const when the LS requests the type at const.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@amcasey

Andrew Casey (amcasey) commented Feb 12, 2020

Copy link
Copy Markdown
Member

So it changes from wrong to right, rather than from nothing to something? Makes sense.

All reactions

Sorry, something went wrong.

Uh oh!

There was an error while loading. Please reload this page.

@weswigham Wesley Wigham (weswigham) mentioned this pull request Mar 28, 2022
Prevent looking up symbol for as const from triggering an error #48464
Merged
@microsoft Microsoft (microsoft) locked as resolved and limited conversation to collaborators Oct 21, 2025
@typescript-bot TypeScript Bot (typescript-bot) assigned Wesley Wigham (weswigham) Oct 21, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Reviewers

@sheetalkamat Sheetal Nandi (sheetalkamat) sheetalkamat left review comments
@sandersn Nathan Shively-Sanders (sandersn) sandersn approved these changes
@amcasey Andrew Casey (amcasey) Awaiting requested review from amcasey
@RyanCavanaugh Ryan Cavanaugh (RyanCavanaugh) Awaiting requested review from RyanCavanaugh

Uh oh!

There was an error while loading. Please reload this page.

Assignees

@weswigham Wesley Wigham (weswigham)

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Using getTypeAtLocation on a const type reference creates a TS2304 diagnostic

Uh oh!

There was an error while loading. Please reload this page.

4 participants

@weswigham @amcasey @sandersn @sheetalkamat
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
You are visitor 01402503 — GeezerVison does not store the pages you look up.
Best viewed with Netscape Navigator 4.7 or Internet Explorer 6 at 800×600. No cookies required.