Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add preload paths for autosaves #15067

Merged
merged 4 commits into from
May 4, 2019
Merged

Conversation

talldan
Copy link
Contributor

@talldan talldan commented Apr 19, 2019

Description

Adds a preload path for autosaves to help optimize the changes made in #7945.

Note there is a trac ticket that replicates this functionality in core (https://core.trac.wordpress.org/ticket/46974). In the meantime this should bring the same optimization to the plugin.

How has this been tested?

  1. Create a new post and publish it
  2. Open the browser dev tool and monitor the network requests
  3. Reload the editor

With this change: no requests to the autosaves endpoint are made when reloading the editor.
Without this change: a request is made to the autosaves endpoint when reloading the editor.

Types of changes

New feature (non-breaking change which adds functionality)

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.

@talldan talldan added the [Type] Task Issues or PRs that have been broken down into an individual action to take label Apr 19, 2019
@talldan talldan self-assigned this Apr 19, 2019
@talldan talldan changed the title Add/preload paths for autosaves Add preload paths for autosaves Apr 19, 2019
Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

Prior to this I had opened #15061 for a similar need. It's since been merged. There are now conflicts here, though I think it should be simple enough to follow the same pattern.

Filter out any duplicate endpoints
@talldan talldan force-pushed the add/preload_paths_for_autosaves branch 2 times, most recently from 2040c3c to fcaa7bf Compare May 2, 2019 08:05
@talldan talldan force-pushed the add/preload_paths_for_autosaves branch from fcaa7bf to 1d4fda5 Compare May 2, 2019 08:07
@talldan
Copy link
Contributor Author

talldan commented May 2, 2019

@aduth - now rebased. Merging the changes wasn't too bad, the tests were a bit trickier, but I've managed to work it out.

*/
function test_localizes_script() {
$preload_paths = gutenberg_extend_block_editor_preload_paths( array() );
$post = (object) array(
Copy link
Member

Choose a reason for hiding this comment

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

Is this variable (and in the test below) unused?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I made a mistake leaving that there after discovering that I could use the post factory in the test. Thanks for pointing it out.

* the minimum supported version.
*/
$post_type_object = get_post_type_object( $post->post_type );
$rest_base = ! empty( $post_type_object->rest_base ) ? $post_type_object->rest_base : $post_type_object->name;
Copy link
Member

Choose a reason for hiding this comment

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

While it seems unlikely given the filter flow, it makes me nervous to see null as a possible return type of get_post_type_object and then to have unguarded object property references. Should we be more guarded (wrap this whole bit in an if ( ! is_null( $post_type_object ) ) {)?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

👍 good idea, I've addressed that.

Copy link
Member

@aduth aduth left a comment

Choose a reason for hiding this comment

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

👍

@talldan talldan added this to the 5.7 (Gutenberg) milestone May 4, 2019
@talldan talldan merged commit 7addd76 into master May 4, 2019
@talldan talldan deleted the add/preload_paths_for_autosaves branch May 4, 2019 01:52
@youknowriad
Copy link
Contributor

It looks like we need to commit this patch as well https://core.trac.wordpress.org/ticket/46974 for WP 5.3

@youknowriad youknowriad added the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Sep 13, 2019
@jorgefilipecosta jorgefilipecosta removed the Backport to WP 6.7 Beta/RC Pull request that needs to be backported to the WordPress major release that's currently in beta label Feb 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Type] Task Issues or PRs that have been broken down into an individual action to take
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants