Autocompletion support for useSelect (via jsDoc annotations) #41596
Conversation
This file contains 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
|
Size Change: +8.89 kB (+1%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
adamziel
changed the title
adamziel
added
[Package] Data
adamziel
changed the title
|
Edit: This was a super long comment exploring different ways to infer argument types in jsDoc. Turns out I was sent down the wrong path, so I moved it all to a gist. My IDE picked up the DefinitelyTyped types from the following directory and refused to cooperate:
|
|
I ironed out all the kinks in this PR:
It's ready for a review! |
adamziel
changed the title
This was referenced Jun 10, 2022
dmsnell
reviewed
Jun 14, 2022
| ...args: infer P | ||
| ) => infer R | ||
| ? ( ...args: P ) => R | ||
| : F; |
There was a problem hiding this comment.
what was the issue with this inference? is it that we lose the generic type parameters through this inference? so if we had const get<T> = (state, id): T => state[id]; that it widens to any?
There was a problem hiding this comment.
More or less:
type BadlyInferredSignature = CurriedState<
<K extends string | number>(
state: any,
kind: K,
key: K extends string ? 'one value' : false
) => K
>
// BadlyInferredSignature evaluates to:
// (kind: string number, key: false | "one value") => string number
The only solution I found is quite convoluted so I went for shipping the simple Curry type in this PR. We can always ship an updated version in a follow-up PR.
dmsnell
reviewed
Jun 14, 2022
|
|
||
| _Returns_ | ||
|
|
||
| - `Function`: A custom react hook. | ||
| - `UseSelectReturn<T>`: A custom react hook. |
There was a problem hiding this comment.
unfortunately until we get hyperlinked or better-resolved types I feel like this is just as useless as Function before it. not a knock against this PR - nothing I think we can do about it now. maybe it's a little better since you can search for UseSelectReturn in the code…
dmsnell
reviewed
Jun 14, 2022
dmsnell
reviewed
Jun 14, 2022
|
@dmsnell I just addressed your feedback! @sarayourfriend would you also like to take a look at this one? |
|
Hi Adam, sorry for the delay. I've been traveling and have been overwhelmed with other work. I am taking a look at this today. |
mburridge
added
the
Needs Dev Note
|
Added the Needs Dev Note label in case this needs a dev note (either individual or as part of a "misc" dev note) for WP 6.1 release. |
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.
What?
This PR adds TypeScript type definitions to
useSelectvia jsDoc annotations. Effectively, it give us autocompletion support:CleanShot.2022-06-09.at.16.36.16.mp4
It's a step towards #39081, only it focuses just on
useSelectImporting caveat
Using the annotations in the same file where they're defined works flawlessly. However, after importing useSelect in another file, autocompletion doesn't work correctly. I believe it's because DefinitelyTyped definition get in the way. Once I removed (from
~/Library/Caches/typescript/4.8/node_modules/@types/wordpress__data/index.d.ts), the return value ofcreateReduxStorebecame simplyany:To confirm that hypothesis, I set up a separate repository with clean slate and no Lerna packages. Suddenly, all the annotations worked flawlessly when imported. I would like to make it work across Gutenberg packages but let's explore that in follow-up PR.
Test plan
Open the
test_jsdoc.jsfile shipped with this PR and confirm autocompletion behaves like on the screencast above. Of course, this file needs to be removed before this PR is merged.IDE tests:
✅ I tried it in VisualStudio code, where it worked perfectly
✅
I then tried it in PHPStorm, and it didn't work at allAfter updating PHPStorm and its configuration, everything worked perfectly just like in VSCode.Other considerations
I went through the following resources which I'm documenting here for posterity:
CC @dmsnell @sarayourfriend @sirreal @jsnajdr