useInnerBlockTemplateSync: remove microtask#80100
Conversation
|
Size Change: +99 B (0%) Total Size: 7.68 MB 📦 View Changed
|
|
The existing of these micro tasks has to do with loading a long posts with a lot of container blocks (lists, quotes...) and doing things synchronously used to result in a lot of block rerendering. So it's a good idea to keep an eye on the metrics here. |
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
I assume here that when we are loading an existing long post, the block structure is fully formed and doesn't need any templates to be applied. For example, there won't be an empty Template insertion typically happens only when a block is inserted in the UI. So, let's check the metrics for sure, but I expect that these two workflows have zero influence on each other. |
|
The performance job doesn't seem to show any meaningful impact, so I guess you're probably right. |
We don't have a test suite for that. Current fixtures:
Makes sense. @jsnajdr, any manual testing instructions or e2e tests should cover that? |
|
The large post contains a lot of quote and list block IIRC, all of these are container blocks. But yeah, as @jsnajdr shares, these probably don't use the template because they're not empty, but I also don't expect a big post with a lot of empty containers. |
That the e2e suite passes is a pretty good signal. But if there is one thing I'd like to test, it's related to this comment before the code that flushes nested settings before performing template sync:
That means that if we removed the I remember I used to know how this works 3 years ago when I worked on block lazy loading (#51778), but today I'm failing to reproduce anything. Maybe @youknowriad or @ellatrix will know more about this. |
This is a spinoff from #79021 that extract a valuable part of the PR: do the template sync more straightforwardly, without and extra mysterious
queueMicrotask. The reason why the microtask was there is thatuseNestedSettingsUpdatealso defers store updates to a microtask, because of batching. In our new implementation we do a selective synchronous flush of just the one relevant update that we need.Doing the template sync a bit earlier causes a reordering conflict with
useMigrateOnLoadin the List and Quote block. I'm solving this by moving the migration to anInnerBlockschild. Then the effects will run in the original order: migration first, template sync second.