-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
Components: Ship TypeScript types #49229
Conversation
Size Change: 0 B Total Size: 1.34 MB ℹ️ View Unchanged
|
Wow, it’s so close 🎉🎉🎉 That’s quite an achievement, congrats to everyone involved! |
I gave this PR a quick look, and types seems to work as expected, at least in VSCode. It would be good to get more eyes on this, especially:
Pinging a few folks in that regard: @gziolo @aaronrobertshaw @andrewserong @noisysocks @sirreal |
@@ -43,7 +43,6 @@ | |||
"src/**/*.native.js", | |||
"src/**/react-native-*", | |||
"src/**/stories/**/*.js", // only exclude js files, tsx files should be checked | |||
"src/**/test/**/*.js", // only exclude js files, ts{x} files should be checked | |||
"src/index.js" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On top of excluding index.js
, I wonder if there's any value in renaming it to index.ts
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I tried renaming it, let's see if anything breaks
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice work on this @mirka ✨
I've only been able to give this a run using VS Code but it's looking good.
✅ Build process completes without issue
✅ Intellisense tooltips show correct info
✅ Go to source definition lands on the correct file and export
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gave this some testing on Sublime Text and it worked well 👍
- Builds without errors ✅
- Tooltips display properly ✅
- Going to definition or type definition works as expected ✅
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is really cool... working well for me in VSCode as well.
6224b5b
to
98e379a
Compare
Flaky tests detected in 98e379a. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/4504205931
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
🚀
DT removal: DefinitelyTyped/DefinitelyTyped#65213 |
What?
Related to #35744
Properly ship first-party TypeScript declarations with the
@wordpress/components
package.Why?
We were relying on
@types
, which could be wrong or outdated.Follow-up: remove types from Definitely Typed
Testing Instructions
npm run build