Skip to content

fix(53138): go-to-definition not working on expression of SatisfiesExpression #53164

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 6 commits into from
Apr 13, 2023

Conversation

Zzzen
Copy link
Contributor

@Zzzen Zzzen commented Mar 8, 2023

Fixes #53138

@typescript-bot typescript-bot added the For Backlog Bug PRs that fix a backlog bug label Mar 8, 2023
//////somewhere in app
////STRINGS.[|/*usage*/title|]

// verify.goToDefinition("usage", "definition");
Copy link
Member

Choose a reason for hiding this comment

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

Is this leftover?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, removed.

@@ -0,0 +1,14 @@
/// <reference path="./fourslash.ts"/>

// @noImplicitOverride: true
Copy link
Member

Choose a reason for hiding this comment

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

What is this doing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

😅 It's copied from other files accidentally.

@@ -274,8 +274,9 @@ function getDefinitionFromObjectLiteralElement(typeChecker: TypeChecker, node: N
if (element) {
const contextualType = element && typeChecker.getContextualType(element.parent);
if (contextualType) {
return flatMap(getPropertySymbolsFromContextualType(element, typeChecker, contextualType, /*unionSymbolOk*/ false), propertySymbol =>
const definitions = flatMap(getPropertySymbolsFromContextualType(element, typeChecker, contextualType, /*unionSymbolOk*/ false), propertySymbol =>
Copy link
Member

Choose a reason for hiding this comment

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

This seems okay, though, I half wonder if it's better to change this to always return an empty array, then fix the one caller who cares about undefined to check length instead. Maybe Andrew has further comments here.

Copy link
Member

Choose a reason for hiding this comment

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

I read the usage--you'll have to write the code once somewhere, but moving it to the caller allows the other caller to be simplified, since it is getDefinitionFromObjectLiteralElement(checker, node) || [] right now. I vote to make this change.

@@ -274,8 +274,9 @@ function getDefinitionFromObjectLiteralElement(typeChecker: TypeChecker, node: N
if (element) {
const contextualType = element && typeChecker.getContextualType(element.parent);
if (contextualType) {
return flatMap(getPropertySymbolsFromContextualType(element, typeChecker, contextualType, /*unionSymbolOk*/ false), propertySymbol =>
const definitions = flatMap(getPropertySymbolsFromContextualType(element, typeChecker, contextualType, /*unionSymbolOk*/ false), propertySymbol =>
Copy link
Member

Choose a reason for hiding this comment

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

I read the usage--you'll have to write the code once somewhere, but moving it to the caller allows the other caller to be simplified, since it is getDefinitionFromObjectLiteralElement(checker, node) || [] right now. I vote to make this change.

@sandersn sandersn assigned sandersn and unassigned andrewbranch Mar 17, 2023
@sandersn
Copy link
Member

I had to switch this to the baseline format -- I think maybe the non-baseline verifier got removed recently?

@sandersn sandersn merged commit 458c5e6 into microsoft:main Apr 13, 2023
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
Archived in project
Development

Successfully merging this pull request may close these issues.

CMD click on key doesn't navigate to references on typescript satisfies Record<>.
5 participants