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

[WordPress Trac]

#21170: JavaScript actions and filters
-----------------------------+-------------------------
 Reporter:  koopersmith      |       Owner:
     Type:  feature request  |      Status:  assigned
 Priority:  normal           |   Milestone:  4.9
Component:  General          |     Version:  3.4
 Severity:  normal           |  Resolution:
 Keywords:                   |     Focuses:  javascript
-----------------------------+-------------------------

Comment (by westonruter):

 Replying to [comment:148 pento]:
 > - If we haven't already, have a better way of importing modules into
 Core. Committing the webpack-ed version of the module to the SVN repo is
 unsustainable.

 This is a need for the newly-committed CodeMirror bundle as well. See
 #41870 (Code Editor: Add grunt task for building new CodeMirror bundles
 from external dependencies)

 Replying to [comment:151 adamsilverstein]:
 > Also, @westonruter have you looked at wp.hooks, is it something you
 think would be useful in making the //Customizer more extensible?//

 The Customizer has it's own events interface which has served it well. I
 suppose I'm so accustomed to to the way that JS classes in the Customizer
 have been extended with subclasses and function wrapping that I haven't
 felt the need for hooks. The main problem with the current system for
 extending Customizer JS (and in Media JS) is that it is not particularly
 elegant to be interfacing with function prototypes all over the place,
 IMHO. I'd have to do an analysis of all of the Customizer feature plugins
 to see all of the ways that it is being extended. The normal way that I've
 implemented filter-like functionality in the Customizer is to wrap a
 `wp.customize.Value`'s `validate` method to override its set value to
 always be as desired. For example, in [https://github.com/xwp/wp-
 customizer-responsive-device-preview Customizer Responsive Server-Side
 Components Device Preview] it will force the URL in the
 `wp.customize.previewer.previewUrl` to always include the current
 `previewedDevice` value in a `customize_previewed_device` query parameter
 ([https://github.com/xwp/wp-customizer-responsive-device-
 preview/blob/master/customizer-responsive-device-preview.js#L27-L35
 link]):

 {{{#!js
 var originalPreviewUrlValidate;
 originalPreviewUrlValidate = component.api.previewer.previewUrl.validate;
 component.api.previewer.previewUrl.validate = function validatePreviewUrl(
 newUrl ) {
         var url = newUrl;
         if ( null !== url ) {
                 url = component.amendUrlWithPreviewedDevice( url );
         }
         return originalPreviewUrlValidate.call( this, url );
 };
 }}}

 It works very well, but it's not particularly elegant. It would be cleaner
 if there was some 'previewUrl' filter that could be used instead of having
 to wrap the `validate` function.

 I'll note that I've tried to take a similar approach for filtering values
 being set on Backbone models, but there is no equivalent `validate` method
 in Backbone. The `validate` method on `wp.customize.Value` in the
 Customizer is really more similar to a `sanitize` method, whereas in
 Backbone it's used only as a strict a pass/fail kind of thing (like
 `validate` in the REST API). So in the media widgets, in order to
 implement the same kind of sanitization logic it is currently
 [https://github.com/WordPress/wordpress-
 develop/blob/95a263212177a4944f96c927eae7f80bdd020105/src/wp-
 admin/js/widgets/media-widgets.js#L971-L1015 overridding] the entire
 `Model#set` method itself.

 One point of concern I have with `wp.hooks` as to me it would seem to
 encourage a break in the encapsulation of logic inside of individual
 components. In other words, JS components that would be running the hooks
 would all have to reach out to this one object in global scope where all
 of the hooks are registered. Namespaces would have to be added to the
 hooks to prevent them from conflicting due to the fact that they are all
 global.

 This just came to mind now (and maybe this has already been considered in
 the design of `wp.hooks`, so excuse my ignorance): ¿would it not be a
 better architectural pattern to instead introduce hooks as something that
 can be instantiated onto a specific object? For example, Backbone has its
 `Events` mixin and Customizer similarly has its `wp.customize.Events`
 interface. You can augment any object with this mixin to get the interface
 to trigger and listen for events, and when the events are triggered you
 don't need a namespace because they're naturally scoped to that object.
 There is also less overhead then by not having to manage a massive object
 of hook/handler mappings with the risk then of leaking memory (I recall
 PHP actions and filters being identified as a performance bottleneck in
 WordPress).

 In contrast, what if you have a `wp.Hooks` interface that you add to some
 `Thing` (like `Object.assign( Thing.prototype, wp.Hooks )` and then you do
 `obj = new Thing()` and attach a filter to it like `obj.addFilter( 'foo',
 function() {…} )`  then when `obj` is destroyed then that anonymous
 function handler could also be garbage collected. In contrast, if you do
 `wp.hooks.addFilter( 'someNamespace.thing.foo', function() {} )` the you'd
 potentially need to remember to `removeFilter` later or else that
 anonymous function will stick around forever.

 Replying to [comment:173 adamsilverstein]:
 > The goal of getting `wp.hooks` into trunk is to start trying to use is
 internally - I'm going to work on a demonstration of that next.
 >
 > If we find the `wp.hooks` library is not useful or feel its API is not
 stable before the 4.9 release, I would support reverting or possubly
 authoring a dev note as well as liberal inline comments explaining that
 this is a new API that may have breaking changes and if developers use it
 they should watch development closely.

 We're planning to be working on a Customizer v2 feature plugin after the
 4.9 release, and at that point we'll definitely taking a look at the
 current JS API and anywhere that its extensibility is lacking. I'd want to
 incorporate `wp.hooks` wherever possible in that project.

 In conclusion, I would be open to keeping `wp.hooks` in `trunk` for the
 remainder of `4.9-alpha` with an expectation that it will be reverted
 before beta, if that helps with testing patches that prototype making use
 of `wp.hooks`. (While an exception was made for the REST API, I think we
 shouldn't ship something that isn't used in core.) In that way, it would
 be treated like “temp hooks” for feature plugins (which aren't used very
 often). But if having `wp.hooks` in core is only to facilitate writing
 patches for implementing hooks for parts of core, then I'd suggest instead
 creating a new “wp-js-hooks” '''feature plugin''' that simply bundles the
 JS and registers it so that other feature plugins can enqueue it and
 utilize it. (Plugin dependencies, anyone?)

--
Ticket URL: <https://core.trac.wordpress.org/ticket/21170#comment:175>
WordPress Trac <https://core.trac.wordpress.org/>
WordPress publishing platform