fix: resolve property assignments in object literals #373
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
added 2 commits
September 12, 2024 10:52
Arrays of objects create definitions for all object keys contained
This follows the implementation I found in the TypeScript goToDefinition service https://github.com/microsoft/TypeScript/blob/88809467e8761e71483e2f4948ef411d8e447188/src/services/goToDefinition.ts#L307-L316
olafurpg
reviewed
Sep 23, 2024
There was a problem hiding this comment.
I reviewed the LSP implementation quite thoroughly and did not find getContextualType. It makes a ton of sense that this API exists, and I'm 100% in favor of this much simpler approach. I would try porting some of the tests from the other PR to see how it behaves.
This change is a significantly quality improvement for TypeScript navigation so I think it's absolutely worth it to get this through the finish line.
added 6 commits
September 30, 2024 10:00
without these `npm run eslint` wouldn't pass locally
kritzcreek
force-pushed
the
christoph/array-elem-defs
branch
from
c909a93 to
9c755a7
Compare
kritzcreek
force-pushed
the
christoph/array-elem-defs
branch
from
9c755a7 to
a7ad7bd
Compare
kritzcreek
force-pushed
the
christoph/array-elem-defs
branch
from
8dc6cca to
bd71c9d
Compare
kritzcreek
force-pushed
the
christoph/array-elem-defs
branch
from
bd71c9d to
d235021
Compare
olafurpg
approved these changes
Sep 30, 2024
| @@ -49,7 +49,7 @@ export const LoaderInput2: React.FunctionComponent<Props> = props => { | |||
| return <LoaderInput loading={true} key="key" children={props.children} /> | |||
| // ^^^^^^^^^^^ reference react-example 1.0.0 src/`LoaderInput.tsx`/LoaderInput. | |||
| // ^^^^^^^ reference react-example 1.0.0 src/`LoaderInput.tsx`/Props#loading. | |||
| // ^^^ reference local 10 | |||
| // ^^^ reference @types/react 18.2.39 `index.d.ts`/React/Attributes#key. | |||
|
|
||
| @MyDecorator({ property: 42, property2: '42' }) | ||
| //^^^^^^^^^^^ reference syntax 1.0.0 src/`decorators.ts`/MyDecorator(). | ||
| // ^^^^^^^^ reference syntax 1.0.0 src/`reusable-types.ts`/Numbers#property. |
| // ^^^^^^ reference syntax 1.0.0 src/`object-literals-arrow-function.ts`/Foobar# | ||
| return fn({ foobar: 42 + something }) | ||
| // ^^ reference syntax 1.0.0 src/`object-literals-arrow-function.ts`/hasArrowFunctionParameter().(fn) | ||
| // ^^^^^^ reference syntax 1.0.0 src/`object-literals-arrow-function.ts`/Foobar#foobar. |
There was a problem hiding this comment.
Woo hoo, this is such a significant leap for navigation quality. Closest huge gap against tsserver
| if (!objectElement) { | ||
| return | ||
| } | ||
| const contextualType = this.checker.getContextualType(objectElement.parent) |
| // even if `this.checker.getTypeAtLocation({y: 42})` does not return | ||
| // `SomeInterface`. The object literal could satisfy many types, but in this | ||
| // particular location must only satisfy `SomeInterface`. | ||
| private inferredTypeOfObjectLiteral( |
There was a problem hiding this comment.
Yeah this one felt pretty good :D
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Simpler alternative to #256
This follows the implementation I found in the TypeScript goToDefinition service https://github.com/microsoft/TypeScript/blob/88809467e8761e71483e2f4948ef411d8e447188/src/services/goToDefinition.ts#L307-L316
Leans on the TypeScript compilers
getContextualTypefunction to resolve object literals to the interfaces, or type definitions they are being checked against. This means we generate useful references in a lot more places, where we'd generate "dead" definitions before.Test plan
Ports all tests from #256 and adds some basic new ones. Check the updated snapshot output to gauge the improvement or if there are any regressions.