[wp-trac] [WordPress Trac] #21170: JavaScript actions and filters

[WordPress Trac]

#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