Make go to definition go to the constraint's properties for object literals in argument positions #62361
Conversation
…terals in argument positions
| None = 0, | ||
| Signature = 1 << 0, // Obtaining contextual signature | ||
| NoConstraints = 1 << 1, // Don't obtain type variable constraints | ||
| IgnoreNodeInferences = 1 << 2, // Ignore inference to current node and parent nodes out to the containing call for, for example, completions |
There was a problem hiding this comment.
As shown by this PR, this mechanism is handy for other things than just completions - so I decided to rename it.
This comment was marked as outdated.
This comment was marked as outdated.
Sorry, something went wrong.
src/services/goToDefinition.ts
Outdated
| if (contextualType) { | ||
| return flatMap(getPropertySymbolsFromContextualType(element, typeChecker, contextualType, /*unionSymbolOk*/ false), propertySymbol => getDefinitionFromSymbol(typeChecker, propertySymbol, node)); | ||
| let properties = getPropertySymbolsFromContextualType(element, typeChecker, contextualType, /*unionSymbolOk*/ false); | ||
| if (properties.length === 1) { |
There was a problem hiding this comment.
Is there no way that this would ever apply if there were more than one symbol here? Like, can the definition be both the constraint but also something else via a union?
There was a problem hiding this comment.
It could come from multiple sources. This is just me using a conservative approach that covers the majority of use cases. I don't see a reason why it wouldn't be OK to change the algorithm to:
- check if
propertiescontain a "self declaration" - if yes then get contextual type with
ContextFlags.IgnoreNodeInferences - if we get some results then get
withoutNodeInferencesPropertiesand if there are some then filter out self-declaration and concat both arrays
This should leave all potential meaningful definitions in the result. WDYT?
A simple playground for experimenting with how TypeScript behaves today in presence of unions: TS playground
There was a problem hiding this comment.
I feel like it must be better to not hardcode "exactly one property", but then again I don't know what the change I suggested looks like in code.
There was a problem hiding this comment.
@jakebailey I went for quite a simple change here that should address your concern. I can't think of a test case right now that would prove this has to be complicated further.
Andarist
force-pushed
the
gotodef-obj-literal-constraint-def
branch
from
e9cce24 to
594a01b
Compare
typescript-bot
added
the
For Uncommitted Bug
|
This PR doesn't have any linked issues. Please open an issue that references this PR. From there we can discuss and prioritise. |
4 participants
No description provided.