Composite: accept store props on top level component#64832
Conversation
1b9270c to
4c941ea
Compare
4c941ea to
f454e85
Compare
f454e85 to
2f1e4e3
Compare
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
|
Flaky tests detected in fb1db16. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/10644161549
|
tyxla
left a comment
There was a problem hiding this comment.
LGTM 👍
Tested in the inserter and in data views, all works well.
🚀
| rtl, | ||
| } ); | ||
|
|
||
| const store = storeProp || newStore; |
There was a problem hiding this comment.
Any good reason to prefer || to ?? here?
There was a problem hiding this comment.
Main reasons is that I copied the same style as the ariakit repo when dealing with store fallbacks:
Also, store shouldn't have falsy values ('', 0) which would cause || to behave differently than ??
What?
Extracted from #64723
Tweak
Compositeso that store props can be also passed to the top-levelCompositecomponent.Why?
This is the first step towards removing explicit usage of the
Compositestore. All props that were previously passed to theuseCompositeStorehook can be now passed to the top-level component instead.This PR is the requirement for all upcoming refactors.
How?
By changing the type defs and reconciliating existing
storeobjects.Testing Instructions
Make sure that existing usages of
Compositecontinue to work.