-
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
Redux routine: Add types #21313
Redux routine: Add types #21313
Conversation
35b70ba
to
62ca114
Compare
62ca114
to
d531fcc
Compare
Ah shoot, I blew away your earlier attempt @sirreal Do you happen to have a local copy of this still that you could repush? I didn't realize I was pushing to an already existing branch 🤦 |
Size Change: +665 B (0%) Total Size: 1.04 MB
ℹ️ View Unchanged
|
Never mind, I'll be merging our approaches, figured out how to fix it. Sorry for the ping! |
d531fcc
to
ef317bb
Compare
I've update the |
@@ -0,0 +1,17 @@ | |||
declare module 'rungen' { |
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.
@youknowriad Would you be interested in publishing types for rungen
? We could include the types in the package without requiring any code changes.
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.
Whatever works best for you. I'm also happy to add collaborators on that package, I didn't touch it for some time now 😬
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.
Thanks. Providing the types from the library itself seems ideal to me, although we don't need to wait for it here.
280678c
to
5037f58
Compare
5037f58
to
2b21f85
Compare
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 have one concern about introducing a new dependency on redux
@@ -11,8 +11,7 @@ | |||
{ "path": "../element" }, | |||
{ "path": "../is-shallow-equal" }, | |||
{ "path": "../priority-queue" }, | |||
// Needs to be added once it is typed | |||
// { "path": "../redux-routine" } | |||
{ "path": "../redux-routine" } |
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.
✨ Love it
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 can't approve because I opened this originally, but it looks good. Thanks for getting it over the line @sarayourfriend!
👍 ✅ 🚢
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.
LGTM (and to @sirreal ) 🚀
Description
Work in progress
Help is welcome 🙂
Add types to
@wordpress/redux-routine
.Part of #18838
How has this been tested?
Screenshots
Types of changes
Checklist: