From 982c8bab93cb65154781670ff05e8bc277c5ee2b Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 21 Jul 2017 12:49:20 -0400 Subject: [PATCH 1/5] Create withApiData higher-order component --- .../higher-order/with-api-data/index.js | 182 ++++++++++++++++++ .../higher-order/with-api-data/provider.js | 29 +++ .../higher-order/with-api-data/test/index.js | 60 ++++++ components/index.js | 2 + package.json | 1 + 5 files changed, 274 insertions(+) create mode 100644 components/higher-order/with-api-data/index.js create mode 100644 components/higher-order/with-api-data/provider.js create mode 100644 components/higher-order/with-api-data/test/index.js diff --git a/components/higher-order/with-api-data/index.js b/components/higher-order/with-api-data/index.js new file mode 100644 index 00000000000000..f653c13e745ed9 --- /dev/null +++ b/components/higher-order/with-api-data/index.js @@ -0,0 +1,182 @@ +/** + * External dependencies + */ +import balanced from 'balanced-match'; +import { mapValues, noop } from 'lodash'; + +/** + * WordPress dependencies + */ +import { Component } from 'element'; + +export function createEndpointTag( schema ) { + return function( fragments, ...args ) { + // Assign candidates as pairing from route paths, where first entry is + // a working candidate to match fragments against, and the second entry + // the original path value (to reference matched schema route) + let candidates = []; + for ( const path in schema.routes ) { + if ( schema.routes.hasOwnProperty( path ) ) { + candidates.push( [ path, path ] ); + } + } + + let path = ''; + for ( let i = 0; i < fragments.length; i++ ) { + const fragment = fragments[ i ]; + const arg = args[ i ]; + + // Append to working path + path += fragment; + if ( undefined !== arg ) { + path += arg; + } + + candidates = candidates.filter( ( candidate ) => { + let [ working ] = candidate; + + // Reject if does not start with fragment + if ( working.indexOf( fragment ) !== 0 ) { + return false; + } + + working = working.substr( fragment.length ); + + // Test argument for this fragment + if ( undefined !== arg && null !== arg ) { + if ( working[ 0 ] === '(' ) { + // Check if route parameter exists + const param = balanced( '(', ')', working ); + if ( ! param || param.start !== 0 ) { + return false; + } + + working = working.substr( param.end + 1 ); + } else { + // Reject if simple argument and not immediately + // following fragment + if ( working.indexOf( arg ) !== 0 ) { + return false; + } + + working = working.substr( arg.length ); + } + } + + // Update working candidate + candidate[ 0 ] = working; + + return true; + } ); + + if ( ! candidates.length ) { + return; + } + } + + // Return first candidate for which there is no remaining working text + for ( let i = 0; i < candidates.length; i++ ) { + const [ working, original ] = candidates[ i ]; + if ( ! working ) { + return [ path, schema.routes[ original ] ]; + } + } + }; +} + +export default ( mapPropsToData ) => ( WrappedComponent ) => { + class ApiDataComponent extends Component { + constructor() { + super( ...arguments ); + + this.state = {}; + } + + componentWillMount() { + this.isStillMounted = true; + this.applyMapping( this.props ); + } + + componentWillReceiveProps( nextProps ) { + this.applyMapping( nextProps ); + } + + componentWillUnmount() { + this.isStillMounted = false; + } + + setIntoDataProp( propName, values ) { + this.setState( { + ...this.state, + [ propName ]: { + ...this.state[ propName ], + ...values, + }, + } ); + } + + fetchData( propName, path ) { + const url = this.context.getApiRoot() + path.replace( /^\//, '' ); + + this.setIntoDataProp( propName, { isLoading: true } ); + + window.fetch( url, { + credentials: 'include', + // TODO: Nonce management + headers: new window.Headers( { + 'X-WP-Nonce': this.context.getApiNonce(), + } ), + } ).then( ( response ) => response.json() ).then( ( value ) => { + // Don't set state if component unmounted since request started + if ( this.isStillMounted ) { + this.setIntoDataProp( propName, { + isLoading: false, + value, + } ); + } + } ); + } + + applyMapping( props ) { + const schema = this.context.getApiSchema(); + const endpoint = createEndpointTag( schema ); + const mapping = mapPropsToData( props, endpoint ); + const nextState = mapValues( mapping, ( result, propName ) => { + if ( ! result ) { + return; + } + + const [ path, route ] = result; + + return route.methods.reduce( ( stateValue, method ) => { + switch ( method ) { + case 'GET': + stateValue.get = this.fetchData.bind( this, propName, path ); + stateValue.get(); + break; + } + + return stateValue; + }, {} ); + } ); + + this.setState( nextState ); + } + + render() { + return ; + } + } + + // Derive display name from original component + const { displayName = WrappedComponent.name || 'Component' } = WrappedComponent; + ApiDataComponent.displayName = `api(${ displayName })`; + + ApiDataComponent.contextTypes = { + getApiSchema: noop, + getApiRoot: noop, + getApiNonce: noop, + }; + + return ApiDataComponent; +}; diff --git a/components/higher-order/with-api-data/provider.js b/components/higher-order/with-api-data/provider.js new file mode 100644 index 00000000000000..ad186f5aa347af --- /dev/null +++ b/components/higher-order/with-api-data/provider.js @@ -0,0 +1,29 @@ +/** + * External dependencies + */ +import { noop } from 'lodash'; + +/** + * WordPress dependencies + */ +import { Component } from 'element'; + +export default class ApiProvider extends Component { + getChildContext() { + return { + getApiSchema: () => this.props.schema, + getApiRoot: () => this.props.root, + getApiNonce: () => this.props.nonce, + }; + } + + render() { + return this.props.children; + } +} + +ApiProvider.childContextTypes = { + getApiSchema: noop, + getApiRoot: noop, + getApiNonce: noop, +}; diff --git a/components/higher-order/with-api-data/test/index.js b/components/higher-order/with-api-data/test/index.js new file mode 100644 index 00000000000000..fe39cb82ca414d --- /dev/null +++ b/components/higher-order/with-api-data/test/index.js @@ -0,0 +1,60 @@ +/** + * Internal dependencies + */ +import { createEndpointTag } from '../'; + +describe( 'createEndpointTag', () => { + const expected = { base: {}, nested: {} }; + const schema = { + routes: { + '/wp/v2/pages/(?P[\\d]+)/revisions': expected.base, + '/wp/v2/pages/(?P[\\d]+)/revisions/(?P[\d]+)': expected.nested, + }, + }; + + it( 'should match base route with balanced match pattern', () => { + const endpoint = createEndpointTag( schema ); + const match = endpoint`/wp/v2/pages/${ 1 }/revisions`; + + expect( match[ 0 ] ).toBe( '/wp/v2/pages/1/revisions' ); + expect( match[ 1 ] ).toBe( expected.base ); + } ); + + it( 'should match base route with simple fragments', () => { + const endpoint = createEndpointTag( schema ); + const match = endpoint`/wp/v2/${ 'pages' }/${ 1 }/revisions`; + + expect( match[ 0 ] ).toBe( '/wp/v2/pages/1/revisions' ); + expect( match[ 1 ] ).toBe( expected.base ); + } ); + + it( 'should ignore undefined arguments', () => { + const endpoint = createEndpointTag( schema ); + const match = endpoint`/wp/v2/pages/${ undefined }/revisions`; + + expect( match ).toBeUndefined(); + } ); + + it( 'should ignore null arguments', () => { + const endpoint = createEndpointTag( schema ); + const match = endpoint`/wp/v2/pages/${ null }/revisions`; + + expect( match ).toBeUndefined(); + } ); + + it( 'should match nested route with balanced match pattern', () => { + const endpoint = createEndpointTag( schema ); + const match = endpoint`/wp/v2/pages/${ 1 }/revisions/${ 2 }`; + + expect( match[ 0 ] ).toBe( '/wp/v2/pages/1/revisions/2' ); + expect( match[ 1 ] ).toBe( expected.nested ); + } ); + + it( 'should match nested route with simple fragments', () => { + const endpoint = createEndpointTag( schema ); + const match = endpoint`/wp/v2/${ 'pages' }/${ 1 }/revisions/${ 2 }`; + + expect( match[ 0 ] ).toBe( '/wp/v2/pages/1/revisions/2' ); + expect( match[ 1 ] ).toBe( expected.nested ); + } ); +} ); diff --git a/components/index.js b/components/index.js index 90ba593b97fdbf..1718ae02b255e0 100644 --- a/components/index.js +++ b/components/index.js @@ -1,4 +1,5 @@ // Components +export { default as ApiProvider } from './higher-order/with-api-data/provider'; export { default as Button } from './button'; export { default as ClipboardButton } from './clipboard-button'; export { default as Dashicon } from './dashicon'; @@ -26,6 +27,7 @@ export { default as Toolbar } from './toolbar'; export { default as Tooltip } from './tooltip'; // Higher-Order Components +export { default as withApiData } from './higher-order/with-api-data'; export { default as withFocusReturn } from './higher-order/with-focus-return'; export { default as withInstanceId } from './higher-order/with-instance-id'; export { default as withSpokenMessages } from './higher-order/with-spoken-messages'; diff --git a/package.json b/package.json index 99f5bbcb7c4afc..f5723ccfef2766 100644 --- a/package.json +++ b/package.json @@ -12,6 +12,7 @@ ], "dependencies": { "@wordpress/url": "0.1.0-beta.1", + "balanced-match": "^1.0.0", "classnames": "^2.2.5", "clipboard": "^1.7.1", "dom-react": "^2.2.0", From 91baadd28995c8d0950af35ae2e2583c057e5e21 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Fri, 21 Jul 2017 12:49:37 -0400 Subject: [PATCH 2/5] Convert LastRevision to use withApiData --- editor/index.js | 12 ++- editor/sidebar/last-revision/index.js | 131 +++++++------------------- 2 files changed, 47 insertions(+), 96 deletions(-) diff --git a/editor/index.js b/editor/index.js index e10a4d8a671437..6943ccb4ccac5d 100644 --- a/editor/index.js +++ b/editor/index.js @@ -13,7 +13,7 @@ import 'moment-timezone/moment-timezone-utils'; */ import { EditableProvider } from '@wordpress/blocks'; import { createElement, render } from '@wordpress/element'; -import { PopoverProvider } from '@wordpress/components'; +import { ApiProvider, PopoverProvider } from '@wordpress/components'; import { settings as dateSettings } from '@wordpress/date'; /** @@ -119,6 +119,16 @@ export function createEditorInstance( id, post, settings ) { PopoverProvider, { target }, ], + + // ApiProvider + // + // - context.getApiSchema + // - context.getApiRoot + // - context.getApiNonce + [ + ApiProvider, + { ...wpApiSettings }, + ], ]; const createEditorElement = flow( diff --git a/editor/sidebar/last-revision/index.js b/editor/sidebar/last-revision/index.js index bfaf0c89b47274..5ee71796d68e74 100644 --- a/editor/sidebar/last-revision/index.js +++ b/editor/sidebar/last-revision/index.js @@ -2,13 +2,13 @@ * External dependencies */ import { connect } from 'react-redux'; +import { flowRight, last } from 'lodash'; /** * WordPress dependencies */ -import { Component } from '@wordpress/element'; import { sprintf, _n } from '@wordpress/i18n'; -import { IconButton, PanelBody } from '@wordpress/components'; +import { IconButton, PanelBody, withApiData } from '@wordpress/components'; /** * Internal dependencies @@ -22,104 +22,45 @@ import { } from '../../selectors'; import { getWPAdminURL } from '../../utils/url'; -class LastRevision extends Component { - constructor() { - super( ...arguments ); - this.state = { - revisions: [], - loading: false, - }; - } - - componentDidMount() { - this.fetchRevisions(); - } - - componentDidUpdate( prevProps ) { - if ( prevProps.postId !== this.props.postId ) { - this.setState( { revisions: [] } ); - } - - if ( - ( prevProps.postId !== this.props.postId ) || - ( prevProps.isSaving && ! this.props.isSaving ) - ) { - this.fetchRevisions(); - } - } - - componentWillUnmount() { - if ( this.fetchMediaRequest ) { - this.fetchRevisionsRequest.abort(); - } +function LastRevision( { revisions } ) { + const lastRevision = last( revisions.data ); + if ( ! lastRevision ) { + return null; } - fetchRevisions() { - const { isNew, postId, postType } = this.props; - if ( isNew || ! postId ) { - this.setState( { loading: false } ); - return; - } - this.setState( { loading: true } ); - const Collection = wp.api.getPostTypeRevisionsCollection( postType ); - if ( ! Collection ) { - return; - } - this.fetchRevisionsRequest = new Collection( {}, { parent: postId } ).fetch() - .done( ( revisions ) => { - if ( this.props.postId !== postId ) { - return; - } - this.setState( { - loading: false, - revisions, - } ); - } ) - .fail( () => { - if ( this.props.postId !== postId ) { - return; + return ( + + + { + sprintf( + _n( '%d Revision', '%d Revisions', revisions.data.length ), + revisions.data.length + ) } - this.setState( { - loading: false, - } ); - } ); - } - - render() { - const { revisions } = this.state; - - if ( ! revisions.length ) { - return null; - } - - const lastRevision = revisions[ 0 ]; - - return ( - - - { - sprintf( - _n( '%d Revision', '%d Revisions', revisions.length ), - revisions.length - ) - } - - - ); - } + + + ); } -export default connect( - ( state ) => { +export default flowRight( + connect( + ( state ) => { + return { + isNew: isEditedPostNew( state ), + postId: getCurrentPostId( state ), + postType: getCurrentPostType( state ), + isSaving: isSavingPost( state ), + }; + } + ), + withApiData( ( props, { type } ) => { + const { postType, postId } = props; return { - isNew: isEditedPostNew( state ), - postId: getCurrentPostId( state ), - postType: getCurrentPostType( state ), - isSaving: isSavingPost( state ), + revisions: `/wp/v2/${ type( postType ) }/${ postId }/revisions`, }; - } + } ) )( LastRevision ); From f5c03ef11b8cde88ae147824eac79e53f7c31097 Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Sat, 19 Aug 2017 02:57:19 -0400 Subject: [PATCH 3/5] Components: Reimplement withAPIData with plain string paths --- .../higher-order/with-api-data/README.md | 65 ++++ .../higher-order/with-api-data/index.js | 280 ++++++++++-------- .../higher-order/with-api-data/provider.js | 22 +- .../higher-order/with-api-data/request.js | 65 ++++ .../higher-order/with-api-data/routes.js | 54 ++++ .../higher-order/with-api-data/test/index.js | 154 +++++++--- .../with-api-data/test/request.js | 120 ++++++++ .../higher-order/with-api-data/test/routes.js | 84 ++++++ components/index.js | 4 +- editor/index.js | 22 +- editor/sidebar/last-revision/index.js | 4 +- package.json | 2 +- 12 files changed, 693 insertions(+), 183 deletions(-) create mode 100644 components/higher-order/with-api-data/README.md create mode 100644 components/higher-order/with-api-data/request.js create mode 100644 components/higher-order/with-api-data/routes.js create mode 100644 components/higher-order/with-api-data/test/request.js create mode 100644 components/higher-order/with-api-data/test/routes.js diff --git a/components/higher-order/with-api-data/README.md b/components/higher-order/with-api-data/README.md new file mode 100644 index 00000000000000..c51f88b212dac4 --- /dev/null +++ b/components/higher-order/with-api-data/README.md @@ -0,0 +1,65 @@ +# withAPIData + +`withAPIData` is a React [higher-order component](https://facebook.github.io/react/docs/higher-order-components.html) for orechestrating REST API data fetching. Simply compose your component using `withAPIData` and a description of your component's data needs, and the requisite network requests will be performed automatically on your behalf. + +Out of the box, it includes: + +- Auto-fetching when your component mounts +- Reusing cached data if request has already been made +- Provides status updates so you can render accordingly +- Trigger creation, updates, or deletes on data + +## Example: + +Consider a post component which displays a placeholder message while it loads, and the post's title once it becomes available: + +```jsx +function MyPost( { post } ) { + if ( post.isLoading ) { + return
Loading...
; + } + + return
{ post.data.title.rendered }
; +} + +export default withAPIData( ( props, { type } ) => ( { + post: `/wp/v2/${ type( 'post' ) }/${ props.postId }` +} ) )( MyPost ); +``` + +## Usage + +`withAPIData` expects a function argument describing a mapping between prop keys and the REST API endpoint path. In the data mapping function, you have access to the component's incoming props, plus a few REST API helper utilities. It returns a function which can then be used in composing your component. + +The REST API helpers currently include tools to retrieve the `rest_base` of a post type or taxonomy: + +- `type( postType: String ): String` +- `taxonomy( taxonomy: String ): String` + +Data-bound props take the shape of an object with a number of properties, depending on the methods supported for the particular endpoint: + +- `GET` + - `isLoading`: Whether the resource is currently being fetched + - `data`: The resource, available once fetch succeeds + - `get`: Function to invoke a new fetch request + - `error`: The error object, if the fetch failed +- `POST` + - `isCreating`: Whether the resource is currently being created + - `createdData`: The created resource, available once create succeeds + - `create`: Function to invoke a new create request + - `createError`: The error object, if the create failed +- `PUT` + - `isSaving`: Whether the resource is currently being saved + - `savedData`: The saved resource, available once save succeeds + - `save`: Function to invoke a new save request + - `saveError`: The error object, if the save failed +- `PATCH` + - `isPatching`: Whether the resource is currently being patched + - `patchedData`: The patched resource, available once patch succeeds + - `patch`: Function to invoke a new patch request + - `patchError`: The error object, if the patch failed +- `DELETE` + - `isDeleting`: Whether the resource is currently being deleted + - `deletedData`: The deleted resource, available once delete succeeds + - `delete`: Function to invoke a new delete request + - `deleteError`: The error object, if the delete failed diff --git a/components/higher-order/with-api-data/index.js b/components/higher-order/with-api-data/index.js index f653c13e745ed9..979750f4fda012 100644 --- a/components/higher-order/with-api-data/index.js +++ b/components/higher-order/with-api-data/index.js @@ -1,182 +1,212 @@ /** * External dependencies */ -import balanced from 'balanced-match'; -import { mapValues, noop } from 'lodash'; +import { mapValues, reduce, forEach, noop } from 'lodash'; /** * WordPress dependencies */ import { Component } from 'element'; -export function createEndpointTag( schema ) { - return function( fragments, ...args ) { - // Assign candidates as pairing from route paths, where first entry is - // a working candidate to match fragments against, and the second entry - // the original path value (to reference matched schema route) - let candidates = []; - for ( const path in schema.routes ) { - if ( schema.routes.hasOwnProperty( path ) ) { - candidates.push( [ path, path ] ); - } +/** + * Internal dependencies + */ +import request from './request'; +import { getRoute } from './routes'; + +export default ( mapPropsToData ) => ( WrappedComponent ) => { + class APIDataComponent extends Component { + constructor( props, context ) { + super( ...arguments ); + + this.state = { + dataProps: {}, + }; + + this.schema = context.getAPISchema(); + this.routeHelpers = mapValues( { + type: context.getAPIPostTypeRestBaseMapping(), + taxonomy: context.getAPITaxonomyRestBaseMapping(), + }, ( mapping ) => ( key ) => mapping[ key ] ); } - let path = ''; - for ( let i = 0; i < fragments.length; i++ ) { - const fragment = fragments[ i ]; - const arg = args[ i ]; + componentWillMount() { + this.isStillMounted = true; + this.applyMapping( this.props ); + } - // Append to working path - path += fragment; - if ( undefined !== arg ) { - path += arg; - } + componentDidMount() { + this.initializeFetchable( {} ); + } - candidates = candidates.filter( ( candidate ) => { - let [ working ] = candidate; + componentWillReceiveProps( nextProps ) { + this.applyMapping( nextProps ); + } - // Reject if does not start with fragment - if ( working.indexOf( fragment ) !== 0 ) { - return false; - } + componentDidUpdate( prevProps, prevState ) { + this.initializeFetchable( prevState.dataProps ); + } - working = working.substr( fragment.length ); - - // Test argument for this fragment - if ( undefined !== arg && null !== arg ) { - if ( working[ 0 ] === '(' ) { - // Check if route parameter exists - const param = balanced( '(', ')', working ); - if ( ! param || param.start !== 0 ) { - return false; - } - - working = working.substr( param.end + 1 ); - } else { - // Reject if simple argument and not immediately - // following fragment - if ( working.indexOf( arg ) !== 0 ) { - return false; - } - - working = working.substr( arg.length ); - } - } + componentWillUnmount() { + this.isStillMounted = false; + } - // Update working candidate - candidate[ 0 ] = working; + initializeFetchable( prevDataProps ) { + const { dataProps } = this.state; - return true; + // Trigger first fetch on initial entries into state. Assumes GET + // request by presence of isLoading flag. + forEach( dataProps, ( dataProp, propName ) => { + if ( prevDataProps.hasOwnProperty( propName ) ) { + return; + } + + if ( this.getPendingKey( 'GET' ) in dataProp ) { + dataProp[ this.getRequestKey( 'GET' ) ](); + } } ); + } - if ( ! candidates.length ) { + setIntoDataProp( propName, values ) { + if ( ! this.isStillMounted ) { return; } - } - // Return first candidate for which there is no remaining working text - for ( let i = 0; i < candidates.length; i++ ) { - const [ working, original ] = candidates[ i ]; - if ( ! working ) { - return [ path, schema.routes[ original ] ]; - } + this.setState( ( prevState ) => { + const { dataProps } = prevState; + return { + dataProps: { + ...dataProps, + [ propName ]: { + ...dataProps[ propName ], + ...values, + }, + }, + }; + } ); } - }; -} - -export default ( mapPropsToData ) => ( WrappedComponent ) => { - class ApiDataComponent extends Component { - constructor() { - super( ...arguments ); - this.state = {}; + getRequestKey( method ) { + switch ( method ) { + case 'GET': return 'get'; + case 'POST': return 'create'; + case 'PUT': return 'save'; + case 'PATCH': return 'patch'; + case 'DELETE': return 'delete'; + } } - componentWillMount() { - this.isStillMounted = true; - this.applyMapping( this.props ); + getPendingKey( method ) { + switch ( method ) { + case 'GET': return 'isLoading'; + case 'POST': return 'isCreating'; + case 'PUT': return 'isSaving'; + case 'PATCH': return 'isPatching'; + case 'DELETE': return 'isDeleting'; + } } - componentWillReceiveProps( nextProps ) { - this.applyMapping( nextProps ); + getResponseDataKey( method ) { + switch ( method ) { + case 'GET': return 'data'; + case 'POST': return 'createdData'; + case 'PUT': return 'savedData'; + case 'PATCH': return 'patchedData'; + case 'DELETE': return 'deletedData'; + } } - componentWillUnmount() { - this.isStillMounted = false; + getErrorResponseKey( method ) { + switch ( method ) { + case 'GET': return 'error'; + case 'POST': return 'createError'; + case 'PUT': return 'saveError'; + case 'PATCH': return 'patchError'; + case 'DELETE': return 'deleteError'; + } } - setIntoDataProp( propName, values ) { - this.setState( { - ...this.state, - [ propName ]: { - ...this.state[ propName ], - ...values, - }, + request( propName, method, path ) { + this.setIntoDataProp( propName, { + [ this.getPendingKey( method ) ]: true, } ); - } - fetchData( propName, path ) { - const url = this.context.getApiRoot() + path.replace( /^\//, '' ); - - this.setIntoDataProp( propName, { isLoading: true } ); - - window.fetch( url, { - credentials: 'include', - // TODO: Nonce management - headers: new window.Headers( { - 'X-WP-Nonce': this.context.getApiNonce(), - } ), - } ).then( ( response ) => response.json() ).then( ( value ) => { - // Don't set state if component unmounted since request started - if ( this.isStillMounted ) { - this.setIntoDataProp( propName, { - isLoading: false, - value, - } ); - } + request( { path, method } ).then( ( data ) => { + this.setIntoDataProp( propName, { + [ this.getPendingKey( method ) ]: false, + [ this.getResponseDataKey( method ) ]: data, + } ); + } ).catch( ( error ) => { + this.setIntoDataProp( propName, { + [ this.getErrorResponseKey( method ) ]: error, + } ); } ); } applyMapping( props ) { - const schema = this.context.getApiSchema(); - const endpoint = createEndpointTag( schema ); - const mapping = mapPropsToData( props, endpoint ); - const nextState = mapValues( mapping, ( result, propName ) => { - if ( ! result ) { - return; + const { dataProps } = this.state; + + const mapping = mapPropsToData( props, this.routeHelpers ); + const nextDataProps = reduce( mapping, ( result, path, propName ) => { + // Skip if mapping already assigned into state data props + // Exmaple: Component updates with one new prop and other + // previously existing; previously existing should not be + // clobbered or re-trigger fetch + const dataProp = dataProps[ propName ]; + if ( dataProp && dataProp.path === path ) { + result[ propName ] = dataProp; + return result; + } + + const route = getRoute( this.schema, path ); + if ( ! route ) { + return result; } - const [ path, route ] = result; + result[ propName ] = route.methods.reduce( ( stateValue, method ) => { + // Add request initiater into data props + const requestKey = this.getRequestKey( method ); + stateValue[ requestKey ] = this.request.bind( + this, + propName, + method, + path + ); - return route.methods.reduce( ( stateValue, method ) => { - switch ( method ) { - case 'GET': - stateValue.get = this.fetchData.bind( this, propName, path ); - stateValue.get(); - break; - } + // Initialize pending flags as explicitly false + const pendingKey = this.getPendingKey( method ); + stateValue[ pendingKey ] = false; + + // Track path for future map skipping + stateValue.path = path; return stateValue; }, {} ); - } ); - this.setState( nextState ); + return result; + }, {} ); + + this.setState( () => ( { dataProps: nextDataProps } ) ); } render() { - return ; + return ( + + ); } } // Derive display name from original component const { displayName = WrappedComponent.name || 'Component' } = WrappedComponent; - ApiDataComponent.displayName = `api(${ displayName })`; + APIDataComponent.displayName = `apiData(${ displayName })`; - ApiDataComponent.contextTypes = { - getApiSchema: noop, - getApiRoot: noop, - getApiNonce: noop, + APIDataComponent.contextTypes = { + getAPISchema: noop, + getAPIPostTypeRestBaseMapping: noop, + getAPITaxonomyRestBaseMapping: noop, }; - return ApiDataComponent; + return APIDataComponent; }; diff --git a/components/higher-order/with-api-data/provider.js b/components/higher-order/with-api-data/provider.js index ad186f5aa347af..ba43a7f4b59ae4 100644 --- a/components/higher-order/with-api-data/provider.js +++ b/components/higher-order/with-api-data/provider.js @@ -1,20 +1,20 @@ /** * External dependencies */ -import { noop } from 'lodash'; +import { mapValues, noop } from 'lodash'; /** * WordPress dependencies */ import { Component } from 'element'; -export default class ApiProvider extends Component { +export default class APIProvider extends Component { getChildContext() { - return { - getApiSchema: () => this.props.schema, - getApiRoot: () => this.props.root, - getApiNonce: () => this.props.nonce, - }; + return mapValues( { + getAPISchema: 'schema', + getAPIPostTypeRestBaseMapping: 'postTypeRestBaseMapping', + getAPITaxonomyRestBaseMapping: 'taxonomyRestBaseMapping', + }, ( key ) => () => this.props[ key ] ); } render() { @@ -22,8 +22,8 @@ export default class ApiProvider extends Component { } } -ApiProvider.childContextTypes = { - getApiSchema: noop, - getApiRoot: noop, - getApiNonce: noop, +APIProvider.childContextTypes = { + getAPISchema: noop, + getAPIPostTypeRestBaseMapping: noop, + getAPITaxonomyRestBaseMapping: noop, }; diff --git a/components/higher-order/with-api-data/request.js b/components/higher-order/with-api-data/request.js new file mode 100644 index 00000000000000..8929d087f76651 --- /dev/null +++ b/components/higher-order/with-api-data/request.js @@ -0,0 +1,65 @@ +/** + * External dependencies + */ +import memoize from 'memize'; + +export const cache = {}; + +export const getStablePath = memoize( ( path ) => { + const [ base, query ] = path.split( '?' ); + if ( ! query ) { + return base; + } + + return base + '?' + query + // 'b=1&c=2&a=5' + + .split( '&' ) + // [ 'b=1', 'c=2', 'a=5' ] + + .map( ( entry ) => entry.split( '=' ) ) + // [ [ 'b, '1' ], [ 'c', '2' ], [ 'a', '5' ] ] + + .sort( ( a, b ) => a[ 0 ].localeCompare( b[ 0 ] ) ) + // [ [ 'a', '5' ], [ 'b, '1' ], [ 'c', '2' ] ] + + .map( ( pair ) => pair.join( '=' ) ) + // [ 'a=5', 'b=1', 'c=2' ] + + .join( '&' ); + // 'a=5&b=1&c=2' +} ); + +export function getResponseFromCache( request ) { + const response = cache[ getStablePath( request.path ) ]; + return Promise.resolve( response ); +} + +export function getResponseFromNetwork( request ) { + const promise = wp.apiRequest( request ).promise(); + + if ( isRequestMethod( request, 'GET' ) ) { + promise.then( ( data ) => { + cache[ getStablePath( request.path ) ] = data; + } ); + } + + return promise; +} + +export function isRequestMethod( request, method ) { + return request.method === method; +} + +export default function( request ) { + if ( ! isRequestMethod( request, 'GET' ) ) { + return getResponseFromNetwork( request ); + } + + return getResponseFromCache( request ) + .then( ( response ) => ( + undefined === response + ? getResponseFromNetwork( request ) + : response + ) ); +} diff --git a/components/higher-order/with-api-data/routes.js b/components/higher-order/with-api-data/routes.js new file mode 100644 index 00000000000000..752cb54ad68f2a --- /dev/null +++ b/components/higher-order/with-api-data/routes.js @@ -0,0 +1,54 @@ +/** + * External dependencies + */ +import { find } from 'lodash'; +import createSelector from 'rememo'; + +/** + * Match PCRE named subpatterns in a string. This implementation is not strict + * on balanced delimiters, but assumes this would be a broken pattern anyways. + * + * See: http://www.pcre.org/original/doc/html/pcrepattern.html#SEC16 + * See: http://php.net/manual/en/function.preg-match.php + * + * @type {RegExp} + */ +const RE_NAMED_SUBPATTERN = /\(\?P?[<']\w+[>'](.*?)\)/g; + +/** + * Coerces a REST route pattern to an equivalent JavaScript regular expression, + * replacing named subpatterns (unsupported in JavaScript), allowing trailing + * slash, allowing query parameters, but otherwise enforcing strict equality. + * + * @param {String} pattern PCRE regular expression string + * @return {RegExp} Equivalent JavaScript RegExp + */ +export function getNormalizedRegExp( pattern ) { + pattern = pattern.replace( RE_NAMED_SUBPATTERN, '($1)' ); + pattern = '^' + pattern + '/?(\\?.*)?$'; + return new RegExp( pattern ); +} + +/** + * Returns true if the route path pattern string matches the given path. + * + * @param {String} pattern PCRE route path pattern + * @param {String} path URL path + * @return {Boolean} Whether path is a match + */ +export function isRouteMatch( pattern, path ) { + return getNormalizedRegExp( pattern ).test( path ); +} + +/** + * Returns a REST route object for a given path, if one exists. + * + * @param {Object} schema REST schema + * @param {String} path URL path + * @return {?Object} REST route + */ +export const getRoute = createSelector( ( schema, path ) => { + return find( schema.routes, ( route, pattern ) => { + return isRouteMatch( pattern, path ); + } ); +} ); diff --git a/components/higher-order/with-api-data/test/index.js b/components/higher-order/with-api-data/test/index.js index fe39cb82ca414d..0738bd045c4e07 100644 --- a/components/higher-order/with-api-data/test/index.js +++ b/components/higher-order/with-api-data/test/index.js @@ -1,60 +1,146 @@ +/** + * External dependencies + */ +import { shallow } from 'enzyme'; +import { identity, fromPairs } from 'lodash'; + /** * Internal dependencies */ -import { createEndpointTag } from '../'; +import withAPIData from '../'; -describe( 'createEndpointTag', () => { - const expected = { base: {}, nested: {} }; +jest.mock( '../request', () => jest.fn( () => Promise.resolve( {} ) ) ); + +describe( 'withAPIData()', () => { const schema = { routes: { - '/wp/v2/pages/(?P[\\d]+)/revisions': expected.base, - '/wp/v2/pages/(?P[\\d]+)/revisions/(?P[\d]+)': expected.nested, + '/wp/v2/pages/(?P[\\d]+)/revisions': { + methods: [ 'GET' ], + }, + '/wp/v2/pages/(?P[\\d]+)': { + methods: [ + 'GET', + 'POST', + 'PUT', + 'PATCH', + 'DELETE', + ], + }, }, }; - it( 'should match base route with balanced match pattern', () => { - const endpoint = createEndpointTag( schema ); - const match = endpoint`/wp/v2/pages/${ 1 }/revisions`; + function getWrapper( mapPropsToData, props ) { + if ( ! mapPropsToData ) { + mapPropsToData = () => ( { + revisions: '/wp/v2/pages/5/revisions', + } ); + } - expect( match[ 0 ] ).toBe( '/wp/v2/pages/1/revisions' ); - expect( match[ 1 ] ).toBe( expected.base ); - } ); + const Component = withAPIData( mapPropsToData )( () => '' ); + + return shallow( , { + lifecycleExperimental: true, + context: { + getAPISchema: () => schema, + getAPIPostTypeRestBaseMapping: identity, + getAPITaxonomyRestBaseMapping: identity, + }, + } ); + } - it( 'should match base route with simple fragments', () => { - const endpoint = createEndpointTag( schema ); - const match = endpoint`/wp/v2/${ 'pages' }/${ 1 }/revisions`; + it( 'should initialize fetchables on mount', ( done ) => { + const wrapper = getWrapper(); - expect( match[ 0 ] ).toBe( '/wp/v2/pages/1/revisions' ); - expect( match[ 1 ] ).toBe( expected.base ); + const dataProps = wrapper.state( 'dataProps' ); + expect( Object.keys( dataProps ) ).toEqual( [ 'revisions' ] ); + expect( Object.keys( dataProps.revisions ) ).toEqual( [ + 'get', + 'isLoading', + 'path', + ] ); + expect( dataProps.revisions.isLoading ).toBe( true ); + + process.nextTick( () => { + expect( wrapper.state( 'dataProps' ).revisions.isLoading ).toBe( false ); + expect( wrapper.state( 'dataProps' ).revisions.data ).toEqual( {} ); + + done(); + } ); } ); - it( 'should ignore undefined arguments', () => { - const endpoint = createEndpointTag( schema ); - const match = endpoint`/wp/v2/pages/${ undefined }/revisions`; + it( 'should ignore unmatched resources', () => { + const wrapper = getWrapper( () => ( { + revision: '/wp/v2/pages/5/revisions/10', + } ) ); - expect( match ).toBeUndefined(); + expect( wrapper.state( 'dataProps' ) ).toEqual( {} ); } ); - it( 'should ignore null arguments', () => { - const endpoint = createEndpointTag( schema ); - const match = endpoint`/wp/v2/pages/${ null }/revisions`; + it( 'should include full gamut of method available properties', () => { + const wrapper = getWrapper( () => ( { + page: '/wp/v2/pages/5', + } ) ); - expect( match ).toBeUndefined(); + const dataProps = wrapper.state( 'dataProps' ); + expect( Object.keys( dataProps ) ).toEqual( [ 'page' ] ); + expect( Object.keys( dataProps.page ) ).toEqual( [ + 'get', + 'isLoading', + 'path', + 'create', + 'isCreating', + 'save', + 'isSaving', + 'patch', + 'isPatching', + 'delete', + 'isDeleting', + ] ); + expect( dataProps.page.isLoading ).toBe( true ); + expect( dataProps.page.isCreating ).toBe( false ); + expect( dataProps.page.isSaving ).toBe( false ); + expect( dataProps.page.isPatching ).toBe( false ); + expect( dataProps.page.isDeleting ).toBe( false ); } ); - it( 'should match nested route with balanced match pattern', () => { - const endpoint = createEndpointTag( schema ); - const match = endpoint`/wp/v2/pages/${ 1 }/revisions/${ 2 }`; + it( 'should not clobber existing data when receiving new props', ( done ) => { + const wrapper = getWrapper( + ( { pages } ) => fromPairs( pages.map( ( page ) => [ + 'page' + page, + '/wp/v2/pages/' + page, + ] ) ), + { pages: [ 1 ] } + ); + + process.nextTick( () => { + wrapper.setProps( { pages: [ 1, 2 ] } ); - expect( match[ 0 ] ).toBe( '/wp/v2/pages/1/revisions/2' ); - expect( match[ 1 ] ).toBe( expected.nested ); + const dataProps = wrapper.state( 'dataProps' ); + expect( dataProps.page1.isLoading ).toBe( false ); + expect( dataProps.page1.data ).toEqual( {} ); + expect( dataProps.page2.isLoading ).toBe( true ); + + done(); + } ); } ); - it( 'should match nested route with simple fragments', () => { - const endpoint = createEndpointTag( schema ); - const match = endpoint`/wp/v2/${ 'pages' }/${ 1 }/revisions/${ 2 }`; + it( 'should remove dropped mappings', ( done ) => { + const wrapper = getWrapper( + ( { pages } ) => fromPairs( pages.map( ( page ) => [ + 'page' + page, + '/wp/v2/pages/' + page, + ] ) ), + { pages: [ 1 ] } + ); + + process.nextTick( () => { + wrapper.setProps( { pages: [ 2 ] } ); + + const dataProps = wrapper.state( 'dataProps' ); + expect( dataProps ).not.toHaveProperty( 'page1' ); + expect( dataProps ).toHaveProperty( 'page2' ); - expect( match[ 0 ] ).toBe( '/wp/v2/pages/1/revisions/2' ); - expect( match[ 1 ] ).toBe( expected.nested ); + done(); + } ); } ); } ); diff --git a/components/higher-order/with-api-data/test/request.js b/components/higher-order/with-api-data/test/request.js new file mode 100644 index 00000000000000..8d9ba1dbeb86e3 --- /dev/null +++ b/components/higher-order/with-api-data/test/request.js @@ -0,0 +1,120 @@ +/** + * Internal dependencies + */ +import request, { + cache, + getStablePath, + getResponseFromCache, + getResponseFromNetwork, + isRequestMethod, +} from '../request'; + +describe( 'request', () => { + const actualResponse = {}; + + let wpApiRequest; + beforeEach( () => { + getStablePath.clear(); + for ( const key in cache ) { + delete cache[ key ]; + } + + wpApiRequest = wp.apiRequest; + wp.apiRequest = jest.fn( () => ( { + promise: () => Promise.resolve( actualResponse ), + } ) ); + } ); + + afterEach( () => { + wp.apiRequest = wpApiRequest; + } ); + + describe( 'getResponseFromCache()', () => { + it( 'returns response from cache', () => { + cache[ getStablePath( '/wp?c=5&a=5&b=5' ) ] = actualResponse; + const awaitResponse = getResponseFromCache( { + path: '/wp?b=5&c=5&a=5', + } ); + + expect( awaitResponse ).resolves.toBe( actualResponse ); + } ); + } ); + + describe( 'getResponseFromNetwork()', () => { + it( 'triggers a network request', () => { + const awaitResponse = getResponseFromNetwork( { + path: '/wp?b=5&c=5&a=5', + } ); + + return awaitResponse.then( ( data ) => { + expect( wp.apiRequest ).toHaveBeenCalled(); + expect( data ).toBe( actualResponse ); + } ); + } ); + } ); + + describe( 'getStablePath()', () => { + it( 'should return a path without query arguments', () => { + const path = '/wp'; + + expect( getStablePath( path ) ).toBe( path ); + } ); + + it( 'should return a path with sorted query arguments', () => { + const a = getStablePath( '/wp?c=5&a=5&b=5' ); + const b = getStablePath( '/wp?b=5&c=5&a=5' ); + + expect( a ).toBe( b ); + } ); + } ); + + describe( 'isRequestMethod()', () => { + it( 'returns false if not method', () => { + expect( isRequestMethod( { method: 'POST' }, 'GET' ) ).toBe( false ); + } ); + + it( 'returns true if method', () => { + expect( isRequestMethod( { method: 'GET' }, 'GET' ) ).toBe( true ); + } ); + } ); + + describe( 'request()', () => { + it( 'should try from cache for GET', () => { + cache[ getStablePath( '/wp?c=5&a=5&b=5' ) ] = actualResponse; + const awaitResponse = request( { + path: '/wp?b=5&c=5&a=5', + method: 'GET', + } ); + + return awaitResponse.then( ( data ) => { + expect( wp.apiRequest ).not.toHaveBeenCalled(); + expect( data ).toBe( actualResponse ); + } ); + } ); + + it( 'should not try from cache for non-GET', () => { + cache[ getStablePath( '/wp?c=5&a=5&b=5' ) ] = actualResponse; + const awaitResponse = request( { + path: '/wp?b=5&c=5&a=5', + method: 'POST', + } ); + + return awaitResponse.then( ( data ) => { + expect( wp.apiRequest ).toHaveBeenCalled(); + expect( data ).toBe( actualResponse ); + } ); + } ); + + it( 'should fall back to network', () => { + const awaitResponse = request( { + path: '/wp?b=5&c=5&a=5', + method: 'GET', + } ); + + return awaitResponse.then( ( data ) => { + expect( wp.apiRequest ).toHaveBeenCalled(); + expect( data ).toBe( actualResponse ); + } ); + } ); + } ); +} ); diff --git a/components/higher-order/with-api-data/test/routes.js b/components/higher-order/with-api-data/test/routes.js new file mode 100644 index 00000000000000..994575e2035dc4 --- /dev/null +++ b/components/higher-order/with-api-data/test/routes.js @@ -0,0 +1,84 @@ +/** + * Internal dependencies + */ +import { + isRouteMatch, + getRoute, + getNormalizedRegExp, +} from '../routes'; + +describe( 'routes', () => { + describe( 'getNormalizedRegExp()', () => { + it( 'should strip named subpatterns', () => { + const regexp = getNormalizedRegExp( '(?P[\\d]+)' ); + + expect( '5' ).toMatch( regexp ); + } ); + + it( 'should match with trailing slashes', () => { + const regexp = getNormalizedRegExp( '/wp' ); + + expect( '/wp/' ).toMatch( regexp ); + } ); + + it( 'should match with query parameters slashes', () => { + const regexp = getNormalizedRegExp( '/wp' ); + + expect( '/wp?ok=true' ).toMatch( regexp ); + } ); + + it( 'should otherwise be strictly equal', () => { + const regexp = getNormalizedRegExp( '/wp' ); + + expect( '/wp/notok' ).not.toMatch( regexp ); + } ); + } ); + + describe( 'isRouteMatch()', () => { + it( 'should return false for non-match', () => { + const isMatch = isRouteMatch( + '/wp/v2/pages/(?P[\\d]+)/revisions', + '/wp/v2/pages/1/revisions/2' + ); + + expect( isMatch ).toBe( false ); + } ); + + it( 'should return true for match', () => { + const isMatch = isRouteMatch( + '/wp/v2/pages/(?P[\\d]+)/revisions/(?P[\\d]+)', + '/wp/v2/pages/1/revisions/2' + ); + + expect( isMatch ).toBe( true ); + } ); + } ); + + describe( 'getRoute()', () => { + const expected = { base: {}, nested: {} }; + const schema = { + routes: { + '/wp/v2/pages/(?P[\\d]+)/revisions': expected.base, + '/wp/v2/pages/(?P[\\d]+)/revisions/(?P[\\d]+)': expected.nested, + }, + }; + + beforeEach( () => { + getRoute.clear(); + } ); + + it( 'should match base route with balanced match pattern', () => { + const path = '/wp/v2/pages/1/revisions'; + const route = getRoute( schema, path ); + + expect( route ).toBe( expected.base ); + } ); + + it( 'should match nested route with balanced match pattern', () => { + const path = '/wp/v2/pages/1/revisions/2'; + const route = getRoute( schema, path ); + + expect( route ).toBe( expected.nested ); + } ); + } ); +} ); diff --git a/components/index.js b/components/index.js index 1718ae02b255e0..d3c1a6ecbd01ba 100644 --- a/components/index.js +++ b/components/index.js @@ -1,5 +1,5 @@ // Components -export { default as ApiProvider } from './higher-order/with-api-data/provider'; +export { default as APIProvider } from './higher-order/with-api-data/provider'; export { default as Button } from './button'; export { default as ClipboardButton } from './clipboard-button'; export { default as Dashicon } from './dashicon'; @@ -27,7 +27,7 @@ export { default as Toolbar } from './toolbar'; export { default as Tooltip } from './tooltip'; // Higher-Order Components -export { default as withApiData } from './higher-order/with-api-data'; +export { default as withAPIData } from './higher-order/with-api-data'; export { default as withFocusReturn } from './higher-order/with-focus-return'; export { default as withInstanceId } from './higher-order/with-instance-id'; export { default as withSpokenMessages } from './higher-order/with-spoken-messages'; diff --git a/editor/index.js b/editor/index.js index 6943ccb4ccac5d..4e297aa0d8b6ce 100644 --- a/editor/index.js +++ b/editor/index.js @@ -4,7 +4,7 @@ import { bindActionCreators } from 'redux'; import { Provider as ReduxProvider } from 'react-redux'; import { Provider as SlotFillProvider } from 'react-slot-fill'; -import { flow } from 'lodash'; +import { flow, pick } from 'lodash'; import moment from 'moment-timezone'; import 'moment-timezone/moment-timezone-utils'; @@ -13,7 +13,7 @@ import 'moment-timezone/moment-timezone-utils'; */ import { EditableProvider } from '@wordpress/blocks'; import { createElement, render } from '@wordpress/element'; -import { ApiProvider, PopoverProvider } from '@wordpress/components'; +import { APIProvider, PopoverProvider } from '@wordpress/components'; import { settings as dateSettings } from '@wordpress/date'; /** @@ -120,14 +120,20 @@ export function createEditorInstance( id, post, settings ) { { target }, ], - // ApiProvider + // APIProvider // - // - context.getApiSchema - // - context.getApiRoot - // - context.getApiNonce + // - context.getAPISchema + // - context.getAPIPostTypeRestBaseMapping + // - context.getAPITaxonomyRestBaseMapping [ - ApiProvider, - { ...wpApiSettings }, + APIProvider, + { + ...wpApiSettings, + ...pick( wp.api, [ + 'postTypeRestBaseMapping', + 'taxonomyRestBaseMapping', + ] ), + }, ], ]; diff --git a/editor/sidebar/last-revision/index.js b/editor/sidebar/last-revision/index.js index 5ee71796d68e74..bc0f0c34960c1f 100644 --- a/editor/sidebar/last-revision/index.js +++ b/editor/sidebar/last-revision/index.js @@ -8,7 +8,7 @@ import { flowRight, last } from 'lodash'; * WordPress dependencies */ import { sprintf, _n } from '@wordpress/i18n'; -import { IconButton, PanelBody, withApiData } from '@wordpress/components'; +import { IconButton, PanelBody, withAPIData } from '@wordpress/components'; /** * Internal dependencies @@ -57,7 +57,7 @@ export default flowRight( }; } ), - withApiData( ( props, { type } ) => { + withAPIData( ( props, { type } ) => { const { postType, postId } = props; return { revisions: `/wp/v2/${ type( postType ) }/${ postId }/revisions`, diff --git a/package.json b/package.json index f5723ccfef2766..6053a05db558ed 100644 --- a/package.json +++ b/package.json @@ -12,7 +12,6 @@ ], "dependencies": { "@wordpress/url": "0.1.0-beta.1", - "balanced-match": "^1.0.0", "classnames": "^2.2.5", "clipboard": "^1.7.1", "dom-react": "^2.2.0", @@ -22,6 +21,7 @@ "jed": "^1.1.1", "js-beautify": "^1.6.12", "lodash": "^4.17.4", + "memize": "^1.0.1", "moment": "^2.18.1", "moment-timezone": "^0.5.13", "mousetrap": "^1.6.1", From c37a1cc6d9c39fa314e395050b3e14369277752d Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Mon, 21 Aug 2017 15:04:32 -0400 Subject: [PATCH 4/5] Compat: Shim wp-api-request for installations pre-4.9 --- lib/client-assets.php | 2 +- lib/compat.php | 28 ++++++++++++++++++++++++++++ 2 files changed, 29 insertions(+), 1 deletion(-) diff --git a/lib/client-assets.php b/lib/client-assets.php index 47cdd0c3489973..b9a9765471d19a 100644 --- a/lib/client-assets.php +++ b/lib/client-assets.php @@ -129,7 +129,7 @@ function gutenberg_register_scripts_and_styles() { wp_register_script( 'wp-components', gutenberg_url( 'components/build/index.js' ), - array( 'wp-element', 'wp-a11y', 'wp-i18n', 'wp-utils' ), + array( 'wp-element', 'wp-a11y', 'wp-i18n', 'wp-utils', 'wp-api-request' ), filemtime( gutenberg_dir_path() . 'components/build/index.js' ) ); wp_register_script( diff --git a/lib/compat.php b/lib/compat.php index a0a9fe8aab7fe1..b9ace4f135a456 100644 --- a/lib/compat.php +++ b/lib/compat.php @@ -87,3 +87,31 @@ function gutenberg_fix_jetpack_freeform_block_conflict() { ); } } + +/** + * Shims wp-api-request for WordPress installations not running 4.9-alpha or + * newer. + * + * @see https://core.trac.wordpress.org/ticket/40919 + * + * @since 0.10.0 + */ +function gutenberg_ensure_wp_api_request() { + if ( wp_script_is( 'wp-api-request', 'registered' ) ) { + return; + } + + gutenberg_register_vendor_script( + 'wp-api-request', + 'https://rawgit.com/WordPress/wordpress-develop/master/src/wp-includes/js/api-request.js' + ); + + // Localize wp-api-request using wp-api handle data (swapped in 4.9-alpha). + global $wp_scripts; + $wp_api_localized_data = $wp_scripts->get_data( 'wp-api', 'data' ); + if ( false !== $wp_api_localized_data ) { + wp_add_inline_script( 'wp-api-request', $wp_api_localized_data, 'before' ); + } +} +add_action( 'wp_enqueue_scripts', 'gutenberg_ensure_wp_api_request' ); +add_action( 'admin_enqueue_scripts', 'gutenberg_ensure_wp_api_request' ); From 0b98816f02c9eaea3d4371cc52cec0480635780f Mon Sep 17 00:00:00 2001 From: Andrew Duthie Date: Tue, 22 Aug 2017 10:54:38 -0400 Subject: [PATCH 5/5] Scripts: Move shim registration into client-assets The plugin bundler will not pick up on scripts registered outside this file, and it is simpler to add here than enhance bundler to force reigstration of the shim from compat --- lib/client-assets.php | 9 +++++++++ lib/compat.php | 19 +++++++++++++------ 2 files changed, 22 insertions(+), 6 deletions(-) diff --git a/lib/client-assets.php b/lib/client-assets.php index b9a9765471d19a..6074277950b385 100644 --- a/lib/client-assets.php +++ b/lib/client-assets.php @@ -230,6 +230,15 @@ function gutenberg_register_vendor_scripts() { 'promise', 'https://unpkg.com/promise-polyfill/promise' . $suffix . '.js' ); + + // TODO: This is only necessary so long as WordPress 4.9 is not yet stable, + // since we depend on the newly-introduced wp-api-request script handle. + // + // See: gutenberg_ensure_wp_api_request (compat.php). + gutenberg_register_vendor_script( + 'wp-api-request-shim', + 'https://rawgit.com/WordPress/wordpress-develop/master/src/wp-includes/js/api-request.js' + ); } /** diff --git a/lib/compat.php b/lib/compat.php index b9ace4f135a456..d580452aa14d7d 100644 --- a/lib/compat.php +++ b/lib/compat.php @@ -97,21 +97,28 @@ function gutenberg_fix_jetpack_freeform_block_conflict() { * @since 0.10.0 */ function gutenberg_ensure_wp_api_request() { - if ( wp_script_is( 'wp-api-request', 'registered' ) ) { + if ( wp_script_is( 'wp-api-request', 'registered' ) || + ! wp_script_is( 'wp-api-request-shim', 'registered' ) ) { return; } - gutenberg_register_vendor_script( + global $wp_scripts; + + // Define script using existing shim. We do this because we must define the + // vendor script in client-assets.php, but want to use consistent handle. + $shim = $wp_scripts->registered['wp-api-request-shim']; + wp_register_script( 'wp-api-request', - 'https://rawgit.com/WordPress/wordpress-develop/master/src/wp-includes/js/api-request.js' + $shim->src, + $shim->deps, + $shim->ver ); // Localize wp-api-request using wp-api handle data (swapped in 4.9-alpha). - global $wp_scripts; $wp_api_localized_data = $wp_scripts->get_data( 'wp-api', 'data' ); if ( false !== $wp_api_localized_data ) { wp_add_inline_script( 'wp-api-request', $wp_api_localized_data, 'before' ); } } -add_action( 'wp_enqueue_scripts', 'gutenberg_ensure_wp_api_request' ); -add_action( 'admin_enqueue_scripts', 'gutenberg_ensure_wp_api_request' ); +add_action( 'wp_enqueue_scripts', 'gutenberg_ensure_wp_api_request', 20 ); +add_action( 'admin_enqueue_scripts', 'gutenberg_ensure_wp_api_request', 20 );