-
Notifications
You must be signed in to change notification settings - Fork 4.2k
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
Core data: Fix some typing issues #39212
Conversation
Size Change: +566 B (0%) Total Size: 1.15 MB
ℹ️ View Unchanged
|
packages/core-data/src/actions.js
Outdated
@@ -607,8 +603,8 @@ export const saveEntityRecord = ( | |||
* `saveEntityRecord`, `saveEditedEntityRecord`, and | |||
* `deleteEntityRecord`. | |||
* | |||
* @return {Promise} A promise that resolves to an array containing the return | |||
* values of each function given in `requests`. | |||
* @return {(thinkArgs: Object) => Promise} A promise that resolves to an array containing the return |
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.
Why thinkArgs
?
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.
Looking at the original PR, I think this was meant to say thunk
– I went ahead and updated it
* @return {(thinkArgs: Object) => Promise} A promise that resolves to an array containing the return | |
* @return {(thunkArgs: Object) => Promise} A promise that resolves to an array containing the return |
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 for the update; you were spot on - thunkArgs - We could have made a better type here but mostly I want to get things set so we don’t have errors once we turn on TypeScript
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 nitpick, but looks good otherwise.
In this commit we're cleaning up type issues in the core-data package that prevent us from telling TypeScript to run on the package and all of its existing code, even the JS files. After these changes we should be able to do so and start converting more modules to TypeScript with less friction. This patch follows a series of other smaller updates: - #39212 - #39214 - #39225 - #39476 - #39477 - #39479 - #39480 - #39525 - #39526 - #39655 - #39656 - #39659 It was built in order to support ongoing work to add types to the `getEntityRecord` family of functions in #39025.
In this commit we're cleaning up type issues in the core-data package that prevent us from telling TypeScript to run on the package and all of its existing code, even the JS files. After these changes we should be able to do so and start converting more modules to TypeScript with less friction. This patch follows a series of other smaller updates: - #39212 - #39214 - #39225 - #39476 - #39477 - #39479 - #39480 - #39525 - #39526 - #39655 - #39656 - #39659 It was built in order to support ongoing work to add types to the `getEntityRecord` family of functions in #39025.
Part of #39211
Description
Fixes a few small issues with the JSDoc types in
@wordpress/core-data
.undefined
was the wrong return type for the given files. removing that type to remove the error and leaving them untyped.undoIgnore
parameter is optional foreditEntityRecord
__experimentalBatch
is a function that returns aPromise
, not aPromise
Testing Instructions
Audit the types. As a type-only change this won't impact the built files
Types of changes
This is a type-only change and won't impact the build.
Checklist:
*.native.js
files for terms that need renaming or removal).