WordPress.org

Make WordPress Core

Opened 22 months ago

Last modified 6 weeks ago

#21170 new feature request

JavaScript actions and filters

Reported by: koopersmith Owned by: koopersmith
Milestone: Future Release Priority: normal
Severity: normal Version: 3.4
Component: General Keywords: 2nd-opinion has-patch dev-feedback
Focuses: javascript Cc:

Description

The concept of adding JavaScript actions and filters has been tossed around for some time. We've experimented with various configurations of actions in both the fullscreen and customizer APIs, and they've proven their utility enough to graduate them to a core feature in their own right.


I think that a good events API should satisfy these parameters:

  1. Support jQuery-style dot namespacing to allow functions to be easily removed.
  1. Should (likely) support priorities. While seemingly random numbers aren't fun to use, it allows plugins to cooperate without having to know of each other's existence. We can't expect plugin authors to rearrange the array of callbacks.
  1. Should not force functions to have unique IDs. Anonymous functions are extremely common in JavaScript — forcing them to be named is contrary to the nature of the language.
  1. Should be structured as a mixin. The global event loop should be an instance of the core Events object. Using a mixin will allow developers to easily create event loops for their own plugins (to prevent polluting the global namespace — think about large plugins, like bbPress). An events mixin will also enable developers to create more powerful abstractions, such as observable values, collections, and pretty much any structural JS object you can dream up.
  1. Should allow the looping process to be overwritten. This will result in less code and added flexibility. The only difference between actions and filters is how they handle the callbacks object. There are other types of looping processes that could be beneficial in JS. One example would be returning false if any callback returns false, which could be used to stop a process, much like the native event.stopPropagation method.

Why not use custom jQuery events?

Custom jQuery events are great when we need to trigger actions on a DOM element. Triggering plain events on the body element (or any other hidden element) is not performant — every jQuery event normalizes an DOM Event object, which we then completely ignore.

Should we require jQuery to use the API?

I'm not sure. jQuery.Callbacks may be a helpful solution here, provided we can properly integrate priorities and namespacing. jQuery.Callbacks also only requires jQuery.each and jQuery.extend, so writing a shim that doesn't use the rest of jQuery would not be exceptionally difficult. Either way, switching between one and the other as we develop should not be exceptionally difficult.

Attachments (4)

21170.patch (6.2 KB) - added by azaozz 22 months ago.
wp-hooks.js (2.5 KB) - added by mikeschinkel 22 months ago.
wp-hook.js which is a copy of the same functionality from Sunrise.
21170-1.patch (20.5 KB) - added by TV productions 6 weeks ago.
Implemented the wp hooks from https://github.com/gcorne/WP-JS-Hooks/
21170-2.patch (22.3 KB) - added by TV productions 6 weeks ago.

Download all attachments as: .zip

Change History (97)

comment:1 lgedeon22 months ago

  • Cc luke.gedeon@… added

comment:2 WraithKenny22 months ago

  • Cc Ken@… added

comment:3 scribu22 months ago

I think 1) is just a consequence of 3) and 3) follows naturally, since in JavaScript you have to pass functions around directly, instead of strings.

I'm not sure about 5). Why would we need this in JS, but not in PHP?

comment:4 sabreuse22 months ago

  • Cc sabreuse@… added

azaozz22 months ago

comment:5 vhauri22 months ago

  • Cc vhauri added

comment:6 azaozz22 months ago

21170.patch is a simple "pub/sub" based on Amplify.js. It implements the above points and can be used as example/base for this.

comment:7 follow-up: vhauri22 months ago

+1 for staying away from the DOM to do this.

I feel like the Callbacks object would be a preferable solution to using a modified version of something like AmplifyJS, as it allows us to develop this with only jQuery as a dependency (and even there only the two methods .each() and .extend() ), rather than relying on or attempting to duplicate the functionality of a 3rd-party plugin, which will be getting updated of its own accord, etc.

comment:8 in reply to: ↑ 7 azaozz22 months ago

Replying to vhauri:

... as it allows us to develop this with only jQuery as a dependency...

The above script doesn't have any dependencies. That may be preferable for some cases (front-end use?). The corresponding jQuery methods (each(), extend(), etc.) are tuned for wider range of uses like traversing and extending complex objects or the DOM which is not needed at the moment. So using a for... in loop instead of jQuery.each() and obj[key] = val; instead of jQuery.extend() is faster and keeps the script self-contained.

Last edited 22 months ago by azaozz (previous) (diff)

comment:9 vhauri22 months ago

@azaozz The above script is a maintained plugin that is being constantly changed (i.e. it is itself the dependency). I'm not sure how we'd maintain cross-compatibility with it, but if your thought is that we use it as a basis for a vanilla JS library (one that doesn't maintain any sort of compatibility with the latest and greatest version of AmplifyJS) that does the actions/hooks stuff, that sounds reasonable. My preference for basing off jQuery is that the Callbacks object will likely be maintained for us, so code that utilizes it will also be somewhat future-proof. I'm just not sure whether that'll be the case for AmplifyJS, but obviously if we're just using the codebase as it stands now as a branching point, no issues.

I think this speaks to the larger issue of whether jQuery itself should be assumed. Your point about the front-end (i.e. in comment-reply.js) does make me think that maybe there's a need to separate this from jQuery, although I do feel that jQuery is the de-facto JS library that WP uses at this point.

comment:10 usermrpapa22 months ago

  • Cc steve@… added

comment:11 in reply to: ↑ description mikeschinkel22 months ago

  • Cc mikeschinkel@… added

Replying to koopersmith:

The concept of adding JavaScript actions and filters has been tossed around for some time. We've experimented with various configurations of actions in both the fullscreen and customizer APIs, and they've proven their utility enough to graduate them to a core feature in their own right.

This is an excellent ticket, really glad to see it. We've been using something very much like this in our client's systems for the past year. I've copied the code from plugin below changing the name from our plugin's name to 'wp'; I've also added as an attachment.

Basically what we did was added a "hooks" object as a wp property of the jQuery object, i.e. $.wp:

/*
  jQuery Hooks for WordPress

  Examples:

  // Add three different test actions
  $.wp.addAction( 'test', function() { alert('Foo!'); } );
  $.wp.addAction( 'test', function() { alert('Bar!'); } );
  $.wp.addAction( 'test', function() { alert('Baz!'); } );

  // Remove the first one
  $.wp.removeAction( 'test', 'test_1' );

  // Do the remaining test actions
  $.wp.doAction( 'test' );


  // Add a filter somewhere
  $.wp.addFilter('filterOptions',function(options) {
    // Do stuff here to modify variable options
    return options;
  } );

  // Use the filter here
  options = $.wp.applyFilters('filterOptions',options);

 */
jQuery(document).ready(function($) {
  $.wp = {
    /**
     * Implement a WordPress-link Hook System for Javascript 
     * TODO: Change 'tag' to 'args', allow number (priority), string (tag), object (priority+tag)
     */
    hooks: { action: {}, filter: {} },
    addAction: function( action, callable, tag ) {
      jQuery.wp.addHook( 'action', action, callable, tag );
    },
    addFilter: function( action, callable, tag ) {
      jQuery.wp.addHook( 'filter', action, callable, tag );
    },
    doAction: function( action, args ) {
      jQuery.wp.doHook( 'action', action, null, args );
    },
    applyFilters: function( action, value, args ) {
      return jQuery.wp.doHook( 'filter', action, value, args );
    },
    removeAction: function( action, tag ) {
      jQuery.wp.removeHook( 'action', action, tag );
    },
    removeFilter: function( action, tag ) {
      jQuery.wp.removeHook( 'filter', action, tag );
    },
    addHook: function( hookType, action, callable, tag ) {
      if ( undefined == jQuery.wp.hooks[hookType][action] ) {
        jQuery.wp.hooks[hookType][action] = [];
      }
      var hooks = jQuery.wp.hooks[hookType][action];
      if ( undefined == tag ) {
        tag = action + '_' + hooks.length;
      }
      jQuery.wp.hooks[hookType][action].push( { tag:tag, callable:callable } );
    },
    doHook: function( hookType, action, value, args ) {
      if ( undefined != jQuery.wp.hooks[hookType][action] ) {
        var hooks = jQuery.wp.hooks[hookType][action];
        for( var i=0; i<hooks.length; i++) {
          if ( 'action'==hookType ) {
            hooks[i].callable(args);
          } else {
            value = hooks[i].callable(value, args);
          }
        }
      }
      if ( 'filter'==hookType ) {
        return value;
      }
    },
    removeHook: function( hookType, action, tag ) {
      if ( undefined != jQuery.wp.hooks[hookType][action] ) {
        var hooks = jQuery.wp.hooks[hookType][action];
        for( var i=hooks.length-1; i>=0; i--) {
          if (undefined==tag||tag==hooks[i].tag)
            hooks.splice(i,1);
          }
        }
      }
  }
});

This code has worked really well for us and stood the test of time, though I do have a TODO comment in the code to enable priorities. Maybe it could be a base for something in 3.5?

mikeschinkel22 months ago

wp-hook.js which is a copy of the same functionality from Sunrise.

comment:12 follow-up: azaozz22 months ago

Replying to vhauri:

...but if your thought is that we use it as a basis for a vanilla JS library (one that doesn't maintain any sort of compatibility with the latest and greatest version of AmplifyJS) that does the actions/hooks stuff, that sounds reasonable.

Exactly, the patch is an earlier attempt to implement this as simple as possible. It was based off of AmplifyJS (it "borrows" some code from there). However it is quite different from the original and doesn't interfere with it, and there would be no need for any maintenance when AmplifyJS is updated.

The jQuery Callbacks() looks like a good candidate but is missing some key features like priorities and namespaces. Considering it would be preferable to keep this API self-contained, perhaps we can settle for a simpler solution using few loops instead of trying to patch/extend Callbacks(). That would need less maintenance too.

comment:13 in reply to: ↑ 12 ; follow-up: vhauri22 months ago

Replying to azaozz:

The jQuery Callbacks() looks like a good candidate but is missing some key features like priorities and namespaces. Considering it would be preferable to keep this API self-contained, perhaps we can settle for a simpler solution using few loops instead of trying to patch/extend Callbacks(). That would need less maintenance too.

Callbacks does offer the .fired() method to test if a particular callback has been fired: http://api.jquery.com/callbacks.fired/. Being able to tell specifically whether or not a particular callback or list of callbacks has fired would be useful. That doesn't solve the problem of actually being able to call one method before another, but I do think it's a worthwhile feature that should be added into whatever solution ends up being used.

comment:14 in reply to: ↑ 13 azaozz22 months ago

Replying to vhauri:

Callbacks does offer the .fired() method to test if a particular callback has been fired... I do think it's a worthwhile feature that should be added into whatever solution ends up being used.

Agree, there is similar functionality in PHP: did_action() and in the above example: wp.didAction(). It wouldn't be hard to extend this so it can be used for each callback, i.e. support both wp.didAction(action) and wp.didAction(action.namespace).

comment:15 sirzooro22 months ago

  • Cc sirzooro added

comment:16 ocean9022 months ago

  • Cc ocean90 added

comment:17 CaptainN22 months ago

Just thought I'd offer a somewhat different model (especially since you don't want to go with a standard DOM implementation).

The idea is modeled after Robert Penner's AS3Signals, which itself if modeled after C#'s event model. It has a couple of advantages compared with DOM style events (and some disadvantages - propagation would need to be implemented - that's not as much of an issue for WP style filters and actions though) - the primary benefit IMO is it uses properties instead of strings for event types (louder errors when you type something wrong).

Here is a tiny implementation of Signals in JS:
https://github.com/CaptainN/SignalsLite.js/blob/master/src/SignalLite.js

This library doesn't have the requirements listed in the ticket, but they could be added pretty easily (I'll probably add them this weekend, cause they seem like nice features to have).

Here's the primary difference in syntax for the end user:

// DOM style events:
/* event is of type Event */
wphooks.actions.addEventListener( "click.namespace", function( event ) {}, 2 /* priority */);
// or
function funcRef( event ) {}
wphooks.actions.addEventListener( "click.namespace", funcRef, 2  /* priority */);
wphooks.actions.removeEventListener( "click.namespace", funcRef );

// signals (with unimplemented priority and namespacing):
/* obj is of whatever type you want it to be - much more like WP filters */
wphooks.actions.clicked.add( "namespace", function( obj ) {}, 2 /* priority */ );

// or
function funcRef( obj ) {}
wphooks.actions.clicked.add( funcRef, 2 /* priority */ );

// removes all namespaced listeners by string
wphooks.actions.clicked.remove( "namespace" );

// removes only the one listener by reference (DOM style)
wphooks.actions.clicked.remove( funcRef );

You'd construct it like so:

window.wphooks = {
    actions: {
        clicked: new SignalLite(),
        other_custom_action: new SignalLite()
    }
    filters: {
        some_filter: new SignalLite(),
        some_other_filter: new SignalLite()
    }
}

// add additional hooks elsewhere (you could do an addAction abstraction too).
wphooks.actions.my_added_action = new SignalLite();

dispatch works like this:

// maps "this" in the listener to thisObj
wphooks.actions.clicked.target = thisObj;
// obj is whatever you need it to be for this hook
wphooks.actions.clicked.dispatch( obj  );

This may be out of left field, but I thought I'd throw it in the discussion. :-)

comment:18 scribu22 months ago

In the signals model, you have to create a new SignalLite instance for each hook. That seems like a hindrance rather than an advantage.

comment:19 follow-up: CaptainN22 months ago

It's a lot of very small objects (as apposed to a lot of strings in other models), is that really a big deal?

You could also use a factory method of some kind to construct those (and to add/remove custom actions so plugin authors wouldn't have to instantiate any Signal instances directly).

I was also thinking of a way to do lazy instantiation using a getter/setter model (you still get the better error checking on property access, without instantiating all those objects up front) - but that IE monster keeps it from happening (there are ways in IE even if they are bit hacky).

comment:20 in reply to: ↑ 19 azaozz22 months ago

Replying to CaptainN:

It's a lot of very small objects (as apposed to a lot of strings in other models)

Yes, I can see some advantages in that. If each hook name refers to an instance, we could have couple of basic methods there, add(), remove(), fired(), etc. even if add() is just [].splice() and remove is [].slice().

The important thing is to keep the callbacks (which could be instances of a small "callback" class) in a true JS array so the priority is maintained. Unlike PHP, JS doesn't guarantee that ordering in objects will be maintained.

You could also use a factory method of some kind to construct those (and to add/remove custom actions so plugin authors wouldn't have to instantiate any Signal instances directly).

Yeah, that's a must. Another somewhat similar implementation of custom events is in TinyMCE. It uses instances for both events and callbacks and seems pretty well build (doesn't have priority for the callbacks but has addToTop() method): http://www.tinymce.com/wiki.php/API3:class.tinymce.util.Dispatcher,
code at: https://github.com/tinymce/tinymce/blob/master/jscripts/tiny_mce/classes/util/Dispatcher.js

comment:21 johnbillion22 months ago

  • Cc johnbillion added

comment:22 mikeschinkel22 months ago

Is this now actually planned for v3.5?

comment:23 CaptainN22 months ago

  • Cc CaptainN added

SignalsLite.js now has most of these features. It's missing only the priority flag - namespaces, addToTop and safe dispatching are all implemented.

https://github.com/CaptainN/SignalsLite.js

There are even a bunch of unit tests (check there for usage for now, if you are curious):
http://www.unfocus.com/SignalsLite.js/tests/AllTests.html

In case there is any interest.

comment:24 follow-up: CaptainN22 months ago

Hmm, one thing I just noticed about SignalLite, and a lot of the other JavaScript callback systems, is they don't really provide a way to do WordPress style filters (other than having an additional whole set of callbacks). That is, send a value to the listener, and then deal with whatever the listener returns between each listener.

It seems a JavaScript way to do something like that would be the DOM Event model of passing an object with some properties that can be used to apply the filter, then the system could just ignore return values. But that's not very WordPressy.

Maybe the way to go is to add a return handler that the implementer is meant to support in the SignalLite constructor, if they choose:

var signaled = new SignalLite( null /* default target */,
    function( value ) {
        // do something with the returned value
    }
);

This would actually create a property on the signal:

// you could also use this alternative syntax
var signaled = new SignalLite();
signaled.eachReturned = function( value ) {
    // do something with the returned value
}

Here's what an implemented filter might look like:

/// in core/plugin code - implementation
var value_tofilter = "some value of stuff";

// use this or a factory method
window.wpFilter = {
  value_filter: new SignalLite( null,
    function( value ) {
        value_tofilter = value;
    }
  )
}

// ... in client code - user of the filter
wpFilter.value_filter.add( function( value ) {
    value.replace( "some", "THE" );
    return value;
} );

// ... back in core code - dispatching the filter
value_tofilter = wpFilter.a_filter.dispatch( value_tofilter );

console.log( value_tofilter );
// LOG: THE value of stuff

I'll probably go ahead and implement that. :-)

comment:25 in reply to: ↑ 24 azaozz22 months ago

Replying to CaptainN:

Hmm, one thing I just noticed about SignalLite, and a lot of the other JavaScript callback systems, is they don't really provide a way to do WordPress style filters...

The JS way to do something similar is to pass around simple objects. As they are always passed by reference changing a value in a callback function would change it "everywhere".

Was planning to add "filters" and handle that automatically in the above example but passing object(s) is doable with the "actions" too, so not sure if it's needed. The only reason is that it would make this API more familiar to developers that are used to the PHP API.

comment:26 CaptainN22 months ago

Yeah, the JS way is to pass by reference, for sure. I added support to SignalLite anyway, because it was a gap (you can even filter by value):

var signaled = new SignalLite();

var value_tofilter = "some value of stuff";

var wpFilter = {
	value_filter: new SignalLite( null,
		function( value, args ) {
			value_tofilter = value;
			// set the argument so it'll pass modified to the next listener
			args[ 0 ] = value;
		}
	)
};

wpFilter.value_filter.add( function( value ) {
	value = value.replace( "some", "THE" );
	return value;
} );

wpFilter.value_filter.add( function( value ) {
	value = value.replace( "stuff", "things" );
	return value;
} );

wpFilter.value_filter.dispatch( value_tofilter );

equal( value_tofilter, "THE value of things", '"some" should be "THE" and "stuff" should be "things"' );

comment:27 Mamaduka22 months ago

  • Cc georgemamadashvili@… added

comment:28 carldanley22 months ago

Hi all,

I've been working with lgedeon to compile a list of requirements for this library and then spent some time building something custom to support what we might need. Long story short, let me jump into it:

Supported Features:

  • jQuery style dot namespacing
    • Can add/remove namespaces like so:
      $wp.hooks.addAction( 'some.crazy.action', func, 5 );
      $wp.hooks.addAction( 'some.crazy', func2, 7);
      $wp.hooks.removeAction( 'some.crazy.action' );
  • Supports method chaining to create a jQuery-like function chain:
    $wp.hooks.addAction().addFilter().removeFilter().doAction();
  • Priorities
    • Ability to pass applicable callbacks through a process method of choice so filters can be processed in predefined functions. The object that holds the predefined filtering methods can also be worked to support extensions in case developers want to write their own filtering methods. In example:
      $wp.hooks.doAction( 'some.action', { 'some' : 'data' }, 'processingMethod' );
      *note: also works for applyFilter().
    • Currently has support for handling processing callbacks through a default 'sequential' method; I do have plans to implement the following methods:
      • minValue
      • maxValue
      • allTrue
      • allFalse
  • Supports unique identifiers
  • Anonymous functions are supported:
    $wp.hooks.addAction( 'anonymous.funcs', function(){ /* do something */ }, 30 );
  • Checks for a $wp global and binds itself to $wp.hooks
    • $wp was used in case we decide to create other library functions, etc.

Supported Functions:

  • addAction( action, callback, priority )
  • doAction( action, data, processMethod )
    • processMethod is currently string only but when I add support for registering new process methods, I intend on adding support for passing a defined or anonymous function directly through the processMethod parameter.
  • removeAction( action )
  • addFilter( filter, callback, priority )
  • applyFilter( filter, data, processMethod )
    • see note for 'processMethod' under the doAction() function.
  • removeFilter( filter )
  • didAction( action )
    • returns false or an integer count
  • didFilter( filter )
    • returns false or an integer count

If you have questions concerning logic and/or code implementation, please refer to my code commenting first. I have tried to heavily comment the API in areas that I thought may need further explanation or where I had ideas for future improvements. I will patch the github as necessary.

Also wanted to note that, when compressed, this library is only around 2kb ( so far ).

Code can be found on my github here: https://github.com/carldanley/WordPress-Action-Filter-Library
Demo can be temporarily found here: http://codebyter.dyndns.info/wp.hooks/

Last edited 21 months ago by carldanley (previous) (diff)

comment:29 follow-up: azaozz21 months ago

Looks very interesting. Couple of "first look" notes:

  • The complex namespace implementation looks good. However I'm not sure we need to go there. The term "namespace" is used loosely in the above example. It works about the same like in jQuery event namespacing and is more of a "unique identifier" than a real namespace. (That's my fault btw, should have called it "identifier"). It can be implemented as above: addAction('actionname.identifier', function(args){...}); or as separate arg: addAction('actionname', 'identifier', function(args){...});.

Generally the actions (and filters) are a way to call specific function at a specific time. Having a complex structure/tree of actions is not necessary as they are all called at that specific time anyway (same as attaching events to the DOM).

Perhaps if we are implementing "stop execution if any callback returns false" (and that's a big if, since there's no such functionality in the PHP version), having a more complex structure of identifiers (namespaces) would allow for one branch to be stopped while other branches continue. Still, that would be a "border case" functionality. (Normally a plugin would register one callback with addAction and would expect it to be executed every time when doAction is triggered. For a "tree" structure to work, plugins would have to share the "branch names" which is quite hard to do, or we will end up with a big tree with many branches that have only one "leaf" (callback) each, and that's not useful).

  • Not sure about support for 'processingMethod'. Imho it's better/more important to be able to directly set the scope for the callbacks (all similar libraries support that). It would be good if you can add some usage examples for this.
  • Chaining might be useful in some cases, however not all methods can be "chainable" as applyFilters will have to return the filtered value. Maybe it would be better to leave chaining off and return true/false on whether addAction/addFilter/removeAction/removeFilter was successful or not.

comment:30 in reply to: ↑ 29 ; follow-up: carldanley21 months ago

It was fun to write! Couple of responses/questions:

The term "namespace" is used loosely in the above example.

I agree, it was hard coming up with a term that encapsulates the object for both actions & filters. If you looked through the demo, they are essentially treated the same, except for how they are processed; the only slight variance is that filters can modify the value passed to it. That said, what do you think we could call these "objects" to help reduce confusion and standardize it? I will make the changes in comments/code to reflect the more accurate choice of words.

Having a complex structure/tree of actions is not necessary as they are all called at that specific time anyway (same as attaching events to the DOM).

Are you referencing how the objects for filters & actions are bound with 3 properties ( id, callbacks & children ) and sort of nested within each other? What if we did the following:

  • rework the internal storage to have 2 variables or keys for 'actions' and 'filters'.
  • when addAction() is called with a namespace, split it and find every hierarchical identifier to check to see if they exist, creating them, if they don't already exist, as you go, ie. you add "my.awesome.func", build an array of "levels" of identifiers ( [ 'my', 'my.awesome', 'my.awesome.func' ] ) and then loop through the array. The identifiers could be hashed or literally written as the exact key with string filter to make sure there are no illegal characters as a key within the corresponding array.

Chaining might be useful in some cases, however not all methods can be "chainable" as applyFilters will have to return the filtered value.

Just to point out that not all methods in jQuery are 'chainable' either, example; $( object ).height(); or $( object ).width(); I think for this particular case chaining makes good sense when doing something. In terms of useability, I'd rather see/type

$wp.hooks.addAction().addAction().addFilter().addFilter();

than

$wp.hooks.addAction();
$wp.hooks.addAction();
$wp.hooks.addFilter();
$wp.hooks.addFilter();

It's easier to type the first example than the latter.

What do you think? I appreciate the feedback and am willing to rework logic on this. I've been pumped ever since lgedeon mentioned this ticket to me.

Last edited 21 months ago by carldanley (previous) (diff)

comment:31 in reply to: ↑ 30 ; follow-up: azaozz21 months ago

Replying to carldanley:

... they (actions and filters) are essentially treated the same, except for how they are processed; the only slight variance is that filters can modify the value passed to them.

Right, they are handled the same way in PHP too. Essentially they are the same thing except for filters the returned value of each callback is passed on to the next callback as first arg, and returned at the end.

Are you referencing how the objects for filters & actions are bound with 3 properties ( id, callbacks & children ) and sort of nested within each other?

Uh, sry I didn't explain that well. Was referring to support for more than one namespace/identifier, like: actionname.namespace.other.third. When parsed that would create a tree-like structure for each action that doesn't seem needed as all actions would be called regardless of the structure. So a "flat" structure, i.e. one branch (actionname) with many leaves (callbacks) would work just as well. The .namespace/identifier bit would only be used to identify the callback, so each "leaf" on the "branch" would have a name and we would be able to easily remove anonymous "leaves" by branch name + leaf name :)

(In 21170.patch the term "namespace" should be read "identifier" as that probably describes it's role better).

Also thinking the namespaces/identifiers should be required as that would make removing anonymous functions easy. As far as I see the most common use of this would be with anonymous functions: `addAction('init.my-thing', function(){...});

What if we did the following:

  • rework the internal storage to have 2 variables or keys for 'actions' and 'filters'.

Actions and filters should be kept separate, i.e. addAction('init') makes this an "action node", there can't be a filter 'init', applyFilters('something', val) would be another node, etc. If we create the nodes as instances of a small class, or even as simple objects, we can store the nodeType in each.

  • when addAction() is called with a namespace, split it and find every hierarchical identifier to check to see if they exist, creating them, if they don't already exist, as you go, ie. you add "my.awesome.func", build an array of "levels" of identifiers ( [ 'my', 'my.awesome', 'my.awesome.func' ] ) and then loop through the array.

Not sure there's a need for that hierarchy (as explained above). Perhaps some usage examples would help here.

...I think for this particular case chaining makes good sense when doing something. In terms of useability, I'd rather see/type

$wp.hooks.addAction().addAction().addFilter().addFilter();

Yeah, chaining is definitely handy. The question is whether it's better to support chaining or to return true/false on success/failure when adding or removing actions and filter. The example in 21170.patch (and jQuery.Callbacks() I believe) would only add a particular callback once, so returning false on attempts to add it again makes sense. I'm 50/50 on this at the moment, would be good to get more opinions :)

What do you think? I appreciate the feedback and am willing to rework logic on this. I've been pumped ever since lgedeon mentioned this ticket to me.

Yes, it's an interesting challenge. Perhaps if we get it right, it can also be released as a stand-alone JS class.

comment:32 in reply to: ↑ 31 ; follow-up: carldanley21 months ago

Replying to azaozz:

The .namespace/identifier bit would only be used do identify the callback, so each "leaf" on the "branch" would have a name and we would be able to easily remove anonymous "leaves" by branch name + leaf name

I understand what you're saying now. The only question I have is how do we traverse the array to find simulated 'parents' and 'children'. If doAction( 'some.action' ) was called and we know there were several actions that existed such as 'some.action.a' and 'some.action.b', would we first find 'some.action' and then maybe look to a 'family' property that will indicate who the parent/children are? I don't think traversing each key in this flat array and determining who is prefixed with the correct action/identifier is the safest way, especially using for( var in ) to iterate. Thoughts?

If we create the nodes as instances of a small class, or even as simple objects, we can store the nodeType in each.

I agree in saying that we should keep the actions/filters in separate containers. I don't think it's necessary to specify nodeType of 'action' for 100+ different actions, as an example. We already know that it's an action because we have to access the 'ACTIONS' variable and, for this example, storing a 'nodeType' for each ACTION of value 'action' would mean making the array 600+ bytes heavier at run-time. I would use the same new 'object' class for both actions and filters and maybe pass the nodeType to the scope when needed. Thoughts?

Yeah, chaining is definitely handy. The question is whether it's better to support chaining or to return true/false on success/failure when adding or removing actions and filter.

You're also making me sway on chaining vs. true/false. It would interesting to hear other opinions as well.

Perhaps if we get it right, it can also be released as a stand-alone JS class.

That's what I'm shooting for! :)

Last edited 21 months ago by carldanley (previous) (diff)

comment:33 in reply to: ↑ 32 ; follow-up: azaozz21 months ago

Replying to carldanley:

I understand what you're saying now. The only question I have is how do we traverse the array to find simulated 'parents' and 'children'.

Yes, that's exactly the thing. This is modelled after the PHP actions API that has been in WP for years and works quite well. There aren't any sub-divisions / sub-branches / children and grandchildren in an action. That keeps things simple. Each action is a single branch that only has leaves (callbacks). It also matches the DOM events API, there's no 'click.one', click.other', etc. When 'click' is triggered, everything attached to it is fired.

If doAction( 'some.action' ) was called and we know there were several actions that existed such as 'some.action.a' and 'some.action.b', would we first find 'some.action' and then maybe look to a 'family' property that will indicate who the parent/children are? I don't think traversing each key in this flat array and determining who is prefixed with the correct action/identifier is the safest way, especially using for( var in ) to iterate. Thoughts?

If we stick to the current model,

doAction( 'some.action' )
doAction( 'some.action.a' )
doAction( 'some.other' )

will not be supported. The action for all these would be

doAction( 'some' )

and all callbacks that were registered with

addAction( 'some.thing', function(){...} )
addAction( 'some.action', function(){...} )
addAction( 'some.identifier', function(){...} )

would be fired. So in addAction() the identifier only applies to the callback and has nothing to do with the action itself.

On the other hand, as I said in comment:29, perhaps we could support "true" namespacing for the actions. That would open some advanced possibilities to additionally "filter" which callbacks are called.

In this case each action would be a container of many actions that start with the same name, and each child action can also be a container for even more actions (grandchildren), and so on. So an action would contain both callbacks and other actions. This would complicate the API quite a bit even if we limit the depth to, lets say 3 levels.

Perhaps we should consider all pros and cons. For example, lets say we have an action 'init' with several namespaces/children: 'init.some', 'init.some.more', 'init.other'.

  • Firing 'init' would fire all callbacks registered directly under 'init', plus all callbacks registered under 'init.some', plus all callbacks registered under 'init.some.more', plus all callbacks registered under 'init.other', plus... etc. Exactly the same thing can be achieved by firing several actions: 'init', 'init_some', 'init_some_more', 'init_other', etc. one after the other.

So with namespaced actions we only need:

doAction( 'init' );

without namespaces we need:

doAction( 'init' );
doAction( 'init_some' );
doAction( 'init_some_more' );
doAction( 'init_other' );

and we have control over which action is fired first, second, etc.

Advantage of namespacing: shorter code, disadvantage: no control over firing order.

  • Setting a namespace first then firing the action. Lets say we can set the namespace to '.some'. Firing 'init' should fire only callbacks under 'init.some' and 'init.some.more'. That's still exactly the same as firing 'init_some' and 'init_some_more' as separate actions.

With namespaced actions we need:

setNamespace( 'some' );
doAction( 'init' );
setNamespace( '' ); // depends on whether we reset the namespace after each action

without namespaces we need:

doAction( 'init_some' );
doAction( 'init_some_more' );

and we can choose to do 'init_some_more' before 'init_some'. Advantage: none, disadvantage: same as above.

  • Firing directly 'init.some' should fire callbacks under 'init.some' and 'init.some.more'. Still the same as firing the callbacks in 'init_some' and 'init_some_more' separately with the same advantages and disadvantages.
  • Setting the namespace to 'other' or firing directly 'init.other' should fire the callbacks registered under 'init.other'. That's exactly the same as simply firing 'init_other'.

So the advantage of using namespaced vs. "flat" actions is a bit shorter code, the disadvantages are: more complex API, different than the PHP actions (that's a big one), no control of child actions firing order.

Another possibility is to add namespacing support in the API and never use it in core. Usually the core code does doAction and applyFilters, and plugins do addAction and addFilter, so if core never uses action namespaces, the plugins wouldn't need to deal with the differences from the PHP API.

I agree in saying that we should keep the actions/filters in separate containers.

Yes, they either have to be in separate containers or we can have a flag in each object, perhaps something like isFilter: true/false. Both would work well and require about the same amount of code.

comment:34 in reply to: ↑ 33 mikeschinkel21 months ago

Replying to azaozz:

Perhaps we should consider all pros and cons. For example, lets say we have an action 'init' with several namespaces/children: 'init.some', 'init.some.more', 'init.other'.

Keep it very simple, like the original wiki:

"What's the simplest thing that will work?"

If there have ideas we might want to implement, like using periods for namespacing, then enforce a convention where hook names can't contain periods in the first implementation. Then wait and see how people actually use JavaScript actions and filters before making them more advanced based on speculation.

Better to get something simple into 3.5 than waiting who knows how many versions for perfection. Shipping is a feature. :)

comment:35 CaptainN21 months ago

I added namespace support to SignalsLite.js (all unit tested):

It works like this:

// setup in core somewhere, or elsewhere
var actions = {
    // simple factory method
    addAction: function( name ) {
        this[ name ] = new SignalLite( this );
    }
}

// wp core or someone else, adds the action hooks
actions.add( 'wpAction' );

actions.wpAction.ns.add( 'myNamespace' );

// now the namespaces actions can be used
actions.wpAction.myNamespace.add( function() {
    // do something with the action    
} );

// later when it comes time to cleanup:
actions.wpAction.myNamespace.remove();

I didn't add a way to dispatch a Signal only to a namespaced sub-set of listeners, because it didn't seem to make sense to expose that kind of functionality. If a listener was added to a Signal, it seems inappropriate to only send the signal to certain listeners, it adds a second level of subscriber sets, and that can be abused to basically duplicate the dispatcher functionality, where there should probably be a second named action - it may be better to encourage the creation of another distinct hook.

I think event namespaces are mostly useful as a tool for plugin developers to use to be able to clean themselves up, without having to retain a reference to all the listeners they may add all over.

jQuery does support namespaced dispatching, and if there was a strong desire for namespaced dispatching, it could be added easily to SignalsLite.js. I'm not sure how often you'd have plugin or extension developers triggering or dispatching actions and filters (like they frequently do in jQuery), so it may not really be an issue, unless it's wanted for core use

Edit: I used the wrong markup format. heh

Last edited 21 months ago by CaptainN (previous) (diff)

comment:36 koopersmith21 months ago

Some comments on the wp-hooks implementation in IRC.

comment:37 follow-up: CaptainN21 months ago

SignalsLite.js now has every feature in the ticket (and then some, like safe dispatching, addToTop, once, eachReturn handler for filtering) - including priorities, namespaces and a way to cancel dispatch. And, it's incredibly fast (crazy fast!). I did end up moving the safe dispatcher out to it's own sub class (SignalDispatcher), because it was big, relies on the DOM, and is frankly, not the fasted thing in the world (it's worth it some cases! but maybe not every case). In its place is a simple trigger method, which will just error out (like jQuery) if there's an error in a listener.

Anyway, now that that base is done, maybe I'll take a stab at putting the actual WordPress part around it - if there is any interest (I'm not sure how far along any other implementations are).

Here's what the API might look like with Signals - you could keep things string based if you wanted, and still expose the Signal feature set (which I think has some advantages compared with period based namespacing, or the full set of add methods you get with Signal priorities). Here's a quick example of what could be done:

// add hook with chaining
// everything after addAction() is implemented and unit tested in SignalsLite.js
$wp.hooks.addAction( "actionname" )
    .ns( "myNamespace" )
        .add( function() {} );

// additional adds can be property based (they don't have to be though)
$wp.hooks.actionname.myNamespace.add( function() {} );

// here's what priorities look like
$wp.hooks.addAction( "actionname" )
    .priority( 10 )
        .addToTop( function() {} );

Edit: Er, SignalsLite.js has every feature in the ticket required by the dispatcher component. It still doesn't have the WP specific stuff - the filter/action structure, or the ability to track which ones have already been triggered, etc. That'd need to be implemented on top of SignalsLite.js.

Last edited 21 months ago by CaptainN (previous) (diff)

comment:38 iandunn20 months ago

  • Cc ian_dunn@… added

comment:39 mitchoyoshitaka20 months ago

  • Cc mitcho@… added

comment:40 husobj19 months ago

  • Cc ben@… added

comment:41 toscho19 months ago

  • Cc info@… added

comment:42 in reply to: ↑ 37 ; follow-up: webord19 months ago

  • Cc webord.net@… added

Replying to CaptainN:
Question, what's the status on this ticket? I would love to check the code you guys have been working with.

comment:43 in reply to: ↑ 42 CaptainN19 months ago

Replying to webord: I haven't done much in the way of a patch. I mostly was focusing on how to do the actual dispatching, and figured a lot of the feature requirements actually belong in that realm. It was actually a fun way to get back into JavaScript after a couple years on other platforms. You can find all the SignalsLite.js stuff here:

https://github.com/CaptainN/SignalsLite.js

It's pretty robust at this point, and there are unit tests for it. I wasn't sure of the interest level in using SignalsLite.js, and it looks like there may already be another dispatch infrastructure.

This is from the IRC link:
https://github.com/carldanley/WordPress-Action-Filter-Library/blob/master/wp.hooks.js

Anyway, I don't own this ticket. You'd have to as the WP folks for more status info. :-)

comment:44 nacin19 months ago

  • Milestone changed from 3.5 to Future Release
  • Type changed from task (blessed) to feature request

comment:45 kadamwhite18 months ago

  • Cc kadamwhite added

comment:46 juliobox18 months ago

  • Cc juliobosk@… added

comment:47 technosailor18 months ago

  • Cc aaron@… added

comment:48 alexkingorg18 months ago

  • Cc public@… added

comment:49 Bueltge18 months ago

  • Cc frank@… added

comment:50 aaroncampbell15 months ago

  • Cc aaroncampbell added

comment:51 follow-up: scribu15 months ago

  • Keywords 2nd-opinion added

Is this still needed, given that Backbone is bundled into Core since 3.5? Backbone.events seems to be good enough.

Basic usage:

var events = _.clone(Backbone.events);

events.on('some_action', some_callback);

// later

events.trigger('some_action', 'some_data');

comment:52 in reply to: ↑ 51 ; follow-ups: azaozz15 months ago

Is this still needed...

The original idea was to have add_action(), add_filter(), do_action(), etc. in JS that behave similarly to PHP. Unfortunately neither Backbone.events nor jQuery.Callbacks() have a way to specify callback priority. Also missing did_action() which might be useful to have.

comment:53 in reply to: ↑ 52 vhauri15 months ago

The original idea was to have add_action(), add_filter(), do_action(), etc. in JS that behave similarly to PHP. Unfortunately neither Backbone.events nor jQuery.Callbacks() have a way to specify callback priority. Also missing did_action() which might be useful to have.

+1 to what @azaozz said...the priority of execution is important in the use-cases I have (or at least the ability to perform a did_action()-type call to check if another action has fired).

comment:54 azaozz14 months ago

A really nice (super fast and simple to use) implementation of add_action(), do_action(), etc. in JS: https://github.com/carldanley/WP-JS-Hooks

comment:55 stephenharris12 months ago

  • Cc stephenharris added

comment:56 Jesper80012 months ago

  • Cc jeve0@… added

comment:57 sennza12 months ago

  • Cc bronson@… added

comment:58 jblz11 months ago

  • Cc jeff@… added

comment:59 sc0ttkclark11 months ago

  • Cc lol@… added

comment:60 blepoxp10 months ago

  • Cc glenn@… added

comment:61 adamsilverstein9 months ago

  • Cc adamsilverstein@… added

comment:62 westonruter9 months ago

  • Cc weston@… added

comment:63 follow-up: adamsilverstein9 months ago

this would be a great feature to work on for 3.7!

comment:64 in reply to: ↑ 63 carldanley9 months ago

Replying to adamsilverstein:

this would be a great feature to work on for 3.7!

Couldn't agree more. I think there is a lot of potential for something like this in core. Filters come in super handy in JS when building an application that has a lot of different components. The library I wrote is pretty well optimized. I still need to tweak a few small things and cleanup some code BUT I think it's pretty solid.

There was some good talk about whether or not we could implement a solution that just extends jQuery to support the filters needed. I would love to continue working on this piece however possible; maybe even squeeze it into 3.7.

comment:65 MikeSchinkel9 months ago

Since WordPress has added Backbone.js as an important component of core, maybe the best approach is to adopt (an extension of) Backbone.Events for this?

comment:66 carldanley9 months ago

I've thought about that many times over and the fact I keep coming back to is that we'd need to enqueue backbone on the frontend as well (for things like comments, etc). I'm ok enqueueing Backbone wherever needed as I use it for all of my applications but more often than not, most plugins aren't using backbone and so they wouldn't make use of it at all if it were enqueued. Also, it's not a heavy library so it's not a huge determining factor.

We can extend things really easily when needed, I'm just trying to think of the most efficient way to make this available to everyone without a ton of dependencies.

comment:67 in reply to: ↑ 52 ; follow-up: MikeSchinkel9 months ago

Replying to azaozz:

The original idea was to have add_action(), add_filter(), do_action(), etc. in JS that behave similarly to PHP. Unfortunately neither Backbone.events nor jQuery.Callbacks() have a way to specify callback priority. Also missing did_action() which might be useful to have.

Saw this after posting today.

Rather than dismissing Backbone.Events you might want to consider extending them as Backbone.Wreqr does. It would not be too difficult to add priority and did_action() on top of Backbone.Events, and it's even possible someone else has already written most (all?) of the needed extensions.

comment:68 in reply to: ↑ 67 ; follow-up: carldanley9 months ago

Replying to MikeSchinkel:

Rather than dismissing Backbone.Events you might want to consider extending them as Backbone.Wreqr does. It would not be too difficult to add priority and did_action() on top of Backbone.Events, and it's even possible someone else has already written most (all?) of the needed extensions.

Just to clarify, I'm not dismissing it rather just saying that it becomes cumbersome. It's a bigger decision to extend Backbone because we then need to include Backbone & underscore everywhere in core (frontend & backend) to make this events implementation available. If that's the route people are leaning to, that's fine with me.

You can easily extend Backbone.Events when it's enqueued, to provide Backbone.Events the additional functionality this library would offer. Curious to hear thoughts from nacin and azaozz about this.

Version 0, edited 9 months ago by carldanley (next)

comment:69 in reply to: ↑ 68 MikeSchinkel9 months ago

Replying to carldanley:

Just to clarify, I'm not dismissing it rather just saying that it becomes cumbersome.

I was really just replying to the ticket vs. replying to your posts specifically. I wasn't ignoring your comments on purpose; your comments just caused me to see the ticket in my inbox again. I've been working with Backbone.js since I last commented on this ticket and now had some perspective to share.

It's a bigger decision to extend Backbone because we then need to include Backbone & underscore everywhere in core (frontend & backend) to make this events implementation available. If that's the route people are leaning to, that's fine with me.

That's a good point. I'm obviously not in the position to decide any of that.

What I have noticed is that the Events subset of Backbone is both elegant and small, and could be forked to load for when full Backbone is not loaded plus extended in a way that would be compatible with an event-only fork as well as full Backbone. FWIW.

BTW, the more I learning about the Javascript features used to create Backbone.js, the less I like programming in PHP.

comment:70 lgedeon9 months ago

I think a more compact solution that does not rely on backbone would provide better stability going forward. It could be done as a fork of backbone or designed to be compatible with it, though. Just saying, if many plugins are going to depend on it, we probably need to own it.

I am thinking something like Carl's code, if not a bit more compact, could become a very baked-in part of core code that doesn't have to be reworked or replaced as we add and remove other libraries.

Last edited 9 months ago by lgedeon (previous) (diff)

comment:71 azaozz9 months ago

...something like Carl's code could become a very baked-in part of core code that doesn't have to be reworked or replaced as we add and remove other libraries.

Agreed. Also note that for hooks to be effective the JS has to be the first thing that loads in the head (so the hooks can be used at any time). Having an external dependency is not ideal for this.

comment:72 follow-up: carldanley9 months ago

Dropping a note here for future reference: changed the lib at https://github.com/carldanley/WP-JS-Hooks to remove a lot of NFE's. Also changed the module pattern to a revealing module pattern. All tests pass.

comment:73 in reply to: ↑ 72 ; follow-up: adamsilverstein9 months ago

Replying to carldanley:

Dropping a note here for future reference: changed the lib at https://github.com/carldanley/WP-JS-Hooks to remove a lot of NFE's. Also changed the module pattern to a revealing module pattern. All tests pass.

I agree with not using Backbone unless we need it, and I love Backbone! I looked at your code very briefly and that got me excited - it looks like you are a long ways toward a solution for Core! I hope you will have some time to fine tune and test in the next two months, we could use your expertise.

comment:74 in reply to: ↑ 73 ; follow-up: carldanley9 months ago

Replying to adamsilverstein:

I hope you will have some time to fine tune and test in the next two months, we could use your expertise.

For sure! I'm watching this thread pretty closely. Just let me know however I can help.

comment:75 in reply to: ↑ 74 ; follow-up: adamsilverstein9 months ago

Replying to carldanley:

Replying to adamsilverstein:

I hope you will have some time to fine tune and test in the next two months, we could use your expertise.

For sure! I'm watching this thread pretty closely. Just let me know however I can help.

i may miss this wednesday's meeting in irc (travel day), but hoping we can form a 'JavaScript team' for 3.7 - in addition to Hooks & Filters, I'm hoping we can start the ball rolling on getting JavaScript unit tests into core - currently there is a strong push for PHP unit tests everywhere, and we need JS unit tests. See #24870.

comment:76 swissspidy9 months ago

  • Cc hello@… added

comment:77 beaulebens8 months ago

  • Cc beau@… added

comment:78 bradt8 months ago

  • Cc brad@… added

comment:79 itsananderson8 months ago

  • Cc will@… added

comment:80 buffler8 months ago

  • Cc jeremy.buller@… added

comment:81 Chouby7 months ago

  • Cc frederic.demarle@… added

comment:82 in reply to: ↑ 75 withinboredom3 months ago

Is there any update to this? I'm working on a overhauling a plugin's javascript and could really use this in core -- if there's someone working on it, I would love to help out and provide real world examples. And if not, I'm more than willing to get it started.

comment:83 ircbot3 months ago

This ticket was mentioned in IRC in #wordpress-dev by azaozz. View the logs.

comment:84 gcorne3 months ago

I am just now trying to get caught up with all the thinking that has gone into this. And, I still need to spend some time looking at and playing around with the fine work that Carl has done (Yay, for unit tests!). Here are few initial thoughts:

  • In the javascript land, we should probably consider garbage collection. In cases where we are dealing with short-lived “view” code that has references to DOM nodes, it would be nice if subscribers could easily clean up references to the event bus. This is the reason that Backbone added the listenTo and stopListening methods to Backbone.Events which is mixed in to all the other Backbone constructors.
  • I think we would want to support a context parameter so that when binding a callback the subscriber code can control what this refers to in the callback. We also need to consider how to best handle context on the publisher side. Do we want to support an arbitrary set of parameters that are then passed to the subscriber callback like the PHP plugin API supports?
  • I am not super up on benchmarks for the jQuery event system vs. others, but one thing that I don’t like about jQuery is that it is hard to inspect the catalog of registered subscribers to events. I think that it should be pretty easy to look at the data structure holding all the event bindings.
  • If we are going to create a dependency, I think it might be best for the dependency to be on underscore.js, which is only 4.7k compressed and includes lots of useful stuff for building a robust event/filter system.
  • Building on the idea of using underscore, we should build the core library so that it can be used under a separate namespace, so a core feature or a plugin could do something like:
wp.plugins.myAwesomePlugin.hooks = _.extend( {}, wp.Events );
  • In terms of style, I am not 100% sure how close it needs to match the PHP API, especially as the role of “front-end engineer” and the role’s heavy javascript skill set becomes more commonplace.
  • Has anyone looked through core for places that are good candidates for adding some hooks? It might be worth considering some specific use cases.

comment:85 jeremyfelt3 months ago

  • Focuses javascript added

comment:86 follow-up: azaozz3 months ago

In my opinion the biggest advantages of having a dedicated hooks API are:

  • Similarity to the PHP hooks. It's true that more and more (plugin) developers understand JS better, but there are a lot that will find this very desirable.
  • Interoperability. None of the event APIs we can use allow a callback to be scheduled to run before or after another callback, even before the second callback is defined. In the PHP actions and filters this is done with 'priority'. It has its limitations but has been working really well for about 10 years. Considering that some hooks would be used by core plus 2-3-4 plugins, having priority would be great.

Has anyone looked through core for places that are good candidates for adding some hooks?

Heartbeat works only with hooks (both in PHP and JS), now autosave has several hooks too.

I think we would want to support a context parameter

Agreed, having a scope can be quite handy and will make this better.

If we are going to create a dependency...

Don't think we need a dependency. The current patch is fast, simple and works very well.

...we should build the core library so that it can be used under a separate namespace

This may be handy in some cases however there are already jQuery custom events, jQuery callbacks, and Backbone events that can be used.

...it would be nice if subscribers could easily clean up references to the event bus. This is the reason that Backbone added the listenTo and stopListening

Yes, I see this rather as supporting removing hooks in "bulk" like jQuery's $(document).off('.my-hooks').

Last edited 3 months ago by azaozz (previous) (diff)

comment:87 adamsilverstein3 months ago

I keep coming across the desire to add JS hooks to core. Overall I would prefer functionality that closely mirrors the way we do hooks in PHP - carldanley's github code looks good and I would love to see this in core so we can start adding some hooks and testing!

If there is general consensus that this makes sense I would be happy to work up a patch to add the code (including unit tests) to core.

comment:88 in reply to: ↑ 86 ; follow-up: gcorne3 months ago

Considering that some hooks would be used by core plus 2-3-4 plugins, having priority would be great.

The ability to control execution order is definitely worth having.

Agreed, having a scope can be quite handy and will make this better.

I went ahead and had a go at adding a context parameter. See https://github.com/gcorne/WP-JS-Hooks/commit/e7ab26d9aae2f5383b8ec156454ce241bcb4a87e

I also think that we should incorporate the ability to remove a specific hook based on === comparison. Something like:

var func = function() {
    // do awesome stuff
};

wp.hooks.addAction( 'foo', func );
// lots of code

wp.hooks.removeAction( 'foo', func );

We could either make the callback reference optional in removeAction() and when not passed remove all, or perhaps create a separate method removeAllActions() .

comment:89 in reply to: ↑ 88 carldanley3 months ago

Replying to gcorne:

The ability to control execution order is definitely worth having.

+1.

I went ahead and had a go at adding a context parameter. See https://github.com/gcorne/WP-JS-Hooks/commit/e7ab26d9aae2f5383b8ec156454ce241bcb4a87e

+1. I left comments - just needs adjusted documentation.

I also think that we should incorporate the ability to remove a specific hook based on === comparison.
We could either make the callback reference optional in removeAction() and when not passed remove all, or perhaps create a separate method removeAllActions() .

+1 on this; would love to see a patch come through!

comment:90 gcorne3 months ago

I went ahead and added the ability to remove specific callbacks in https://github.com/gcorne/WP-JS-Hooks/commit/c109d4f4cd19fc22d89779cfa1a9ef3edcea6ee0.

One other enhancement that I think needs to before adding to core is support for jQuery-like namespacing to make cleaning up hooks easier, although since we don't have to deal with built-in event names, I wonder if we should be using a convention that is a bit less confusing. Appending a namespace (e.g. event.namespace seems a little backwards to me.

TV productions6 weeks ago

Implemented the wp hooks from https://github.com/gcorne/WP-JS-Hooks/

comment:91 TV productions6 weeks ago

In 21170-1.patch:

  • Implemented the last WP JS Hooks from gcorne.
  • Replaced $(document).trigger() for autosave.js, heartbeat.js, word-count.js, post.js, inline-edit-post.js, tinymce/plugins/wordpress/plugin.js and wp-auth-check.js.
  • I've tested it, but you should test it too.

Why I implemented this? Because I really like the idea of js hooks and this solution of gcorne seems good for now, because it looks a lot like the PHP API, so probably a lot developers will recognise it. Besides that it has no dependencies, what is nice for front-end (as you guys noted earlier).

TV productions6 weeks ago

comment:92 TV productions6 weeks ago

Hmm, forgot to add script-loader.php to 21170-1.patch.
In 21170-2.patch:

  • Extended 21170-1.patch
  • Added wp-hooks.js as dependency of heartbeat.js, autosave.js and word-count.js (among others).
Last edited 6 weeks ago by TV productions (previous) (diff)

comment:93 TV productions6 weeks ago

  • Keywords has-patch dev-feedback added
Note: See TracTickets for help on using tickets.