TRY: Make namespace optional and combine with hookName using dot notation

[adamsilverstein]

This PR seeks to remove the namespace parameter and instead enable support for an optional namespace added to the hookName using dot notation, in the form hookName.vendor/plugin/function. Fixes #107.

Includes updated docs: https://github.com/WordPress/packages/blob/fda1b56a606afccf40b86867f80710f6895559bb/packages/hooks/README.md

• after this change, periods are no longer allowed in the hook name - instead they are used to separate the hook name from the namespaced function name.
• when a namespace is omitted from a call to removeFilters/Actions, all callbacks on that hook name are removed.
• refactored all tests

[codecov]

Codecov Report

Merging #108 into master will increase coverage by 0.16%.
The diff coverage is 86.36%.

@@            Coverage Diff             @@
##           master     #108      +/-   ##
==========================================
+ Coverage   64.54%   64.71%   +0.16%     
==========================================
  Files          41       43       +2     
  Lines         598      615      +17     
  Branches      117      122       +5     
==========================================
+ Hits          386      398      +12     
- Misses        170      174       +4     
- Partials       42       43       +1

Impacted Files	Coverage Δ
packages/hooks/src/createRemoveHook.js	92.3% <100%> (-3.15%)	⬇️
packages/hooks/src/extractHookname.js	100% <100%> (ø)
packages/hooks/src/extractNamespace.js	100% <100%> (ø)
packages/hooks/src/createAddHook.js	100% <100%> (ø)	⬆️
packages/hooks/src/validateHookName.js	80% <50%> (ø)	⬆️
packages/hooks/src/createHasHook.js	71.42% <60%> (-28.58%)	⬇️
packages/hooks/src/validateNamespace.js	71.42% <0%> (-28.58%)	⬇️

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d268a2f...186d9d8. Read the comment docs.

[gziolo]

@adamsilverstein, thanks for opening this PR. It seems like the proposed approach is the best possible moving forward. I have only one concern, which is a blocker in my opinion. This won't work with Gutenberg because of the way we name our hooks. See: https://wordpress.org/gutenberg/handbook/extensibility/extending-blocks/.

Some examples:

• blocks.registerBlockType
• blocks.BlockEdit
• blocks.getSaveElement
• blocks.getSaveContent.extraProps

It won't work properly with what you proposed and I'm afraid it's too late to change those names inside Gutenberg. Can we seek for a better naming convention that would work as you proposed, but would also ensure that Gutenberg hooks will work as before?

Some alternatives are:

• blocks.getSaveContent.extraProps/core/generated-class-name/save-props
• blocks.getSaveContent.extraProps#core/generated-class-name/save-props

I think we have 2 different styles because there were 2 different validation method for both params. We can update 2nd part to match whatever we agree upon in here.

[gziolo]

It's not wrapped in code so we should either do it or remove preceding // :)

[gziolo]

Gutenberg and its hooks depend on periods :(

[herregroen]

I quite like this proposal. Feels very natural and fitting for Javascript.

It could be worth exploring if adding console warnings when namespaces are omitted is something we want to encourage good standards, perhaps as an option that can be passed to createHooks so projects that expect multiple plugins can enable while projects that have no need for it can disable it.

Regarding the above. While periods would be preferred in my opinion due to the consistency with jQuery we could of course replace them with say a # or some other character.

[adamsilverstein]

I took a slightly different approach in #113 by making the namespace parameter optional, but not moving it into the hook parameter. This will give us better back compat for existing use cases, and work correctly with Gutenberg.

For plugins, the idea would be to have registerPlugin pass the plugin namespace to created hooks.

registerPlugin( 'my-namespace', {
...
hooks: [ 
  'blocks.BlockEdit': function() { ... },
etc...
]

[ntwb]

Issue moved to WordPress/gutenberg #7820 via ZenHub