Directory

Adjust the logic of Post Title edit, so it avoids unnecessary OPTIONS requests by kmanijak · Pull Request #49839 · WordPress/gutenberg · GitHub
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

Adjust the logic of Post Title edit, so it avoids unnecessary OPTIONS requests #49839

Merged

Conversation

kmanijak
Copy link
Contributor

@kmanijak kmanijak commented Apr 14, 2023

What?

PR's goal is to optimize the Post Title block render time in the Editor by avoiding unnecessary requests in a particular case.

Why?

Post Title cannot be edited when it's a descendant of a query loop. However, in many cases, the OPTIONS request is made and then its result is ignored. On the pages with Query Loop and N Post Titles, it adds N number of OPTIONS requests that slow down loading the posts in the editor.

How?

This can be optimised, to make a call only when necessary.
Post title checks if it can be edited by useCanEditEntity usage. At the same time if isDescendentOfQueryLoop is true outcome of useCanEditEntity doesn't matter as it's check here and here.

Based on the above we could actually call useCanEditEntity only if isDescendentOfQueryLoop is false, however that's not allowed, as useCanEditEntity is a hook and cannot be called conditionally.

Using the fact that useCanEditEntity returns a falsy value (and skips making OPTIONS request) when there's no proper data we can conditionally provide "empty" data as arguments. That part probably could be improved to make it more bullet-proof, which I'm happy to get a feedback on.

One of the use cases for the above is the Products block from WooCommerce Blocks (variation of Query Loop block). WIth the 9 products there's an overhead of:

  • on average ~0.7s of loading time on a Local setup
  • on average ~1.5s of loading time on a remote setup

Testing Instructions

  1. Go to Editor
  2. Add a Query Loop block (choose option that includes Post Title)
  3. Go to Dev Tools > Network tab
  4. Look for /wp-json/wp/v2/posts/{POST ID} OPTIONS requests
  5. Expected: There's no requests like that and title cannot be edited

  1. Add a new post
  2. Add a Post Title block
  3. Go to Dev Tools > Network tab
  4. Look for /wp-json/wp/v2/posts/{POST ID} OPTIONS requests
  5. Expected: There's a request and title can be edited

@github-actions github-actions bot added the First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository label Apr 14, 2023
@github-actions
Copy link

👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @kmanijak! In case you missed it, we'd love to have you join us in our Slack community, where we hold regularly weekly meetings open to anyone to coordinate with each other.

If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information.

@Mamaduka Mamaduka added [Type] Enhancement A suggestion for improvement. [Block] Post Title Affects the Post Title Block labels Apr 15, 2023
Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

Thanks for the PR @kmanijak and you have a great description!

I can live with this small hack - I found it clever 😄. You just need to rebase and let's 🚢

packages/block-library/src/post-title/edit.js Outdated Show resolved Hide resolved
packages/block-library/src/post-title/edit.js Outdated Show resolved Hide resolved
@kmanijak kmanijak force-pushed the try/avoid-options-requests-from-post-title branch from b47e41c to 76bb969 Compare May 12, 2023 09:42
@kmanijak
Copy link
Contributor Author

Thanks for the PR @kmanijak and you have a great description!
I can live with this small hack - I found it clever 😄. You just need to rebase and let's 🚢

Thanks a lot for the review! I applied the suggested changes and rebased with trunk. However, when re-testing I spotted that sometimes isDescendantOfQueryLoop is false within Query Loop when the pattern chooser is opening and the call is being made anyway. Seem like something may have changed, but I didn't have time to investigate.

Copy link
Contributor

@ntsekouras ntsekouras left a comment

Choose a reason for hiding this comment

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

However, when re-testing I spotted that sometimes isDescendantOfQueryLoop is false within Query Loop when the pattern chooser is opening and the call is being made anyway.

This is happening because of this line setting the queryId in an effect. I'm not sure right now how we can handle this, but I think we can land this PR for now and explore separately.

Thanks again!

@ntsekouras ntsekouras merged commit 3483740 into WordPress:trunk May 12, 2023
48 checks passed
@github-actions
Copy link

Congratulations on your first merged pull request, @kmanijak! We'd like to credit you for your contribution in the post announcing the next WordPress release, but we can't find a WordPress.org profile associated with your GitHub account. When you have a moment, visit the following URL and click "link your GitHub account" under "GitHub Username" to link your accounts:

https://profiles.wordpress.org/me/profile/edit/

And if you don't have a WordPress.org account, you can create one on this page:

https://login.wordpress.org/register

Kudos!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Post Title Affects the Post Title Block First-time Contributor Pull request opened by a first-time contributor to Gutenberg repository [Type] Enhancement A suggestion for improvement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants