StartPageOptions: load and parse patterns only after establishing the need for them #53673
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
jsnajdr
added
[Type] Performance
|
Size Change: -284 B (0%) Total Size: 1.51 MB
ℹ️ View Unchanged
|
youknowriad
reviewed
Aug 15, 2023
| }, | ||
| [ modalState, hasStartPattern ] | ||
| ); | ||
| function StartPageOptionsModal() { |
There was a problem hiding this comment.
I believe there's also StartTemplateOptions in the site editor, I wonder if it suffers the same issue.
There was a problem hiding this comment.
That's a good point, I reviewed StartTemplateOptions, and found that it also parses patterns, but it already uses the "two nested components" technique that my PR introduces for StartPageOptions. The outer component checks the pre-conditions, and only the inner component starts working with patterns.
|
Flaky tests detected in 963558d. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/5866135079
|
tyxla
approved these changes
Aug 23, 2023
There was a problem hiding this comment.
This cleanup looks good to me and tests well 👍
I also found that it's slightly improving some core vitals metrics by 5-6%, for example:
This branch:
- FP: 2.01s
- DCL: 2.20s
- FCP: 2.64s
- LCP: 3.17s
trunk
- FP: 1.97s
- DCL: 2.31s
- FCP: 2.81s
- LCP: 3.38s
Given that it surely doesn't make things worse (and even appears to be improving them) I think we should land it and keep an eye on the averaged code vitals metrics 🚀
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR fixes an issue where block patterns are parsed during the editor initial load, synchronously, and therefore are slowing down the load. I discovered the issue when working on async block loading in #51778.
The problem is with the
<StartPageOptions>component (added in #40034) which displays a modal to select a pattern when creating a brand new page from scratch. This component looks at the list of patterns, which triggers pattern parsing. But most of the time the component will not be displayed at all, because there are preconditions that are rarely satisfied: the page content is empty and unsaved. And yet patterns get synchronously parsed during the initial load.This PR fixes the issue by refactoring
StartPageOptionsinto two nested components. The outer one checks the preconditions (freshly created page) and renders the inner component, the one that parses patterns, only when the precondition is satisfied.The second commit removes the
usePreParsePatternshook (added in #29444) completely. Even though this hook parses patterns only during idle time, I believe we don't need it. It's OK to wait until patterns are actually needed, e.g., when opening the block inserter.How to test:
Verify that before this PR patterns were parsed on initial load, while after this PR they are parsed only when opening the block inserter. Can be done by adding a logpoint to the
__experimentalGetParsedPatternselector.Then verify that
StartPageOptionsis correctly shown. It's shown after creating a new page, and when you have a pattern withblockTypes = [ 'core/post-content' ]. You can temporarily change the condition fromcore/post-contenttocore/queryto see matching patterns in themes like Twenty Twenty Three.