Defer until wp.api has initialized before attempting to use Backbone models

[westonruter]

Fixes #1235.

The first time the WP-API Backbone client is loaded in the browser, the REST API schema is fetched from the server and then stored in sessionStorage. As such, the Backbone models and collections are not available for such initial loads. To ensure they are available, the wp.api.init() promise should first be resolved. Issue can be replicated by doing sessionStorage.clear() and then reloading the Gutenberg editor.

[youknowriad]

These are not the only places we're using the backbone client, we use it also in: effects, tags, categories, featured image

---

I think we should try to find a global solution to this. Anyway, using the backbone client is not so scalable, we're using it for consistency but I think, we should discuss it broadly and probably avoid using in, in favor to more declarative approach (for example a FETCH effect handler)

[adamsilverstein]

@youknowriad curious why you say that using the backbone client "is not so scalable"?

[youknowriad]

Some context here: #907 (comment)

Simple question since I'm probably missing some context too:

• What does the backbone client give us that we can't do without any hassle using window.fetch or superagent...?

I see several issues with the backbone client:

• The fact that it needs an init is an issue for me, it's easy to miss.
• It's hard to implement things like batching, replays, caching.
• How do we use with endpoints like the oEmbed proxy endpoint?

Edit: Worth nothing that we're moving away from a similar client in Calypso to enable advanced features like caching, batching, replays, ...

[adamsilverstein]

What does the backbone client give us that we can't do without any hassle using window.fetch or superagent...?

A simple, declarative way to handle the built in WordPress object types and their idiosyncrasies (writing wp.api.models.Post makes it pretty clear by looking at the code that you are dealing with a post). It also handles objects with a parent relationship (revisions), trashable objects, and API responses with _embeded data. Out of the box handling for nonces, nonce refreshing and cookie based authentication - which is exactly what we need to communicate with the wp-api. Backbone models also offer an excellent, proven approach to handling object data like a post, and collections like a set of revisions or categories.

The fact that it needs an init is an issue for me, it's easy to miss.

It also accepts a localized schema, if we localize the schema no additional ajax call is required, and the init call is essentially synchronous/will return immediately. It needs an init to set up the data types, which is what makes it useful. the init is called automatically when the client loads, the main thing you need to do is wait for it to complete - see https://developer.wordpress.org/rest-api/using-the-rest-api/backbone-javascript-client/#waiting-for-the-client-to-load.

It's hard to implement things like batching, replays, caching.

batching is not supported by the WP-API currently, if it does become supported, that support will be added to the core wp-api client as well. why would you want to rebuild that outside wp-core if gutenburg is destined for core?

replays - not sure what you mean by that, but if you are talking state replays, that seems like a higher level functionality we would handle in redux middleware?

caching of callbacks in localstorage or another storage mechanism is dead simple, something like this makes it plug and play: https://github.com/SaneMethod/jquery-ajax-localstorage-cache

How do we use with endpoints like the oEmbed proxy endpoint?

these are created on a separate namespace for legacy reasons. the client supports multiple endpoints, you would just need to call init a second time specifying the oembed endpoints. wp.api.init( { 'versionString': 'oembed/1.0' } );

Edit: Worth nothing that we're moving away from a similar client in Calypso to enable advanced features like caching, batching, replays, ...

That is fine for Calypso, however this is for core and i'd much rather see us enhance the core client to support the features you need so everyone can benefit from them rather than re-inventing existing functionality.

Also worth noting that while Backbone uses jQuery.ajax by default, it is not dependent on it, you can readily swap this out with fetch or another polyfill.

[youknowriad]

Out of the box handling for nonces, nonce refreshing and cookie based authentication

I agree this is a good feature and this should be backed in the HTTP layer (the backbone client here) 👍

Backbone models also offer an excellent, proven approach to handling object data like a post, and collections like a set of revisions or categories.

This is subjective and I agree that's it's a nice approach when using Backbone as a framework but when we talk about unidirectional data-flow, single store (redux) ... this is not the recommanded practice, the state should ideally contain plain JS objects and any async call should be baked into async action creators. Having such methods in the collections and models is not really a good practice here.

replays - not sure what you mean by that

Or retries: I mean re-trigger the requests on errors (with threshold config, network detetion and stuff like that)

that seems like a higher level functionality we would handle in redux middleware?

Exactly, and if we avoid using a model based HTTP layer and use a generic layer, this middleware could be written easily

caching of callbacks in localstorage or another storage mechanism is dead simple, something like this makes it plug and play: https://github.com/SaneMethod/jquery-ajax-localstorage-cache

I was thinking about memory caching (store): Something to answer these questions:

• Don't fetch settings twice unless an explicit refresh is asked with an action attribute
• Caching configuration per action
• Avoid fetching a resource if the same resource has just been fetch or is inflight

however this is for core and i'd much rather see us enhance the core client to support the features you need so everyone can benefit from them rather than re-inventing existing functionality.

I totally agree and that's why we're using it and I didn't suggest removing it now but more having a discussion later (in js meetings), but the more we move towards Gutenberg Like approaches with a single data store, unidirection data-flow, the more we'd need to rethink the client.

[westonruter]

@youknowriad thanks for pointing out the other instances. I realized that the approach I was taking wasn't ideal and we should just defer initializing the editor in the first place until the Backbone client has finished initializing. I've replaced previous commit in this branch with this change.

[aduth]

@adamsilverstein Is it discouraged to localize the schema as you'd mentioned? It's not entirely clear from the documentation, except from the SCRIPT_DEBUG note I might infer that it's not recommended for production. On the flip-side, waiting to show anything in the browser until the schema has finished loading doesn't seem entirely desirable either.

[aduth]

This might not be the right place to discuss needs around the API client, and am not advocating it being replaced, but a couple specific pain points encountered so far include:

• Testing: See also @BE-Webdesign's Add tests for refx REQUEST_POST_UPDATE effects network requests. #966, which is currently stuck in limbo. In my experience, chainable APIs are very awkward to mock (example) and network mocking with nock feels perhaps too low-level and fragile (example). Have you seen good patterns for testing or mocking wp-api.js requests? In place of mocks, some recent patterns like in Redux Sagas or Calypso's data layer have advocated pushing network requests (side effects) as much as possible to the edge, in order to keep actions as plain objects describing intent to request, such that testing is merely verifying the action matches the expected object shape.
• Isolation: Continuing from the previous point, ideally our action creators or effects are not themselves invoking network requests but instead describing intent to request data, deferring instead the implementation of the request to an isolated request handler. Describing a request as a plain object is not very compatible with wp-api.js's API. We might be able to do some conversion of e.g. { model: 'post' } -> wp.api.models.Post, { method: 'POST', body } -> post.save( body )

Related: #902

[westonruter]

In any case, I think this should be merged because it fixes the editor from being completely broken. We can revisit the approach for interfacing with the UI later.

[youknowriad]

I tend to agree with @westonruter. Obviously, the UX regression here is not great, but we should get this merge to fix the bug (I already saw at least 3 reports of a white page).

So, if I understand properly the delay would be noticeable only on the first load?

[adamsilverstein]

@aduth I would actually encourage localizing the schema, especially given the wp-admin context. We leave this off by default because it adds additional weight/data to the page and might be unexpected by developers enqueueing the script. I will prepare a PR to localize the schema for Gutenburg - no other code will need to change, and we should still merge @westonruter's PR - the main difference is that the deferred will resolve immediately, without requiring an Ajax callback.

In regards to mocking/testing, the approach we use in core would be applicable (especially after a core merge) - see https://core.trac.wordpress.org/browser/branches/4.8/tests/qunit/fixtures/wp-api.js for the js guts - in short, we override Backbone.ajax to returned the mocked data. We generate the mocked data in PHP unit test to ensure it is current with the actual API responses, see https://core.trac.wordpress.org/browser/branches/4.8/tests/phpunit/tests/rest-api/rest-schema-setup.php for the details.

I agree actions should remain pure to keep them testable and I will take a look at the approaches you linked to try to understand how you push the network requests 'to the edge'. I'd like to understand how that works and see where it differs from how we would use wp-api.

[adamsilverstein]

@aduth - I reviewed the approach used the Data Layer in Calypso and I think we could likely use a similar middleware approach here, keeping the client request in the middleware layer. I've also seen a similar approach where the middleware returns a promise, letting you act later on the asynchronous event completion.

[westonruter]

@youknowriad it could only be noticeable for the first time the user accesses the Gutenberg editor if the WP-API has never been loaded yet in the current browser session, such as somewhere else on the WP install.

[aduth]

Would rather see schema localized, but let's defer that to a subsequent pull request (ideally with issue to track).

[mtias]

I don't like this approach very much. Coupling editor initialization with wp.api seems convoluted.

[westonruter]

Agreed. This is just a stop-gap to prevent the white screen of death in the immediate term.