Enable useSelect autocompletion #41911
Merged
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
adamziel
added
[Package] Data
adamziel
requested review from
jsnajdr,
gziolo,
sarayourfriend,
sirreal and
dmsnell
|
Size Change: +3.52 kB (0%) Total Size: 1.25 MB
ℹ️ View Unchanged
|
|
Awesome work! This is probably my most wanted feature when working with
Any idea how we could solve this in the future?
Why wouldn't we want the |
|
@kevin940726 I somehow thought they're not being released – great spot! I am going to update the description to reflect that. The reason I wanted to avoid releasing was to test-drive these types internally and make any necessary adjustments before releasing them to the world. Since the build already includes types, adding {
"types": "build-modules",
"removeTypesResolutionOnBuild": true
}And remove the
We'd have to add |
|
Actually, a webpack plugin won't cut it as the package.json is bundled straight from |
Well then, maybe we could take this another way around. Don't add the IMO, I don't think we have to go all the way just to experiment with the types. For people not using TypeScript, the change doesn't matter to them. For people using TypeScript, they probably already have a custom config to ignore the type of the package anyway. I'd prefer just releasing the types in a minor release as a feature 😅. c.c. @gziolo for other concerns I missed though. |
I'd love that to be the case, but how do you know? Also, I just realized this would be a breaking change as right now the typescript reads the types from the DefinitelyTyped repo whereas after this PR it would start reading them from the data package. The culprit is the two are not compatible :( Also interested in your thoughts @dmsnell and @sarayourfriend |
|
Unfortunately, updating the tarball won't work. Lerna creates a temporary file with a random name and does not pass its path to Which means we can do either of:
I'd love to hear more opinions here. |
|
@kevin940726 I ended up going through with the ambient module declaration to enable this just in the Gutenberg repository for now. Would you be up for a re-review? Also cc @sirbrillig |
dmsnell
reviewed
Jul 6, 2022
| @@ -0,0 +1,3 @@ | |||
| declare module '@wordpress/data' { | |||
| export * from '@wordpress/data/build-types/index'; | |||
There was a problem hiding this comment.
how does this play with the issue of having a global DefinitelyTyped package in for @wordpress/data?
There was a problem hiding this comment.
This overrides the DefinitelyTyped types thanks to the "typeRoots": [ "./typings", "./node_modules/@types" ], setting in tsconfig.json.
adamziel
deleted the
ts/enable-useselect-autocompletion-support-for-a-few-packages
branch
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 builds
@wordpress/datatypes to provide autocompletion support foruseSelectacross the repository.The two included cosmetic changes in the
block-editorpackage are here to showcase how it works:CleanShot.2022-06-23.at.13.44.33.mp4
Note: For now, this only works with the store defined in the same package where
useSelectis called. With this PR, TypeScript knows how to resolve the type of theuseSelecthook, but all the cross-package store imports still come across asany.How
This PR adds a
wordpress__data.d.tsambient module that re-exports the types built for the@wordpress/datapackage.The intention is to keep this change contained to the Gutenberg repository until the types mature more.
Once that happens, the next step would be publishing them by adding
"types": "build-types"topackage.jsonin@wordpress/data. It tells TypeScript how to resolve the typing file for@wordpress/data(source)Why?
Now that useSelect is typed in an autocompletion-friendly way, it only makes sense to enable that feature across the Gutenberg repository.
Testing Instructions
Go to
use-available-alignments.jsand confirm the autocompletion works as on the video above.CC @sarayourfriend @dmsnell @sirreal @jsnajdr @scruffian