-
Notifications
You must be signed in to change notification settings - Fork 13.1k
Consider import.meta a qualified name/property access
#54997
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
Conversation
|
@typescript-bot test this |
|
Heya @gabritto, I've started to run the perf test suite on this PR at 1e2c9ae. You can monitor the build here. Update: The results are in! |
|
Heya @gabritto, I've started to run the diff-based top-repos suite (tsserver) on this PR at 1e2c9ae. You can monitor the build here. Update: The results are in! |
|
Heya @gabritto, I've started to run the diff-based user code test suite (tsserver) on this PR at 1e2c9ae. You can monitor the build here. Update: The results are in! |
|
Heya @gabritto, I've started to run the parallelized Definitely Typed test suite on this PR at 1e2c9ae. You can monitor the build here. Update: The results are in! |
|
Heya @gabritto, I've started to run the diff-based top-repos suite on this PR at 1e2c9ae. You can monitor the build here. Update: The results are in! |
| >foo : () => void | ||
| >new.targe : () => void | ||
| >targe : any | ||
| >targe : () => void |
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.
Weird, how is this changing? This doesn't seem to be using meta at all.
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.
new.something is also a MetaProperty node, I'm guessing this is to also support new.target.
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.
Do I even want to know why targe works without the t? 😅
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.
Probably fall-out logic where any meta-property that's not import.meta is assumed to be new.target.
|
@gabritto Here are the results of running the user test suite comparing Everything looks good! |
|
@gabritto Here they are:
CompilerComparison Report - main..54997
System
Hosts
Scenarios
TSServerComparison Report - main..54997
System
Hosts
Scenarios
StartupComparison Report - main..54997
System
Hosts
Scenarios
Developer Information: |
|||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
@gabritto Here are the results of running the top-repos suite comparing Everything looks good! |
|
Hey @gabritto, it looks like the DT test run failed. Please check the log for more details. |
|
@gabritto Here are the results of running the top-repos suite comparing Everything looks good! |
| >foo : () => void | ||
| >new.targe : () => void | ||
| >targe : any | ||
| >targe : () => void |
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.
Do I even want to know why targe works without the t? 😅
|
@typescript-bot run DT |
|
Heya @gabritto, I've started to run the parallelized Definitely Typed test suite on this PR at 1e2c9ae. You can monitor the build here. Update: The results are in! |
|
Hey @gabritto, the results of running the DT tests are ready. |
Fixes #54590.
isRightSideOfQualifiedNameOrPropertyAccessis called bygetTypeOfSymbolAtLocationto decide if we should consider the location's parent instead, so that if we have a property access/qualified name, e.g.x.y, we look up the whole expression, and not justy. Becauseimport.metawas not considered one of those cases, whengetTypeOfSymbolAtLocationwas called by completions, we'd end up trying to resolve the namemeta, which doesn't resolve to anything, leading to the error.