Breaking: Remove support for "Node-style" error-first callbacks (#428)

Our original HTTP transport `superagent` does not use Promises by default, so all versions of this library to date have supported Node-style error-first callbacks, _e.g._ `.get( ( err, data ) => { if ( err ) {} ... } )`. However, Promises are the preferred and recommended way of using this library, and the vast majority of library consumers do not utilize the callback method signature.

While evaluating additional transport providers, particularly `fetch`, it became obvious that continuing to support callback-style asynchronous methods would be cumbersome with Promise-aware, modern transport methods like `fetch` or libraries like `axios`. As part of the slimming-down of our API for Version 2, this PR removes support for the node-style callback signature on all transport methods.

* Refactor superagent transport: extract bind-transport method

* Remove error-first callback support from superagent transport

Maintaining this node-style interface, which support threads indicate is
rarely used (especially given the rising popularity of `async`/`await`),
no longer outweights the benefit of providing a consistent interface
regardless of the transport library used.

* Update WPRequest & WPAPI for promise-only method signatures

This completes the work to phase out the "node-style" error-first callbacks

* Update CHANGELOG with breaking transport method signature change

* Add note on callback support removal in v2 upgrade guide

Author: kadamwhite

CHANGELOG.md

@@ -1,7 +1,7 @@
 # Changelog
 
 ## v2.0.0 [**alpha**] _Second Toughest in the Infants_
-
+- **BREAKING**: "Node-style" error-first callbacks (_e.g._ `.get( ( err, data ) => {} )`) are no longer supported. All transport request methods return Promises.
 - **BREAKING**: The module exported as `wpapi` no longer includes HTTP methods. Install `superagent` as a peer dependency and `require( 'wpapi/superagent' )` in order to make HTTP requests.
 - **BREAKING**: Autodiscovery now either succeeds or fails; a WPAPI instance configured with default routes will no longer be returned.
 
@@ -11,13 +11,11 @@
 - Throw an error early when `.file()` is passed a Buffer object without an accompanying name string, props @cungminh2710 & @mvhirsch
 
 
-
 ## v1.2.1 _Colomb_
 
 - Fix issue where `li` was improperly declared as a dev-only dependency, props @el-lsan & @jerolan.
 
 
-
 ## v1.2.0 _Space Is Only Noise_
 
 - **BREAKING**: The minimum supported node version is now v8.6, or v8.2.1 with the `--harmony` flag.

README.md

@@ -78,6 +78,8 @@ At present this browser bundle tracks the `wpapi/superagent` module, and include
 
 ### Upgrading from v1
 
+### Require `wpapi/superagent`
+
 Prior to version 2.0 (currently `alpha` status) this library shipped with built-in HTTP functionality using Superagent.
 
 If you maintain an existing project which uses this library and wish to upgrade to v2, you may do so by manually installing Superagent:
@@ -93,6 +95,21 @@ and then changing your `require` statements to use the `wpapi/superagent` entryp
 +++ const WPAPI = require( 'wpapi/superagent' );
 ```
 
+### Use Promises instead of Callbacks
+
+In version 1, you could use "Node-style" error-first callback functions instead of chaining promises. As of version 2, this callback style is no longer supported; use Promise `.then` syntax or [`async`/`await`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Statements/async_function) instead.
+
+```js
+// Version 1
+wp.posts().get( function( error, posts ) { /* ... */ } );
+
+// Version 2, Promises syntax
+wp.posts().get().then( posts => { /* ... */ } );
+
+// Version 2, await syntax
+await wp.posts().get();
+```
+
 ## Using the Client
 
 The module is a constructor, so you can create an instance of the API client bound to the endpoint for your WordPress install:
@@ -103,23 +120,17 @@ var wp = new WPAPI({ endpoint: 'http://src.wordpress-develop.dev/wp-json' });
 ```
 Once an instance is constructed, you can chain off of it to construct a specific request. (Think of it as a query-builder for WordPress!)
 
-We support requesting posts using either a callback-style or promise-style syntax:
+**Compatibility Note:** As of Version 2.0, Node-style error-first callbacks are no longer supported by this library. All request methods return Promises.
 
 ```javascript
-// Callbacks
-wp.posts().get(function( err, data ) {
-    if ( err ) {
-        // handle err
-    }
-    // do something with the returned posts
-});
-
-// Promises
-wp.posts().then(function( data ) {
-    // do something with the returned posts
-}).catch(function( err ) {
-    // handle error
-});
+// Request methods return Promises.
+wp.posts().get()
+    .then(function( data ) {
+        // do something with the returned posts
+    })
+    .catch(function( err ) {
+        // handle error
+    });
 ```
 The `wp` object has endpoint handler methods for every endpoint that ships with the default WordPress REST API plugin.
 
@@ -798,7 +809,9 @@ By default `node-wpapi` uses the [superagent](https://www.npmjs.com/package/supe
 
 **This is advanced behavior; you will only need to utilize this functionality if your application has very specific HTTP handling or caching requirements.**
 
-In order to maintain consistency with the rest of the API, custom transport methods should take in a WordPress API route handler query object (_e.g._ the result of calling `wp.posts()...` or any of the other chaining resource handlers), a `data` object (for POST, PUT and DELETE requests), and an optional callback function (as `node-wpapi` transport methods both return Promise objects _and_ support traditional `function( err, response )` callbacks).
+In order to maintain consistency with the rest of the API, custom transport methods should take in a WordPress API route handler query object (_e.g._ the result of calling `wp.posts()...` or any of the other chaining resource handlers)and a `data` object (for POST, PUT and DELETE requests).
+
+**Note:** Node-style error-first callbacks are no longer supported by this library as of version 2.0. Custom transport methods should therefore not accept or expect a third optional callback parameter.
 
 The default HTTP transport methods are available as `WPAPI.transport` (a property of the constructor object) and may be called within your transports if you wish to extend the existing behavior, as in the example below.
 
@@ -809,16 +822,11 @@ var site = new WPAPI({
   endpoint: 'http://my-site.com/wp-json',
   transport: {
     // Only override the transport for the GET method, in this example
-    // Transport methods should take a wpreq object and a callback:
-    get: function( wpreq, cb ) {
+    // Transport methods should take a wpreq object:
+    get: function( wpreq ) {
       var result = cache[ wpreq ];
-      // If a cache hit is found, return it via the same callback/promise
-      // signature as the default transport method:
+      // If a cache hit is found, return it wrapped in a Promise:
       if ( result ) {
-        if ( cb && typeof cb === 'function' ) {
-          // Invoke the callback function, if one was provided
-          cb( null, result );
-        }
         // Return the data as a promise
         return Promise.resolve( result );
       }
@@ -837,8 +845,8 @@ You may set one or many custom HTTP transport methods on an existing WP site cli
 
 ```js
 site.transport({
-    get: function( wpreq, callbackFn ) { /* ... */},
-    put: function( wpreq, callbackFn ) { /* ... */}
+    get: function( wpreq ) { /* ... */},
+    put: function( wpreq, data ) { /* ... */}
 });
 ```
 

lib/bind-transport.js

@@ -0,0 +1,13 @@
+/**
+ * Utility method for binding a frozen transport object to the WPAPI constructor
+ *
+ * See /axios and /superagent directories
+ * @param {Function} WPAPI         The WPAPI constructor
+ * @param {Object}   httpTransport The HTTP transport object
+ * @returns {Function} The WPAPI object augmented with the provided transport
+ */
+module.exports = function( WPAPI, httpTransport ) {
+	WPAPI.transport = Object.create( httpTransport );
+	Object.freeze( WPAPI.transport );
+	return WPAPI;
+};

lib/constructors/wp-request.js

@@ -709,23 +709,21 @@ WPRequest.prototype.setHeaders = function( headers, value ) {
  *
  * @method
  * @async
- * @param {Function} [callback] A callback to invoke with the results of the GET request
  * @returns {Promise} A promise to the results of the HTTP request
  */
-WPRequest.prototype.get = function( callback ) {
-	return this.transport.get( this, callback );
+WPRequest.prototype.get = function() {
+	return this.transport.get( this );
 };
 
 /**
  * Get the headers for the specified resource
  *
  * @method
  * @async
- * @param {Function} [callback] A callback to invoke with the results of the HEAD request
  * @returns {Promise} A promise to the header results of the HTTP request
  */
-WPRequest.prototype.headers = function( callback ) {
-	return this.transport.head( this, callback );
+WPRequest.prototype.headers = function() {
+	return this.transport.head( this );
 };
 
 /**
@@ -736,11 +734,10 @@ WPRequest.prototype.headers = function( callback ) {
  * @method
  * @async
  * @param {Object} data The data for the POST request
- * @param {Function} [callback] A callback to invoke with the results of the POST request
  * @returns {Promise} A promise to the results of the HTTP request
  */
-WPRequest.prototype.create = function( data, callback ) {
-	return this.transport.post( this, data, callback );
+WPRequest.prototype.create = function( data ) {
+	return this.transport.post( this, data );
 };
 
 /**
@@ -752,11 +749,10 @@ WPRequest.prototype.create = function( data, callback ) {
  * @async
  * @private
  * @param {Object} data The data for the PUT request
- * @param {Function} [callback] A callback to invoke with the results of the PUT request
  * @returns {Promise} A promise to the results of the HTTP request
  */
-WPRequest.prototype.update = function( data, callback ) {
-	return this.transport.put( this, data, callback );
+WPRequest.prototype.update = function( data ) {
+	return this.transport.put( this, data );
 };
 
 /**
@@ -765,11 +761,10 @@ WPRequest.prototype.update = function( data, callback ) {
  * @method
  * @async
  * @param {Object} [data] Data to send along with the DELETE request
- * @param {Function} [callback] A callback to invoke with the results of the DELETE request
  * @returns {Promise} A promise to the results of the HTTP request
  */
-WPRequest.prototype.delete = function( data, callback ) {
-	return this.transport.delete( this, data, callback );
+WPRequest.prototype.delete = function( data ) {
+	return this.transport.delete( this, data );
 };
 
 /**

package.json

@@ -30,6 +30,7 @@
       "**/tests/**/*.js"
     ],
     "testPathIgnorePatterns": [
+      "test.js",
       ".eslintrc.js",
       "/tests/helpers/"
     ],

superagent/index.js

@@ -1,19 +1,6 @@
 const WPAPI = require( '../wpapi' );
+const superagentTransport = require( './superagent-transport' );
+const bindTransport = require( '../lib/bind-transport' );
 
-// Pull in superagent-based HTTP transport
-const httpTransport = require( './http-transport' );
-
-/**
- * The HTTP transport methods object used by all WPAPI instances
- *
- * These methods may be extended or replaced on an instance-by-instance basis
- *
- * @memberof! WPAPI
- * @static
- * @property transport
- * @type {Object}
- */
-WPAPI.transport = Object.create( httpTransport );
-Object.freeze( WPAPI.transport );
-
-module.exports = WPAPI;
+// Bind the superagent-based HTTP transport to the WPAPI constructor
+module.exports = bindTransport( WPAPI, superagentTransport );

superagent/http-transport.js renamed to superagent/superagent-transport.js

@@ -179,16 +179,15 @@ function createPaginationObject( result, options, httpTransport ) {
 // ====================
 
 /**
- * Submit the provided superagent request object, invoke a callback (if it was
- * provided), and return a promise to the response from the HTTP request.
+ * Submit the provided superagent request object and return a promise which
+ * resolves to the response from the HTTP request.
  *
  * @private
  * @param {Object} request A superagent request object
- * @param {Function} callback A callback function (optional)
  * @param {Function} transform A function to transform the result data
  * @returns {Promise} A promise to the superagent request
  */
-function invokeAndPromisify( request, callback, transform ) {
+function invokeAndPromisify( request, transform ) {
 	return new Promise( ( resolve, reject ) => {
 		// Fire off the result
 		request.end( ( err, result ) => {
@@ -200,14 +199,7 @@ function invokeAndPromisify( request, callback, transform ) {
 				resolve( result );
 			}
 		} );
-	} ).then( transform ).then( ( result ) => {
-		// If a node-style callback was provided, call it, but also return the
-		// result value for use via the returned Promise
-		if ( callback && typeof callback === 'function' ) {
-			callback( null, result );
-		}
-		return result;
-	}, ( err ) => {
+	} ).then( transform ).catch( ( err ) => {
 		// If the API provided an error object, it will be available within the
 		// superagent response object as response.body (containing the response
 		// JSON). If that object exists, it will have a .code property if it is
@@ -217,13 +209,8 @@ function invokeAndPromisify( request, callback, transform ) {
 			// all transport-specific (superagent-specific) properties
 			err = err.response.body;
 		}
-		// If a callback was provided, ensure it is called with the error; otherwise
-		// re-throw the error so that it can be handled by a Promise .catch or .then
-		if ( callback && typeof callback === 'function' ) {
-			callback( err );
-		} else {
-			throw err;
-		}
+		// Re-throw the error so that it can be handled by a Promise .catch or .then
+		throw err;
 	} );
 }
 
@@ -264,17 +251,16 @@ function returnHeaders( result ) {
  * @method get
  * @async
  * @param {WPRequest} wpreq A WPRequest query object
- * @param {Function} [callback] A callback to invoke with the results of the GET request
  * @returns {Promise} A promise to the results of the HTTP request
  */
-function _httpGet( wpreq, callback ) {
+function _httpGet( wpreq ) {
 	checkMethodSupport( 'get', wpreq );
 	const url = wpreq.toString();
 
 	let request = _auth( agent.get( url ), wpreq._options );
 	request = _setHeaders( request, wpreq._options );
 
-	return invokeAndPromisify( request, callback, returnBody.bind( null, wpreq ) );
+	return invokeAndPromisify( request, returnBody.bind( null, wpreq ) );
 }
 
 /**
@@ -283,10 +269,9 @@ function _httpGet( wpreq, callback ) {
  * @async
  * @param {WPRequest} wpreq A WPRequest query object
  * @param {Object} data The data for the POST request
- * @param {Function} [callback] A callback to invoke with the results of the POST request
  * @returns {Promise} A promise to the results of the HTTP request
  */
-function _httpPost( wpreq, data, callback ) {
+function _httpPost( wpreq, data ) {
 	checkMethodSupport( 'post', wpreq );
 	const url = wpreq.toString();
 	data = data || {};
@@ -304,63 +289,56 @@ function _httpPost( wpreq, data, callback ) {
 		request = request.send( data );
 	}
 
-	return invokeAndPromisify( request, callback, returnBody.bind( null, wpreq ) );
+	return invokeAndPromisify( request, returnBody.bind( null, wpreq ) );
 }
 
 /**
  * @method put
  * @async
  * @param {WPRequest} wpreq A WPRequest query object
  * @param {Object} data The data for the PUT request
- * @param {Function} [callback] A callback to invoke with the results of the PUT request
  * @returns {Promise} A promise to the results of the HTTP request
  */
-function _httpPut( wpreq, data, callback ) {
+function _httpPut( wpreq, data ) {
 	checkMethodSupport( 'put', wpreq );
 	const url = wpreq.toString();
 	data = data || {};
 
 	let request = _auth( agent.put( url ), wpreq._options, true ).send( data );
 	request = _setHeaders( request, wpreq._options );
 
-	return invokeAndPromisify( request, callback, returnBody.bind( null, wpreq ) );
+	return invokeAndPromisify( request, returnBody.bind( null, wpreq ) );
 }
 
 /**
  * @method delete
  * @async
  * @param {WPRequest} wpreq A WPRequest query object
  * @param {Object} [data] Data to send along with the DELETE request
- * @param {Function} [callback] A callback to invoke with the results of the DELETE request
  * @returns {Promise} A promise to the results of the HTTP request
  */
-function _httpDelete( wpreq, data, callback ) {
-	if ( ! callback && typeof data === 'function' ) {
-		callback = data;
-		data = null;
-	}
+function _httpDelete( wpreq, data ) {
 	checkMethodSupport( 'delete', wpreq );
 	const url = wpreq.toString();
 	let request = _auth( agent.del( url ), wpreq._options, true ).send( data );
 	request = _setHeaders( request, wpreq._options );
 
-	return invokeAndPromisify( request, callback, returnBody.bind( null, wpreq ) );
+	return invokeAndPromisify( request, returnBody.bind( null, wpreq ) );
 }
 
 /**
  * @method head
  * @async
  * @param {WPRequest} wpreq A WPRequest query object
- * @param {Function} [callback] A callback to invoke with the results of the HEAD request
  * @returns {Promise} A promise to the header results of the HTTP request
  */
-function _httpHead( wpreq, callback ) {
+function _httpHead( wpreq ) {
 	checkMethodSupport( 'head', wpreq );
 	const url = wpreq.toString();
 	let request = _auth( agent.head( url ), wpreq._options );
 	request = _setHeaders( request, wpreq._options );
 
-	return invokeAndPromisify( request, callback, returnHeaders );
+	return invokeAndPromisify( request, returnHeaders );
 }
 
 module.exports = {