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

Automatically and transparently load block assets asynchronously #23098

Closed
sarayourfriend opened this issue Jun 11, 2020 · 3 comments
Closed
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Performance Related to performance efforts

Comments

@sarayourfriend
Copy link
Contributor

The problem

Currently whenever a user (a) installs a block through the block directory or (b) installs a plugin which registers blocks, if those blocks have dependencies (scripts and styles, though most often scripts will be heavier) those dependencies are now always loaded on page load for every post type. This creates a non-transparent performance issue for every installed block. Each block with script dependencies will increase the JS payload, meaning block developers (and consumers) have to decide between a faster editor or a more featureful editor. I think we can bridge some parts of that gap to reduce the amount of conflict between those two motivations.

The most egregious offender is the classic block and its dependency on TinyMCE. Unfortunately, TinyMCE is initialized and behaves unlike almost any other external dependency. Its initialization is multi-faceted its dependents are not always obvious or easy to deal with. More on that in this issue #21738.

A solution

I'd like to make block dependency (also known as "assets") load asynchronously whenever possible (primarily when a block is first inserted into a post).

We already do this to some extent for the block-directory, where assets are loaded when the block is installed. This only works for "JS only" blocks, so we will need to devise a solution for blocks which enqueue their dependencies in their render_callbacks.

To accomplish this solution, we could use a block’s declared assets to automatically wrap blocks’ edit functions in a LazyLoad component which would introspect on a block's assets and load them when necessary, delaying the rendering of the block's inner edit component until the dependencies are loaded. This could potentially be done within the registerBlockType JS function and register_block_type PHP function.

The LazyLoad component would need the following support components around it:

  1. It would depend on the REST API proposed by @spacedmonkey in this PR: Add script and style REST APIs #21244. It would use this REST API to request a list of dependencies for a block's declared asset handles.
  2. Create a dependency cache class for the frontend with the following capabilities:
    1. Automatically detecting already loaded dependencies using the API exposed in this PR.
    2. Expose an API for the Gutenberg plugin to pre-load a map of known dependencies (i.e., dependencies for blocks that are already installed/used for a post, whatever criteria we decide) as handle -> dependency URI. NB: We need to figure out a solution for inline scripts as pre-loading them to be evaled sort of defeats the point, even if technically their parsing will be offset.
    3. Expose an API for serially loading a list of dependency handles. This API should handle both the actual loading of the script (creating the script tag, evaling inline scripts, etc), adding them to the cache of already loaded dependencies, and preventing duplicate loads by tracking in-progress dependencies.
  3. Support a plan for refactoring blocks that currently use render_callback to enqueue their dependency scripts to no longer do so. Based on point 2.1 this should be transparent as the currently enqueued dependencies will already be being tracked, so we should hopefully just have to remove the logic for enqueuing them from render_callback and the new structure for automatically loading dependencies for a block on usage from point 2 will handle it.

Additionally, because this would duplicate behaviors in the block-directory where JS block assets are loaded after they are installed, we could remove dependency loading logic from the block-directory and instead simply depend on the async block dependencies API to load the scripts. Currently the block-directory immediately loads the dependencies and then enters the editing for that block (this follows the most common use-case for when someone installs a block from the directory). By maintaining the behavior where the new block type is immediately selected for editing, we’ll automatically cause the dependencies to be loaded through the LazyLoad component. The end result of this work would have the dependencies be loaded at exactly the same time they were being loaded before but also cutting down on the logic in the block-directory (we can effectively completely remove the loadAssets control).

TinyMCE

Because TinyMCE is such an eggregious example of this problem, I've already started tackling it as a problem. However, TinyMCE has proven to be basically a super-special case with lots of edge cases and exceptions. I have learned some things from that work though, primarily that as of today, it is not possible to asynchronously load dependencies for blocks that are already being used on a post because when a post is first rendered in the editor, the edit for each block in the post is run. There are two solutions for this problem, one immediate and one long term.

Problems with blocks already used on a post

The immediate solution is to pre-load dependencies for blocks that are used on a post, as is done today for all available blocks. This will still give us a win because the scope of pre-loaded dependencies will undoubtedly still be smaller.

The long term solution is to not render the edit for a block just to display its content in the editor, when possible. If it is at all possible, my thought is that we could just render the contents of the block. I'm not completely familiar with all block use cases and I'm sure there are some creative ones that would preclude this, but potentially we could create an "opt-in" so that performance oriented blocks would be able to take advantage of this. It would mean expanding the API in a non-trivial way.

This problem also means that we will not be able to asynchronously load style dependencies for already used blocks, probably ever, because the rendered contents of a block may depend on those styles. In any case, as I said in the first section, the script dependencies are more likely to be the heavier part of a block's assets.

Alternative approaches

I haven't really been able to think of alternative approachs that solve this problem. Maybe there's another angle to tackle this from, like maybe there's a different boundary in the block API that would be more worthwhile to async. If so, I'd love to hear about that. Overall I think there's a few different places where we can turn currently synchronous APIs into async ones like parsing, rendering, and so on... but none of those address the particular load time problem that this issue tries to deal with.

@gziolo gziolo added [Type] Performance Related to performance efforts Framework Issues related to broader framework topics, especially as it relates to javascript labels Jun 13, 2020
@gziolo
Copy link
Member

gziolo commented Jun 13, 2020

Great summary 😍

Can we consolidate open issues a little bit? It’s the same idea that was discussed in #2768 😃 We should close one of them as a duplicate, usually we prefer an older one to keep all the history of discussion. It’s up to you here as your issue has way more details included.

I noticed that #21738 overlaps with this issue as well so we might want to close one of them. It would be also nice to put the list of action items in the description (closer to the top?) so it’s easy to follow progress and find all related PRs. #18619 is a good example of such list.

@sarayourfriend
Copy link
Contributor Author

@gziolo Thanks for the feedback, that's super helpful. I'll copy the body of this issue into a comment on the older issue you linked, I also prefer to preserve the history of the discussion in a single place 👍

re: combining the TinyMCE issue into this work, I'm happy to mention it in the existing issue you linked but I think it should be a separate issue simply because there are particularities to TinyMCE that make it unable to be part of the "normal" async block dependencies flow yet (exclusions from meta boxes, changes needed in core to support it, etc)... I'd rather keep that discussion out of the larger discussion about how to approach a general solution lest we end up in a TinyMCE shaped rabbit hole 😂

I'll close this issue and then create a roadmap issue to track the progress of the work including the work that needs to be done in core for TinyMCE and so on. Thanks for linking an example of that, super helpful 👍

@gziolo
Copy link
Member

gziolo commented Jun 16, 2020

Closed in favor of #2768. Let’s continue there 👍

Regarding TinyMCE optimizations. I get your point, if think it’s helpful to keep it open then it’s fine. My thinking was that the detailed discussion would happen in PR that contains implementation details.

I'll close this issue and then create a roadmap issue to track the progress of the work including the work that needs to be done in core for TinyMCE and so on.

I can update the description of #2768 with the roadmap at the top of it when you live a comment here or there. We have over 2k issues opened in Gutenberg and it’s a huge challenge to triage so we always aim to keep everything consolidated as much as possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Framework Issues related to broader framework topics, especially as it relates to javascript [Type] Performance Related to performance efforts
Projects
None yet
Development

No branches or pull requests

2 participants