Conversation
Codecov Report
@@ Coverage Diff @@
## master #45 +/- ##
==========================================
+ Coverage 97.29% 97.31% +0.01%
==========================================
Files 16 17 +1
Lines 148 149 +1
Branches 41 41
==========================================
+ Hits 144 145 +1
Misses 4 4
Continue to review full report at Codecov.
|
aduth
left a comment
There was a problem hiding this comment.
This looks fine to me, and I personally see value in the exposed singleton instance, obviously for our specific needs but generally as well.
The question becomes: When would a developer consuming @wordpress/hooks want to create their own registry of hooks via createHooks ? Would it be bad for many independent modules who each depend on @wordpress/hooks to share the same registry? If we think so, should we put more emphasis in documentation around encouraging createHooks for libraries?
I think that the currently proposed implementation in Gutenberg uses both approaches. We use
|
adamsilverstein
left a comment
There was a problem hiding this comment.
This looks good to me, thanks.
We discussed in during the last Core JS meeting. See: https://wordpress.slack.com/archives/C5UNMSU4R/p1511276154000233.
The idea here is to have
@wordpress/hooks===wp.hooks, so we could use it directly in Gutenberg. It was raised while working on hooks integration with Gutenberg: WordPress/gutenberg#3493. The existing implementation created an ambiguity where we weren't able to easily expose hooks aswp.hooksand import using@wordpress/hooksat the same time. This PR opens this possibility by making import statement for@wordpress/hooksidentical to what is going to be exposed aswp.hooks: