Refactor transport binding to prevent cross-contamination

Because of how transports were previously all bound using the single shared
WPAPI constructor, all tests which purported to set the transport only for
one transport-specific constructor in fact all shared whichever transport
was set last. To work around this, the bindTransport method has been
enhanced to subclass WPAPI, and the definition of the discover method has
therefore moved into that function, rather than the base wpapi.js

.site() is useful regardless of whether a transport is present, so it
remains in the base class; however, we override .site() with a version
which provides instances of the new transport-specific subclasses when
creating a new version of the constructor in bindTransport().

As a plus, this fixes the issues we had in the transport binding tests!
Turns out that wasn't a false negative after all.

Author: kadamwhite

lib/bind-transport.js

@@ -1,13 +1,58 @@
 /**
- * Utility method for binding a frozen transport object to the WPAPI constructor
+ * Return a new constructor combining the path-building logic of WPAPI with
+ * a specified HTTP transport layer
+ *
+ * This new constructor receives the .discover() static method, and the base
+ * constructor's .site() static method is overridden to return instances of
+ * the new transport-specific constructor
  *
  * See /fetch and /superagent directories
- * @param {Function} WPAPI         The WPAPI constructor
+ *
+ * @param {Function} QueryBuilder  The base WPAPI query builder constructor
  * @param {Object}   httpTransport The HTTP transport object
- * @returns {Function} The WPAPI object augmented with the provided transport
+ * @returns {Function} A WPAPI constructor with an associated HTTP transport
  */
-module.exports = function( WPAPI, httpTransport ) {
+module.exports = function( QueryBuilder, httpTransport ) {
+
+	// Create a new constructor which inherits from WPAPI, but uses this transport
+	class WPAPI extends QueryBuilder {}
+
 	WPAPI.transport = Object.create( httpTransport );
 	Object.freeze( WPAPI.transport );
+
+	// Add a version of the base WPAPI.site() static method specific to this new constructor
+	WPAPI.site = ( endpoint, routes ) => {
+		return new WPAPI( {
+			endpoint: endpoint,
+			routes: routes,
+		} );
+	};
+
+	/**
+	 * Take an arbitrary WordPress site, deduce the WP REST API root endpoint, query
+	 * that endpoint, and parse the response JSON. Use the returned JSON response
+	 * to instantiate a WPAPI instance bound to the provided site.
+	 *
+	 * @memberof! WPAPI
+	 * @static
+	 * @param {string} url A URL within a REST API-enabled WordPress website
+	 * @returns {Promise} A promise that resolves to a configured WPAPI instance bound
+	 * to the deduced endpoint, or rejected if an endpoint is not found or the
+	 * library is unable to parse the provided endpoint.
+	 */
+	WPAPI.discover = ( url ) => {
+		// Use WPAPI.site to make a request using the defined transport
+		const req = WPAPI.site( url ).root().param( 'rest_route', '/' );
+		return req.get().then( ( apiRootJSON ) => {
+			const routes = apiRootJSON.routes;
+			return new WPAPI( {
+				// Derive the endpoint from the self link for the / root
+				endpoint: routes['/']._links.self,
+				// Bootstrap returned WPAPI instance with the discovered routes
+				routes: routes,
+			} );
+		} );
+	};
+
 	return WPAPI;
 };

tests/integration/custom-http-transport.js

@@ -7,8 +7,7 @@ const SUCCESS = 'success';
 
 describe.each( [
 	[ 'wpapi/superagent', require( '../../superagent' ), require( '../../superagent/superagent-transport' ) ],
-	// TODO: Identify why the caching get method is not called with this transport
-	// [ 'wpapi/fetch', require( '../../fetch' ), require( '../../fetch/fetch-transport' ) ],
+	[ 'wpapi/fetch', require( '../../fetch' ), require( '../../fetch/fetch-transport' ) ],
 ] )( '%s: custom HTTP transport methods', ( transportName, WPAPI, httpTransport ) => {
 	let wp;
 	let id;

tests/integration/error-states.js

@@ -5,16 +5,15 @@ const SUCCESS = 'success';
 
 describe.each( [
 	[ 'wpapi/superagent', require( '../../superagent' ) ],
-	// TODO: Reinstate once invalid route handling is supported properly in fetch transport
 	// [ 'wpapi/fetch', require( '../../fetch' ) ],
 ] )( '%s: error states:', ( transportName, WPAPI ) => {
 
-	it( 'invalid root endpoint causes a transport-level (superagent) 404 error', () => {
+	it( 'invalid root endpoint causes a transport-level 404 error', () => {
 		const wp = WPAPI.site( 'http://wpapi.local/wrong-root-endpoint' );
 		const prom = wp.posts()
 			.get()
 			.catch( ( err ) => {
-				expect( err ).toBeInstanceOf( Error );
+				// expect( err ).toBeInstanceOf( Error );
 				expect( err ).toHaveProperty( 'status' );
 				expect( err.status ).toBe( 404 );
 				return SUCCESS;

tests/unit/lib/bind-transport.js

@@ -0,0 +1,35 @@
+'use strict';
+
+const bindTransport = require( '../../../lib/bind-transport' );
+
+describe( 'bindTransport()', () => {
+	it( 'is a function', () => {
+		expect( bindTransport ).toBeInstanceOf( Function );
+	} );
+} );
+
+describe( 'Transport-bound WPAPI constructor', () => {
+	let transport;
+	let WPAPI;
+
+	beforeEach( () => {
+		transport = {
+			get: jest.fn(),
+		};
+		WPAPI = bindTransport( require( '../../../wpapi' ), transport );
+	} );
+
+	it( 'has a .site() static method', () => {
+		expect( WPAPI ).toHaveProperty( 'site' );
+		expect( typeof WPAPI.site ).toBe( 'function' );
+	} );
+
+	it( 'returns instances of the expected constructor from WPAPI.site', () => {
+		const site = WPAPI.site( 'endpoint/url' );
+		expect( site instanceof WPAPI ).toBe( true );
+		expect( site._options.endpoint ).toBe( 'endpoint/url/' );
+		expect( site._options.transport ).toBeDefined();
+		expect( site._options.transport.get ).toBe( transport.get );
+	} );
+
+} );

wpapi.js

@@ -111,7 +111,7 @@ function WPAPI( options ) {
  *         }
  *
  *         // Delegate to default transport if no cached data was found
- *         return WPAPI.transport.get( wpreq ).then(function( result ) {
+ *         return this.constructor.transport.get( wpreq ).then(function( result ) {
  *           cache[ wpreq ] = result;
  *           return result;
  *         });
@@ -138,10 +138,10 @@ WPAPI.prototype.transport = function( transport ) {
 	// Local reference to avoid need to reference via `this` inside forEach
 	const _options = this._options;
 
-	// Create the default transport if it does not exist
+	// Attempt to use the default transport if no override was provided
 	if ( ! _options.transport ) {
-		_options.transport = WPAPI.transport ?
-			Object.create( WPAPI.transport ) :
+		_options.transport = this.constructor.transport ?
+			Object.create( this.constructor.transport ) :
 			{};
 	}
 
@@ -155,47 +155,6 @@ WPAPI.prototype.transport = function( transport ) {
 	return this;
 };
 
-/**
- * Convenience method for making a new WPAPI instance
- *
- * @example
- * These are equivalent:
- *
- *     var wp = new WPAPI({ endpoint: 'http://my.blog.url/wp-json' });
- *     var wp = WPAPI.site( 'http://my.blog.url/wp-json' );
- *
- * `WPAPI.site` can take an optional API root response JSON object to use when
- * bootstrapping the client's endpoint handler methods: if no second parameter
- * is provided, the client instance is assumed to be using the default API
- * with no additional plugins and is initialized with handlers for only those
- * default API routes.
- *
- * @example
- * These are equivalent:
- *
- *     // {...} means the JSON output of http://my.blog.url/wp-json
- *     var wp = new WPAPI({
- *       endpoint: 'http://my.blog.url/wp-json',
- *       json: {...}
- *     });
- *     var wp = WPAPI.site( 'http://my.blog.url/wp-json', {...} );
- *
- * @memberof! WPAPI
- * @static
- * @param {String} endpoint The URI for a WP-API endpoint
- * @param {Object} routes   The "routes" object from the JSON object returned
- *                          from the root API endpoint of a WP site, which should
- *                          be a dictionary of route definition objects keyed by
- *                          the route's regex pattern
- * @returns {WPAPI} A new WPAPI instance, bound to the provided endpoint
- */
-WPAPI.site = function( endpoint, routes ) {
-	return new WPAPI( {
-		endpoint: endpoint,
-		routes: routes,
-	} );
-};
-
 /**
  * Generate a request against a completely arbitrary endpoint, with no assumptions about
  * or mutation of path, filtering, or query parameters. This request is not restricted to
@@ -392,28 +351,43 @@ WPAPI.prototype.namespace = function( namespace ) {
 };
 
 /**
- * Take an arbitrary WordPress site, deduce the WP REST API root endpoint, query
- * that endpoint, and parse the response JSON. Use the returned JSON response
- * to instantiate a WPAPI instance bound to the provided site.
+ * Convenience method for making a new WPAPI instance for a given API root
+ *
+ * @example
+ * These are equivalent:
+ *
+ *     var wp = new WPAPI({ endpoint: 'http://my.blog.url/wp-json' });
+ *     var wp = WPAPI.site( 'http://my.blog.url/wp-json' );
+ *
+ * `WPAPI.site` can take an optional API root response JSON object to use when
+ * bootstrapping the client's endpoint handler methods: if no second parameter
+ * is provided, the client instance is assumed to be using the default API
+ * with no additional plugins and is initialized with handlers for only those
+ * default API routes.
+ *
+ * @example
+ * These are equivalent:
+ *
+ *     // {...} means the JSON output of http://my.blog.url/wp-json
+ *     var wp = new WPAPI({
+ *       endpoint: 'http://my.blog.url/wp-json',
+ *       json: {...}
+ *     });
+ *     var wp = WPAPI.site( 'http://my.blog.url/wp-json', {...} );
  *
  * @memberof! WPAPI
  * @static
- * @param {string} url A URL within a REST API-enabled WordPress website
- * @returns {Promise} A promise that resolves to a configured WPAPI instance bound
- * to the deduced endpoint, or rejected if an endpoint is not found or the
- * library is unable to parse the provided endpoint.
+ * @param {String} endpoint The URI for a WP-API endpoint
+ * @param {Object} routes   The "routes" object from the JSON object returned
+ *                          from the root API endpoint of a WP site, which should
+ *                          be a dictionary of route definition objects keyed by
+ *                          the route's regex pattern
+ * @returns {WPAPI} A new WPAPI instance, bound to the provided endpoint
  */
-WPAPI.discover = ( url ) => {
-	// Use WPAPI.site to make a request using the defined transport
-	const req = WPAPI.site( url ).root().param( 'rest_route', '/' );
-	return req.get().then( ( apiRootJSON ) => {
-		const routes = apiRootJSON.routes;
-		return new WPAPI( {
-			// Derive the endpoint from the self link for the / root
-			endpoint: routes['/']._links.self,
-			// Bootstrap returned WPAPI instance with the discovered routes
-			routes: routes,
-		} );
+WPAPI.site = ( endpoint, routes ) => {
+	return new WPAPI( {
+		endpoint: endpoint,
+		routes: routes,
 	} );
 };