[wp-trac] [WordPress Trac] #21170: JavaScript actions and filters
WordPress Trac
noreply at wordpress.org
Thu Mar 9 21:01:01 UTC 2017
#21170: JavaScript actions and filters
------------------------------------+------------------------------
Reporter: koopersmith | Owner: adamsilverstein
Type: feature request | Status: assigned
Priority: normal | Milestone: Future Release
Component: General | Version: 3.4
Severity: normal | Resolution:
Keywords: has-patch dev-feedback | Focuses: javascript
------------------------------------+------------------------------
Comment (by aduth):
Joining this discussion late in the conversation, so forgive me in advance
if I've failed to adequately come up to speed here in my remarks.
@adamsilverstein, the approach in your latest patches is pretty excellent.
I went through the latest in detail. Below are a few of my specific
observations:
- I don't see the value in supporting chaining. I notice there's been some
prior discussion here on its usefulness. Could someone provide a practical
example where it would be useful?
- Similarly, why do we expose `context` as an argument? `this` is a
miserable keyword in JavaScript. At worst, if the developer wanted to
leverage it, they could use `Function#bind` or `_.bind` on the callback
argument.
- Minor: A handful of violations of negation spacing guideline "Any !
negation operator should have a following space.*" (particularly
`_removeHook`)
- Could we generalize implementations between actions and filters to a
common creator functions? e.g. roughly `createHookAdder = ( type ) => (
name, callback, priority ) => _addHook( type, name, callback, priority )`
- Your `_addHook` "prop itself" logic, while more performant, opens you up
to edge cases around object prototype lookup, e.g. `wp.hooks.addFilter(
'valueOf', function() {} )` will throw an error. `hasOwnProperty`
addresses this, or you can consider creating the hook storage with an
empty prototype via `Object.create( null )` (browser support varies)
- Seems odd to me that `_runHook` is generalized while its implementation
then considers very specifically the `type` in how it's executed. Instead
seems `applyFilters` and `doAction` should just contain their respective
logic.
To the first two points, I think perhaps we've tried to provide more
options than are really useful, which adds complexity to the
implementation and breaks alignment with my understanding of the
equivalent PHP functions (where I'd expect these functions to be used
nearly identically).
--
Ticket URL: <https://core.trac.wordpress.org/ticket/21170#comment:112>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform
More information about the wp-trac
mailing list