Skip to content
This repository has been archived by the owner on Jul 9, 2018. It is now read-only.

Add the WordPress hook library from 21170-core #13

Merged
merged 98 commits into from
Sep 22, 2017
Merged
Show file tree
Hide file tree
Changes from 73 commits
Commits
Show all changes
98 commits
Select commit Hold shift + click to select a range
6bca9da
add wp-hooks, first pass
Jul 18, 2017
85c73be
updates from ticket, flesh out removeAll, cleanup
Jul 19, 2017
dc71cd1
inline docs cleanup
Jul 19, 2017
4013c9a
remove some commented code
Jul 19, 2017
ff65810
downgrade initial version
Jul 19, 2017
be62409
clean up imports for (empty) tests file
Jul 19, 2017
45d70c3
bring over object approach from core instead of passing around string
Jul 20, 2017
632c98a
First pass, jest tests
Jul 20, 2017
77c9906
add remainder of tests
Jul 20, 2017
b4b713d
Docs refinements
Jul 20, 2017
9d24e77
Break out code into one file per funciton
Jul 20, 2017
57f6038
Centralize all run logic into `createRunHook`
nylen Jul 20, 2017
1901b43
Use a consistent order for functions
nylen Jul 20, 2017
b05f488
Merge branch 'add-wp-hooks' of github.com:adamsilverstein/packages in…
Jul 20, 2017
f6c2b45
Fix `currentFilter` and unit-test it
nylen Jul 20, 2017
f50aadb
Add `currentAction` and unit-test it
nylen Jul 20, 2017
bee29d7
Declare base hooks objects as objects, not arrays
nylen Jul 20, 2017
f8e129d
Merge branch 'add-wp-hooks' of github.com:adamsilverstein/packages in…
Jul 20, 2017
b02296c
Several fixes to hook removal behavior
nylen Jul 20, 2017
aeca7df
Fix `hasAction` and `hasFilter` if all hooks have been removed
nylen Jul 20, 2017
678f790
Remove now-unused `createRemoveAllHook`
nylen Jul 20, 2017
78acfe1
Naming and documentation cleanup
nylen Jul 20, 2017
637af4d
Make `hasAction` and `hasFilter` return the number of registered hooks
nylen Jul 20, 2017
b7b30f4
Remove unnecessary `describe`/`it` pairs
nylen Jul 20, 2017
b351db4
A bit more function declaration cleanup
nylen Jul 20, 2017
498e15b
Use ES6 object shorthand
nylen Jul 20, 2017
e3392c3
Add explicit error reporting
nylen Jul 20, 2017
53294fd
Test adding and removing multiple filters at the same priority
nylen Jul 20, 2017
07b61a0
Keep the hooks list always sorted instead of explicitly sorting it
nylen Jul 20, 2017
ee303e6
Remove remaining `var` statements
nylen Jul 20, 2017
b1251e3
Remove remaining per-test cleanup logic
nylen Jul 20, 2017
93998e1
Merge branch 'add-wp-hooks' of github.com:adamsilverstein/packages in…
Jul 20, 2017
7eda5e2
Add webpack and build wp-hooks for core
Jul 20, 2017
0e9986a
remove built file and adjust build path
Jul 20, 2017
c9890db
Remove unused dependencies
nylen Jul 20, 2017
de6efd7
Use an ES6 default argument to set the default hook priority
nylen Jul 20, 2017
ad47998
Refactor handlers objects: don't assign extra properties on arrays
nylen Jul 20, 2017
e217d7f
Merge branch 'add-wp-hooks' of github.com:adamsilverstein/packages in…
Jul 20, 2017
562cf37
Run core build JS thru babel
Jul 20, 2017
f882313
remove babel-loader for now
Jul 21, 2017
9eeb198
rename ‘current’ => ‘__current’ and make it a stack
Jul 21, 2017
fd0ccd3
Add initial readme based on https://github.com/adamsilverstein/WP-JS-…
Jul 21, 2017
fc48fbf
test remove a filter callback of lower priority when running hook
Jul 21, 2017
566af31
test remove a filter callback of same priority when running hook
Jul 21, 2017
6ce6268
prevent and test for (dingle level) hook recursion
Jul 21, 2017
83dffa4
test remove a filter callback of higher priority when running hook
Jul 21, 2017
2a60b9f
Failing recursive test: remove one hook in a callback before adding a…
Jul 21, 2017
79ef421
Add babel-loader for core build
Jul 21, 2017
742857a
Merge remote-tracking branch 'origin/master' into add-wp-hooks
nylen Jul 24, 2017
bf69f89
Clean up tests for removing a filter while filters are running
nylen Jul 24, 2017
bd911d0
Add test demonstrating desired recursion behavior
nylen Jul 24, 2017
b802c1d
Convert most functions in tests to arrow functions (for consistency)
nylen Jul 24, 2017
4e393d6
Add failing test for `currentFilter` with multiple filters running
nylen Jul 24, 2017
5d0513c
Fix `currentFilter` behavior
nylen Jul 24, 2017
c11003c
Add more `currentFilter` test assertions
nylen Jul 24, 2017
ad09872
Remove `wp-hooks.js` entry point
nylen Jul 24, 2017
a19fa3b
Merge remote-tracking branch 'origin/master' into add-wp-hooks
nylen Jul 24, 2017
0498ee8
Prevent hook names from starting with `__`
nylen Jul 24, 2017
5b8c5f3
Prevent running hooks with invalid names
nylen Jul 24, 2017
6327a54
Add missing tests for error conditions
nylen Jul 24, 2017
dd10eab
Make hook removal functions return the number of hooks removed
nylen Jul 24, 2017
8be21e5
Ensure that hook callbacks to remove are functions
nylen Jul 24, 2017
70b3ebc
Test running a filter with no callbacks
nylen Jul 24, 2017
a3b4901
`doAction` should return `undefined`
nylen Jul 24, 2017
6653227
Add failing test for recursive filters
nylen Jul 24, 2017
2e9b96e
First attempt at handling adding/removing callbacks during execution
nylen Jul 24, 2017
54f70c1
Make the filter adding/removing/recursion test pass
nylen Jul 24, 2017
afa92ea
Minor style and speed improvement
nylen Jul 24, 2017
0919ab9
Add missing tests
nylen Jul 24, 2017
d319b9d
Merge branch 'add-wp-hooks' of github.com:adamsilverstein/packages in…
Jul 25, 2017
90942a7
Add a centralized webpack folder based build for packages
Aug 1, 2017
e2547d9
Add check: the hook name can only contain numbers, letters and unders…
Aug 8, 2017
cefc79a
use yoda conditional as per wp coding standards
Aug 8, 2017
ca95500
removeHook - accept a namespace parameter and use it to identify item…
Aug 11, 2017
cc8b852
addHook - The hook name can only contain numbers, letters and undersc…
Aug 11, 2017
06ddd95
addHook - accept a namespace parameter and store in handler data
Aug 11, 2017
edd46d0
Use some Yoda conditionals
Aug 11, 2017
77be5ed
Ensure the passed namespace is a string of the form `my-plugin-slug/f…
Aug 11, 2017
2c62cdc
Add a build-modules command to run webpack
Aug 11, 2017
581e715
Add a `validateNamespace` helper, centralizing namespace checks
Aug 11, 2017
4c7f252
Add a `validateHookName` helper, centralizing hookName validation
Aug 11, 2017
ef06958
Use new hookName and namespace validation helpers throughout generato…
Aug 11, 2017
319a023
Update tests to use namespaces
Aug 11, 2017
4ece207
Update readme with new hook/namespace signature
Aug 11, 2017
aa709ac
Improve validateNamespace and validateHookName inline docs
Aug 12, 2017
a8628a0
require a non-empty string when validating namespace/hookName
Aug 12, 2017
242f351
Adjust expected error language in tests.
Aug 12, 2017
d8af28f
Improved regexes for hookName and namespace validation
Aug 12, 2017
650e9e2
Add more tests around hookName and namespace validation
Aug 12, 2017
a617214
Validate namespace takes form `vendorName/pluginName/functionName`
Aug 16, 2017
30e5e82
Update tests with new namespace format
Aug 16, 2017
3a13768
update readme with new namespace format
Aug 16, 2017
7c1fc29
remove module build from branch
Aug 17, 2017
45747e5
Correct namespace strings in docblocks
Sep 4, 2017
a24847b
Add args to applyFilters docs in readme
Sep 7, 2017
9f4cefc
camelCase for readme variable names
Sep 7, 2017
73744fa
update hook and `vendor/plugin/function` wording, removing `Name`
Sep 12, 2017
7465667
complte namespace shortening to `vendor/plugin/function`
Sep 13, 2017
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,9 @@
"jest": "^20.0.4",
"lerna": "^2.0.0",
"mkdirp": "^0.5.1",
"rimraf": "^2.6.1"
"rimraf": "^2.6.1",
"webpack": "^3.3.0",
"babel-loader": "^7.1.1"
},
"jest": {
"collectCoverageFrom": [
Expand Down
34 changes: 34 additions & 0 deletions packages/hooks/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,34 @@
# WP-JS-Hooks

A lightweight & efficient EventManager for JavaScript in WordPress.


### API Usage
API functions can be called via the global `wp.hooks` like this `wp.hooks.addAction()`, etc.

* `addAction( 'namespace.identifier', callback, priority )`
* `addFilter( 'namespace.identifier', callback, priority )`
* `removeAction( 'namespace.identifier', callback )`
* `removeFilter( 'namespace.identifier', callback )`
* `removeAllActions( 'namespace.identifier' )`
* `removeAllFilters( 'namespace.identifier' )`
* `doAction( 'namespace.identifier', arg1, arg2, moreArgs, finalArg )`
* `applyFilters( 'namespace.identifier', content )`
* `doingAction( 'namespace.identifier' )`
* `doingFilter( 'namespace.identifier' )`
* `didAction( 'namespace.identifier' )`
* `didFilter( 'namespace.identifier' )`
* `hasAction( 'namespace.identifier' )`
* `hasFilter( 'namespace.identifier' )`

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As discussed during core JS office hours today, I'd like to see us alter how we refer to these components -- in jQuery and other libraries "namespace" refers to the your-code-specific part following the period, and the event name is the part before the period. I believe that breaking from that convention would lead to a lot of confusion.

What I would propose instead:

addAction( 'actionname.namespace' );

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, action name should come first if that is the way we want to go. However, if it's required, maybe we should just make it a separate argument. See discussion starting at https://core.trac.wordpress.org/ticket/21170#comment:129.

Copy link
Member Author

@adamsilverstein adamsilverstein Sep 4, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kadamwhite this has been updated, can you please review the current format?



### Background
See ticket [#21170](http://core.trac.wordpress.org/ticket/21170) for more information.


### Features

* Fast and lightweight.
* Priorities system ensures hooks with lower integer priority are fired first.
* Uses native object hash lookup for finding hook callbacks.
* Utilizes insertion sort for keeping priorities correct. Best Case: O(n), worst case: O(n^2)
14 changes: 14 additions & 0 deletions packages/hooks/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
{
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest a files values to reduce bloat of distributed module:

https://docs.npmjs.com/files/package.json#files

e.g. omitting tests.

"name": "@wordpress/hooks",
"version": "0.1.0",
"repository": {
"type": "git",
"url": "https://github.com/WordPress/packages.git"
},
"description": "WordPress Hooks library",
"main": "build/index.js",
"module": "build-module/index.js",
"browser": "build-browser/index.js",
"author": "WordPress",
"license": "GPL-2.0+"
}
71 changes: 71 additions & 0 deletions packages/hooks/src/createAddHook.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,71 @@
/**
* Returns a function which, when invoked, will add a hook.
*
* @param {Object} hooks Stored hooks, keyed by hook name.
*
* @return {Function} Function that adds a new hook.
*/
function createAddHook( hooks ) {
/**
* Adds the hook to the appropriate hooks container.
*
* @param {string} hookName Name of hook to add
* @param {Function} callback Function to call when the hook is run
* @param {?number} priority Priority of this hook (default=10)
*/
return function addHook( hookName, callback, priority = 10 ) {
if ( typeof hookName !== 'string' ) {
console.error( 'The hook name must be a string.' );
return;
}

if ( /^__/.test( hookName ) ) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the interest of starting with a restrictive API that allows for expansion later, let's restrict hook names to something like /^[a-z][a-z0-9_]/.

console.error( 'The hook name cannot begin with `__`.' );
return;
}

if ( typeof callback !== 'function' ) {
console.error( 'The hook callback must be a function.' );
return;
}

// Validate numeric priority
if ( typeof priority !== 'number' ) {
console.error( 'If specified, the hook priority must be a number.' );
return;
}

const handler = { callback, priority };

if ( hooks.hasOwnProperty( hookName ) ) {
// Find the correct insert index of the new hook.
const handlers = hooks[ hookName ].handlers;
let i = 0;
while ( i < handlers.length ) {
if ( handlers[ i ].priority > priority ) {
break;
}
i++;
}
// Insert (or append) the new hook.
handlers.splice( i, 0, handler );
// We may also be currently executing this hook. If the callback
// we're adding would come after the current callback, there's no
// problem; otherwise we need to increase the execution index of
// any other runs by 1 to account for the added element.
( hooks.__current || [] ).forEach( hookInfo => {
if ( hookInfo.name === hookName && hookInfo.currentIndex >= i ) {
hookInfo.currentIndex++;
}
} );
} else {
// This is the first hook of its type.
hooks[ hookName ] = {
handlers: [ handler ],
runs: 0,
};
}
};
}

export default createAddHook;
27 changes: 27 additions & 0 deletions packages/hooks/src/createCurrentHook.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
/**
* Returns a function which, when invoked, will return the name of the
* currently running hook, or `null` if no hook of the given type is currently
* running.
*
* @param {Object} hooks Stored hooks, keyed by hook name.
*
* @return {Function} Function that returns the current hook.
*/
function createCurrentHook( hooks, returnFirstArg ) {
/**
* Returns the name of the currently running hook, or `null` if no hook of
* the given type is currently running.
*
* @return {?string} The name of the currently running hook, or
* `null` if no hook is currently running.
*/
return function currentHook() {
if ( ! hooks.__current || ! hooks.__current.length ) {
return null;
}

return hooks.__current[ hooks.__current.length - 1 ].name;
};
}

export default createCurrentHook;
24 changes: 24 additions & 0 deletions packages/hooks/src/createDidHook.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* Returns a function which, when invoked, will return the number of times a
* hook has been called.
*
* @param {Object} hooks Stored hooks, keyed by hook name.
*
* @return {Function} Function that returns a hook's call count.
*/
function createDidHook( hooks ) {
/**
* Returns the number of times an action has been fired.
*
* @param {string} hookName The hook name to check.
*
* @return {number} The number of times the hook has run.
*/
return function didHook( hookName ) {
return hooks.hasOwnProperty( hookName ) && hooks[ hookName ].runs
? hooks[ hookName ].runs
: 0;
};
}

export default createDidHook;
32 changes: 32 additions & 0 deletions packages/hooks/src/createDoingHook.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
/**
* Returns a function which, when invoked, will return whether a hook is
* currently being executed.
*
* @param {Object} hooks Stored hooks, keyed by hook name.
*
* @return {Function} Function that returns whether a hook is currently
* being executed.
*/
function createDoingHook( hooks ) {
/**
* Returns whether a hook is currently being executed.
*
* @param {?string} hookName The name of the hook to check for. If
* omitted, will check for any hook being executed.
*
* @return {bool} Whether the hook is being executed.
*/
return function doingHook( hookName ) {
// If the hookName was not passed, check for any current hook.
if ( 'undefined' === typeof hookName ) {
return 'undefined' !== typeof hooks.__current[0];
}

// Return the __current hook.
return hooks.__current[0]
? hookName === hooks.__current[0].name
: false;
};
}

export default createDoingHook;
26 changes: 26 additions & 0 deletions packages/hooks/src/createHasHook.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
/**
* Returns a function which, when invoked, will return whether any handlers are
* attached to a particular hook.
*
* @param {Object} hooks Stored hooks, keyed by hook name.
*
* @return {Function} Function that returns whether any handlers are
* attached to a particular hook.
*/
function createHasHook( hooks ) {
/**
* Returns how many handlers are attached for the given hook.
*
* @param {string} hookName The name of the hook to check for.
*
* @return {number} The number of handlers that are attached to
* the given hook.
*/
return function hasHook( hookName ) {
return hooks.hasOwnProperty( hookName )
? hooks[ hookName ].handlers.length
: 0;
};
}

export default createHasHook;
66 changes: 66 additions & 0 deletions packages/hooks/src/createRemoveHook.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,66 @@
/**
* Returns a function which, when invoked, will remove a specified hook or all
* hooks by the given name.
*
* @param {Object} hooks Stored hooks, keyed by hook name.
* @param {bool} removeAll Whether to remove all hooked callbacks.
*
* @return {Function} Function that removes hooks.
*/
function createRemoveHook( hooks, removeAll ) {
/**
* Removes the specified callback (or all callbacks) from the hook with a
* given name.
*
* @param {string} hookName The name of the hook to modify.
* @param {?Function} callback The specific callback to be removed. If
* omitted (and `removeAll` is truthy), clears
* all callbacks.
*
* @return {number} The number of callbacks removed.
*/
return function removeHook( hookName, callback ) {
if ( ! removeAll && typeof callback !== 'function' ) {
console.error( 'The hook callback to remove must be a function.' );
return;
}

// Bail if no hooks exist by this name
if ( ! hooks.hasOwnProperty( hookName ) ) {
return 0;
}

let handlersRemoved = 0;

if ( removeAll ) {
handlersRemoved = hooks[ hookName ].handlers.length;
hooks[ hookName ] = {
runs: hooks[ hookName ].runs,
handlers: [],
};
} else {
// Try to find the specified callback to remove.
const handlers = hooks[ hookName ].handlers;
for ( let i = handlers.length - 1; i >= 0; i-- ) {
if ( handlers[ i ].callback === callback ) {
handlers.splice( i, 1 );
handlersRemoved++;
// This callback may also be part of a hook that is
// currently executing. If the callback we're removing
// comes after the current callback, there's no problem;
// otherwise we need to decrease the execution index of any
// other runs by 1 to account for the removed element.
( hooks.__current || [] ).forEach( hookInfo => {
if ( hookInfo.name === hookName && hookInfo.currentIndex >= i ) {
hookInfo.currentIndex--;
}
} );
}
}
}

return handlersRemoved;
};
}

export default createRemoveHook;
80 changes: 80 additions & 0 deletions packages/hooks/src/createRunHook.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,80 @@
/**
* Returns a function which, when invoked, will execute all callbacks
* registered to a hook of the specified type, optionally returning the final
* value of the call chain.
*
* @param {Object} hooks Stored hooks, keyed by hook name.
* @param {?bool} returnFirstArg Whether each hook callback is expected to
* return its first argument.
*
* @return {Function} Function that runs hook callbacks.
*/
function createRunHook( hooks, returnFirstArg ) {
/**
* Runs all callbacks for the specified hook.
*
* @param {string} hookName The name of the hook to run.
* @param {...*} args Arguments to pass to the hook callbacks.
*
* @return {*} Return value of runner, if applicable.
*/
return function runHooks( hookName, ...args ) {
if ( 'string' !== typeof hookName ) {
Copy link
Member

@nylen nylen Aug 8, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The JavaScript coding standards don't recommend Yoda conditions edit: not true, see below. However, the documentation and examples are mostly written in the opposite style.

Requiring === and !== removes the need for yoda conditions anyway, so I wish we could consider that requirement as entirely outdated.

console.error( 'The hook name must be a string.' );
return;
}

if ( /^__/.test( hookName ) ) {
console.error( 'The hook name cannot begin with `__`.' );
return;
}

if ( ! /^[a-z][a-z0-9_]*$/.test( hookName ) ) {
console.error( 'The hook name can only contain numbers, letters and underscores.' );
return;
}

if ( ! hooks.hasOwnProperty( hookName ) ) {
hooks[ hookName ] = {
runs: 0,
handlers: [],
};
}

const handlers = hooks[ hookName ].handlers;

if ( ! handlers.length ) {
return returnFirstArg
? args[ 0 ]
: undefined;
}

const hookInfo = {
name: hookName,
currentIndex: 0,
};

hooks.__current = hooks.__current || [];
hooks.__current.push( hookInfo );
hooks[ hookName ].runs++;

let maybeReturnValue = args[ 0 ];

while ( hookInfo.currentIndex < handlers.length ) {
const handler = handlers[ hookInfo.currentIndex ];
maybeReturnValue = handler.callback.apply( null, args );
if ( returnFirstArg ) {
args[ 0 ] = maybeReturnValue;
}
hookInfo.currentIndex++;
}

hooks.__current.pop();

if ( returnFirstArg ) {
return maybeReturnValue;
}
};
}

export default createRunHook;
Loading