-
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
Interactivity API: Add JSDoc comments to the public API #52469
Conversation
Size Change: +171 B (0%) Total Size: 1.44 MB
ℹ️ View Unchanged
|
Flaky tests detected in 1bb98f3. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5507131083
|
@DAreRodz, is this ready for review? |
Yes, it is. 🙏 |
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, David. I like the examples 🙂
I left a few comments below. Let me know what you think.
@DAreRodz, is anything blocking the merge of this? |
Hey, @luisherranz; sorry for the delay. I prioritized other things, but I'm now addressing your feedback. This PR will be ready to merge in a moment. |
Thanks. Let me know once it's ready for another review 🙂 |
Co-authored-by: Luis Herranz <luisherranz@gmail.com>
Co-authored-by: Luis Herranz <luisherranz@gmail.com>
Co-authored-by: Luis Herranz <luisherranz@gmail.com>
9a6bb07
to
4ec8dbb
Compare
@luisherranz, it's ready for review now. EDIT: I had to fix a jslint error that was accidentally included. |
packages/interactivity/src/store.js
Outdated
* @param {Object} properties Properties to be added to the global store. | ||
* @param {Object} [properties.state] State to be added to the global store. All | ||
* the properties included here become reactive. | ||
* @param {Object} [properties.block] Other properties to be added to the global store. |
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 believe this is not the correct way of documenting a variable that uses the spread operator. It seems to work somehow, but only for the docs, not for types. We could maybe remove 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.
Up to you 🙂
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'll remove it for now, then.
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, David!
packages/interactivity/src/store.js
Outdated
* @param {Object} properties Properties to be added to the global store. | ||
* @param {Object} [properties.state] State to be added to the global store. All | ||
* the properties included here become reactive. | ||
* @param {Object} [properties.block] Other properties to be added to the global store. |
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.
Up to you 🙂
Maybe we can add an |
What?
Adds JSDoc comments to the
store
anddirective
functions, which are currently the public part of the Interactivity API.Why?
To start documenting the Interactivity API.
How?
I followed the format used for other JSDoc comments in the repository. I added basic support for typing, but it is very preliminary and not meant to be fully compatible with TypeScript yet.