search

WordPress / gutenberg

The Block Editor project for WordPress and beyond. Plugin is available from the official repository.
https://wordpress.org/gutenberg/
Other
10.29k stars 4.11k forks source link

Block and Component Lazy Loading #2768

Open gziolo opened 6 years ago

gziolo commented 6 years ago

Tasks List

Issue Overview

Initially raised by @mtias in #665 and something that @dmsnell has already started exploring when working on CodeMirror block. It would be beneficial to have a standardized way to defer loading optional parts of the block and/or the whole block in general when it is resource heavy. I might even risk saying that it would make sense to have all external blocks by default loaded on demand using chunk splitting technique. It would help us ensure that only essential parts of Gutenberg are loaded on initial page load, which would lead to better performance and overall first-time impression. We could start pre-loading optional blocks when the browser is idle or whenever a user is about to select such block using the drop-down menu or / command. All that should help scale Gutenberg as the number of available blocks grows.

Related Issues, PRs and discussions

noisysocks commented 6 years ago

We can probably get some big wins by lazily loading in the assets that are queued when we call:

mtias commented 6 years ago

Yes, this seems more and important to get in place.

noisysocks commented 6 years ago

Two outstanding issues to bear in mind when we eventually revisit how asynchronous components are handled in Gutenberg:

noisysocks commented 6 years ago

I've been thinking about this a little and thought I'd dump some of the contents of my brain. It would be good to get the conversation going. I recall that @youknowriad was mulling over some ideas in this area at WordCamp Europe.

There's two things we're talking about in this issue:

  1. Lazily loading assets that are bundled as part of Gutenberg. For example, the code that defines a gallery block
  2. Lazily loading assets that are included in WordPress. For example, the code that defines wp.codeEditor.

Gutenberg assets

To do this, we can make use of dynamic imports and the import() function.

import( /* webpackChunkName: "wp-block-gallery" */ './gallery' ).then( ( { name, settings } ) => {
    registerBlockType( name, settings );
} );

Here, the code in core-blocks/gallery/index.js will be bundled as a seperate webpack chunk and won't be loaded by the browser until import() is called.

WordPress assets

This is trickier.

One idea is that we could write our own webpack loader which lets you specify a list of WordPress script or style assets that you wish to load.

import 'wp-script-loader!wp-codemirror,code-editor,htmlhint,csslint,jshint,htmlhint-kses';
import 'wp-style-loader!wp-codemirror,code-editor';

The webpack loader would return a script that, when executed by the browser, inserts a <script> element into the DOM which loads the specified assets using the existing /wp-admin/load-scripts.php or /wp-admin/load-styles.php.

We can then combine this technique with the import() function as above to enable lazy asset loading.

Promise.all( [
    import( /* webpackMode: "eager" */ 'wp-script-loader!wp-codemirror,code-editor,htmlhint,csslint,jshint,htmlhint-kses' ),
    import( /* webpackMode: "eager" */ 'wp-style-loader!wp-codemirror,code-editor' ),
] ).then( () => {
    wp.codeEditor.initialize( textarea, settings );
} );

Here, the <script> tag which loads the assets won't be inserted into the DOM until import() is called.

As cool as this is, it's not clear to me that it's a better approach than e.g. writing a wp.components.withLazyAssets HOC which injects a <script> tag into the DOM. The benefits of a HOC is that it's easier to understand and can be be used by third party developers.

dmsnell commented 6 years ago

thanks for the discussion @noisysocks

one idea this leads me to is a realization that we have some resources we need as soon as possible and some that can load later.

for example we want to know the name, category, icon, and other meta details about a block so that we can do things like load the block inserter. however, since some blocks are complicated they might only need to load on first use (which could be a preview in the block inserter or could be in the editor itself).

on that note it also seems reasonable that we could encourage coding blocks that have dynamically loaded elements. this could actually solve the dilemma while leaving open the door that some plugins will abuse it by loading everything up front.

registerBlockType( 'my-cool-block', {
    title: __( 'My Cool Block' ),
    attributes: [ … ],
    save: dynamicLoad( './my-cool-save' ),
    edit: dynamicLoad( './my-cool-edit' ),
} );

with this we can provide a generic loading view for components which are loading but immediately replace them once loaded. behind the scenes we use import() or whatever mechanism is available and then individual authors need not worry about it. (maybe this is the idea of withLazyAssets() - I'm not familiar with that)

aaronjorbin commented 5 years ago

I am putting the future label on this, but if it's something that would be really beneficial for 5.0, please move it into the appropriate milestone.

gziolo commented 5 years ago

It’s going to be essential for blocks discovery at some point, 5.1 should be fine as target.

gziolo commented 5 years ago

Bringing comment from @ockham shared in #12232:

Is your feature request related to a problem? Please describe. While working on Jetpack Gutenberg blocks and plugins, we've recently found a few instances where some sort of async fetching of resources prior to block registration would come in handy. Examples include:

  1. Translation files. This is relevant when Gutenberg is used outside of wp-admin, e.g. in Calypso. We have a Calypso-specific workaround (https://github.com/Automattic/wp-calypso/pull/28304) that takes care of loading translation files prior to loading Gutenberg, but we were wondering if more native tooling might be in order here. (We can't simply load translations after the editor and rely on React re-rendering strings afterwards for things like block names and descriptions that we pass to registerBlockType).
  2. Information relevant for block availability. We might want to register blocks conditionally, depending on criteria obtained from a REST API endpoint.

Describe the solution you'd like A bit fuzzy, but maybe allow registerBlockType() (and registerPlugin()) to return a promise, and wait for that to be resolved?

Describe alternatives you've considered For 1. -- see the workaround mentioned there. For 2. -- In wp-admin, we're using wp_localize_script to set a global variable to contain relevant information (None of which are particularly nice.)

/cc @sirreal @tyxla @simison @lezama @enejb

dmsnell commented 5 years ago

it's also worth discussing the implications of our editing flow and our block invalidation flow. until our editing flow is asynchronous we might still run into pretty massive issues trying to make one part of it asynchronous.

in #7970 I proposed an asynchronous parsing flow which @aduth helped me with but we ran into the same issue that the entire stack is built upon a synchronous model. the issue of the editor invalidating blocks between unregistering on and re-registering highlights the problem well.

it would be really helpful for us to discuss the semantics of an asynchronous editing session and nail down some common vocabulary before hacking away at it - actually I'm sure this is something we should defer until later and not try to get in before "the merge" or even shortly afterwards.

the implications here of moving to an asynchronous model are numerous:

in the short-term for block registration we might be able to get away with creating a replaceBlockType function that swaps a block's implementation in a way where we don't re-render until it's over.

type Block
  = Pending BlockInfo
  | Loaded BlockInfo

type Document
  = Uninitialized
  | Parsing (Stream [Block])
  | Parsed [Block]

type Editor
  = Uninitialized
  | LoadingCoreEditor
  | Pending Document
  | Running Document

Practically some of these states will connote things like "I can or cannot edit this block right now" and "we should wait to invalidate the block content until it's ready" and "the editor itself is changing to stop all input processing momentarily" etc…

Ciantic commented 4 years ago

I think you people have weighed this in some way or another, but switching to using something like SystemJS could make sense. It allows to use modern way to import with older browsers. However I know WordPress is all about backwards compatibility so I wonder if this change even doable with any library?

It's doable to register synchronously resolved libraries with SystemJS e.g. (if used one of the s.js extensions):

System.register("@wordpress/blocks", [], function(_export, _context) {
    _export({ default: wp.blocks, __useDefault: true });
    return {
        execute: function() {
        }
    };
});

However importing is still done in Promise, and since blocks does not support any kind of asynchronous workflow, it won't work according to my tests.

Something simple could suffice to allow users to do similar things, like allowing the second argument in registerBlockType to be a function that returns a Promise of a block.

sarayourfriend commented 4 years ago

Coming into this discussion late, but happy to see how much thought has already gone into this. I've been trying to revive this work a little bit in https://github.com/WordPress/gutenberg/issues/23098, but @gziolo suggested I could just continue the discussion in this issue 😊

Following up on @noisysocks's suggestions here, we have a component in Automattic/wp-calypso called AsyncLoad which works off of this babel transformation. I think it would acocmplish what @noisysocks was suggesting for Gutenberg assets. I'm happy to work on getting that AsyncLoad component ready to share, probably either creating a new version based on it for the @wordpress that we'd later on switch to using in wp-calypso. The babel transformation is already a seperate package. Using it also requires adding asyncRequire to ESLint's globals. Is there interest in a solution like that, given that it's already proven to be working on a large and well used repository? It does also have implications with respect to code-splitting. Not sure what the effects would be for that on Gutenberg or what would need to be done to support block development in that way.

Given what @dmsnell brings up here, however, I think that might not be the right path to go down right away...

The solution I've been exploring (which is roughly mirrored in this work I've been doing to asynchronously load TinyMCE when we can) involves wrapping block's edit functions with a component that will lazily load a block's assets when possible. There are edge cases where we're not able to do so that relate to @dmsnell's description of how the editor initializes and its current dependency and assumption on synchronicity, namely that we cannot lazily-load block assets for blocks that are already used on a page. You can see the result of that in the PR I have open for async TinyMCE: https://github.com/WordPress/gutenberg/pull/23068/commits/efeb76f7bb10819f35859ebf20bd89485bc8c4b4

A solution proposal...

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: https://github.com/WordPress/gutenberg/pull/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).

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.

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 so I think it warrants its own issue and approach outside of the scope of this particular issue... and in the future we can work towards making the classic block and TinyMCE work with whatever generalistic pattern we come up with here.

I'd love thoughts on the solution I'm proposing, whether there are initial issues with it that people can see.

One thing off the top of my head is this concept of using a HOC vs a wrapper component. Personally I find a wrapper component a lot easier to understand how to write and maintain (HOCs famously have a wealth of specific traits that need to be considered) so avoiding a HOC if a wrapper component is possible is always my preferred solution, but there's obviously room for both or only a HOC if that's the preferred style of the WordPress community, at the end of the day the method of implementation makes no difference to me as long as the underlying concepts are sound (which so far I believe they are 😹).

Anyway, I'd love feedback on this solution to the block assets problem as well as whether donating the AsyncLoad component from wp-calypso into the Gutenberg repository for use here would be a worthwhile thing to explore further.

gziolo commented 4 years ago

@saramarcondes, thanks for sharing your detailed proposal. Great work so far. Improving how TinyMCE is handled is extremely hard but it kooks like you are close to having it sorted out ✨

I'm happy to work on getting that AsyncLoad component ready to share, probably either creating a new version based on it for the @wordpress that we'd later on switch to using in wp-calypso. The babel transformation is already a seperate package. Using it also requires adding asyncRequire to ESLint's globals. Is there interest in a solution like that, given that it's already proven to be working on a large and well used repository?

I’m sure we need something similar but I was wondering if we could use now React API? There is lazy and Suspense in React that are recommended for code splitting.

The solution I've been exploring (which is roughly mirrored in this work I've been doing to asynchronously load TinyMCE when we can) involves wrapping block's edit functions with a component that will lazily load a block's assets when possible. There are edge cases where we're not able to do so that relate to @dmsnell's description of how the editor initializes and its current dependency and assumption on synchronicity, namely that we cannot lazily-load block assets for blocks that are already used on a page.

I think the solution you explored for edit implementation is the one that’s the most pressing because it’s where the heavy logic lives. We can start with the wrapper approach you used that would be opt in. It ensures that attributes and save method are present at the time of parsing so it operates as usual removing the blocker raised by @dmsnell. We can tackle that later. In fact it was discussed extensively in the past in other places and we still have it tracked in #16209 as follows:

Client-side properties referenced as files

This was removed from the initial proposal (see this commit: https://github.com/WordPress/gutenberg/commit/5ec9cd9da3bdfd5a97c997af434b5caf4c89faa1) as noted in https://github.com/WordPress/gutenberg/pull/13693#issuecomment-495135951:

I removed the following properties from this document:

  • save
  • edit
  • transforms
  • deprecated

The idea was to keep them referenced from block.json file but this seemed to be a bit confusing in relation to the assets you would normally enqueue in WordPress. This is something we will revisit later as it doesn't seem to be essential for the Block Directory project.

I think we should continue exploring this idea as this could give us some ways to optimize the size of assets loaded by deferring the moment when those files are loaded on the page. In particular deprecated and edit fields loaded only when they are actually used could bring some performance optimizations in case of blocks heavy loaded with JavaScript logic.

A note here, deprecated field is conditionally used in parsing so it suffers from the same issues as save.

sarayourfriend commented 4 years ago

Sounds good @gziolo. Thanks for the details and clarification around what will be possible right now, it aligns with my expectations.

Using lazy and Suspense sounds great. I wasn't actually aware that they were specifically for code-splitting!

Once the TinyMCE specific work is wrapped up I'll get to work right away on the generic solution.

sarayourfriend commented 4 years ago

I was discussing this project with a colleague yesterday and realized that there was some unnecessary work described in my comment here.

Specifically this part:

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.

This was only necessary when we thought that all blocks would always be able to take advantage of this LazyLoad component every time. However, even blocks that implement it would have a problem due to the synchronous nature of the initial editor render which triggers the edit for every block used in the post. This means that blocks need to already be ready to be rendered and cannot wait, meaning we need to pre-load dependencies.

This has two implications: it means that the frequency with which we will be making REST API requests of dependencies goes down to at most once-per-block insertion (and only when the block hasn't already been used) and means that anything that could have had its dependency URIs injected into the map described above will already have had its dependencies loaded.

TL;DR: we don't need this handle -> dependency URI mapping.

Likewise, for already loaded dependencies, we can just use the convention already established in Core around the id="$handle_name-js" that each dependency script tag will have. When the frontend's cache-of-loaded-scripts class is constructed, we can use a selector for IDs that match that pattern, parse them, and store them as the initial list of already loaded scripts. This only needs to be done once per-load of the editor.

sarayourfriend commented 4 years ago

After some work on the more generic LazyLoad component, I have some thoughts about the way forward. The progress so far can be seen in this draft PR: https://github.com/WordPress/gutenberg/pull/24127

What's left for the frontend?

The frontend still needs a working example. I've adapted the draft PR from the TinyMCE stuff, which itself does work, so I'm 99% sure this is "the right path" but expect that there are some bugs.

I'd like to come up with a minimum example of a block with a dependency that does all these things:

  1. The block's edit actually does some transformation on the block content
  2. The block's dependency has inline scripts associated with it
  3. Likewise, translations, and generic extra "data"
  4. Further still, I'd like the dependency to have a dependency and for one of the dependencies to be an alias for inline scripts (i.e., no "src")
  5. The dependency should also require some arbitrary initialization for onLoaded and preferrably something asynchronous. This can be mocked out with a setTimeout, doesn't really matter, just needs to spin an extra bit during onLoaded.

I think this will require a block which has at minimum two dependencies declared with a third transient dependency, one which has a src property, another which does not, and the third which is a dependency of one of the first two. All should have some kind of inline scripts.

Having this example block will allow me to adequately test the different scenarios that need to be coverd by the LazyLoad component's script-loader module.

I also need to have the script-loader actually handle the inline scripts and translations.

Translations is currently blocked in some capacity by a question I've put into the REST API PR about it. Basically core currently does a whole lot of zhuzhing to get translations out of the WP_Dependency object and into an actually usable JS snippet and the result of that isn't currently available from the REST API. That means we either have to a) add the result of the same process to the REST API or b) duplicate the process on the frontend. I'm no fan of duplication, so my preference is option (a) but I'm not sure if that's the approach everyone will be on board with.

What do we need from the backend?

The backend is more complicated.

First thing we need to do is abstract what I did for TinyMCE into something generic that can run for any block type. Namely, we need to detect whether a block's dependencies need to be pre-loaded, for example in the case covered above where the block is being used on the page. At least this is a little simpler than TinyMCE because TinyMCE also had the edge cases for custom meta boxes and the like. Should we include an optional hook in the backend block registration API should_pre_load_dependency that is checked when we detect a lazy block?

Which brings me to the next question: I'd like to avoid trying to the automagical version of this right now and to instead do a lower-effort experimental first-pass which would require block developers to manually wrap the lazy parts of their blocks with the LazyLoad component.

For that to work, we'd need some way for the backend to know that a block is lazy, so I'd like to add an experimental lazy option to blocks that we could key that on. This will require blocks to doubly declare their dependencies, once in the LazyLoad props and once in the block assets. But that doesn't seem so bad for an experimental feature, in my opinion. I'm open to suggestions for alternatives to this.

When a post is loading, in the same hook I'm currently handling TinyMCE's pre-loading, we'll do the following:

  1. Gather all blocks for which the experimental lazy attribute is true.
  2. For all those blocks, check which ones are currently used in the post being loaded.
  3. For all that are being used, pre-load the assets. Potentially we'll want to add an explicit enqueue_assets function that lazy blocks would need to declare? If so we would call that. Otherwise we can do a naïve version where we just enqueue the script assets.

All new properties and functions added to block registration should be have an experimental prefix added so that we can continue to iterate on these concepts and move towards a cleaner API.

Wait, what about the Block Directory?

Oh yes, the block directory. For a moment I thought we didn't need to handle styles, which is why the LazyLoad draft PR currently does not take them into account. For now, I'd like to put off dealing with styles simply to reduce the scope of the eventual PR which looks like it's going to be massive. However if we want to unify the front-end asset loading behavior between the block directory and this lazy loading approach, we do indeed need to handle styles in all of the same ways as scripts. I think that I've written the script-loader module in such a way that it could be easily abstracted into a corresponding style-loader. If anyone has tips or hints about this given what's up so far I'd greatly welcome them.

Okay, so then what about TinyMCE?

Well, TinyMCE is it's own special beast with it's own edge cases and approach to solving this problem. I'd love for some tech-debt clean up work to eventually lead to the unification of the TinyMCE solution and the more general solution, but that's a long ways off.

youknowriad commented 4 years ago

@saramarcondes Thanks for you all your efforts on this subject which is a big one and an important one. It does seem like you're making progress on the right path. I have a few questions:

Thanks.

sarayourfriend commented 4 years ago

Thanks for the questions @youknowriad

Do you want to use an existing block for the v0 or build a custom block only used for this to prove the contept?

I think a real-life example is always best. But I am honestly open to either, I have no strong preference. There are no core blocks that really need this other than the classic block (which needed its own special solution) so I don't believe we would use a core block. We could artificially introduce a new experimental block as the candidate, perhaps syntax highlighted code, given that particular block ran into this use case in the past and is a version of the simplest case scenario for this.

If it's an existing block, while the lazy loading is still experimental, will we be able to use the blocks on Core (packages release) without relying on the lazy loading?

The current implementation already includes a way to by-pass the laziness: simply preload the dependency. Then the cache will see that there's nothing to load and the child component will render immediately. We could do this through a feature flag, Gutenberg setting, or potentially develop the LazyLoad component as a spearate Gutenberg plugin that could be turned on or off.

Gutenberg code ship as packages, this adds a level of complexity. When I tried implementing lazy-loading in the project (a long time ago), I faced an issue where an npm package can't have "lazy loading" in it because lazy loading depends on a public URL in general which is present for applications but not for packages? Do you have some thoughts on that? Did you face this issue?

I'm not sure I understand what you're asking in the context of the part I am addressing which is block asset laziness rather than laziness as understood by an dynamic import('@wordpress/some-package') for example.

We shouldn't run into these issues because the LazyLoad component simply makes requests against the REST API to get a dependency tree and then inserts script tags. There should be no implications for package management at all. In fact, I placed the utility component simply inside the existing blocks package as it can be see as "just another function" the API exports.

The only potential complexity here is that blocks need to be designed with a "library" mindset so that they can be pushed into an enqueuable script.

Do you have a rough idea of what the API looks like for a lazy-loaded block? I mean can you share some pseudo-code for a lazy-loaded block?

For the experimental first pass (which is as far as I'm willing to think in any concrete terms) it could look like this:

// block.json
{
    // ... all the normal stuff
    "supports": {
        // ...
        "__experimentalLazy": true
    },
    "__experimentalLazyScriptDependencies": [ /* array of WP_Scripts handles */ ]
}

__experimentalLazy is a placeholder for whatever experimental name we'd want to give this. If a block does not use register_block_type_from_metadata then it would need to also define that when calling register_block_type. Calling one of the PHP register_block_type functions would be necessary to take advantage of the laziness as the server must know about it to be able to pre-load the __experimentalLazyScriptDependencies handles.

All the handles in __experimentalLazyScriptDependencies would need to be registered script handles by the block's plugin.

// my-lazy-block/edit.js
import metadata from './block.json';

const Edit = () => <Fragment/>;

const LazyEdit = ( props ) => (
    <LazyEdit
        scripts={ metadata.__experimentalLazyScriptDependencies }
        onLoaded={() => window.myLazyBlocksDependency.init()}
        // various a11y props to support `speak`ing loading states
    >
        <Edit {...props} />
    </LazyEdit>
);

export default LazyEdit;

It's that simple. Then, when the editor is loading, we can inspect the set of blocks used in the post being loaded and pre-load dependencies for lazy-loaded blocks by calling wp_enqueue_script with each of the handles in __experimentalLazyScriptDependencies. Then, rather than enqueueing the scripts, lazy blocks would simply depend on that.

An example block that might be a good candidate is the Map block from Jetpack. It uses mapbox-gl and dynamically imports it here. It's also generally an absolutely massive block with lots of code. One thing that could be done is to use LazyLoad to lazily-load the mapbox-gl dependency. Alternatively, for an even bigger win, you could lazily-load the entirety of the block's JavaScript by publishing it as a separately built package that, when run, defines a window variable. For example:

// blocks/map/block.json
{
    // ...
    "supports": {
        "__experimentalLazy": true
    },
    "__experimentalLazyScriptDependencies": [ "jetpack-blocks-map-edit" ]
}
// blocks/map/index.js
import metadata from './block.json';

edit: ( props ) => (
    <LazyLoad
        scripts={ metadata.__experimentalLazyScriptDependencies }
        a11yLoadedMessage={ __( "The map block has finished loading." ) }
        a11yLoadingMessage={ __( "The map block is loading." ) }
        a11yLoadingFailedMessage={ __( "The map block failed to load. Please try again." ) }
    >
        <window.JetpackBlocksMapEdit {...props} />
    </LazyLoad>
)
// blocks/map.php 
// https://github.com/Automattic/jetpack/blob/master/extensions/blocks/map/map.php#L31

// ...

function register_block() {
    wp_scripts()->add( 'jetpack-blocks-map-edit', /* ... */ );

    jetpack_register_block(
        BLOCK_NAME,
        array(
            'render_callback' => __NAMESPACE__ . '\load_assets',
        )
    );
}
add_action( 'init', __NAMESPACE__ . '\register_block' );

// ...

Blocks with editorScript may also be candidates for this type of lazy-loading, however I couldn't figure out how that block metadata is actually used. If anyone can point me in the right direction with that I'd love some pointers. Maybe __experimentalLazyScriptDependencies duplicates it?

youknowriad commented 4 years ago

Thanks for the replies, it's way clearer for me now at least.

perhaps syntax-highlighted code, given that particular block ran into this use case in the past and is a version of the simplest case scenario for this.

We did remove the syntax highlighting from the code block (coremirror) specifically because of lazy-loading. It definitely sounds like a good fit. I wouldn't mind a testing block either to start.

I'm not sure I understand what you're asking in the context of the part I am addressing which is block asset laziness rather than laziness as understood by an dynamic import('@wordpress/some-package') for example.

Let me try to explain more.

Right now when a third-party npm user does (there are some examples of this, the obvious one is asblocks or the playground built-in into Gutenberg's Storybook)

import { registerCoreBlocks } from '@wordpress/block-library';

registerCoreBlocks();

The expectation is that Core blocks work (at least most of them because we already have some issues with blocks relying on the Rest API that we want to fix). If tomorrow, the code block is refactored to look like that

import metadata from './block.json';

const Edit = () => <Fragment/>;

const LazyEdit = ( props ) => (
    <LazyEdit
        scripts={ metadata.__experimentalLazyScriptDependencies }
        onLoaded={() => window.myLazyBlocksDependency.init()}
        // various a11y props to support `speak`ing loading states
    >
        <Edit {...props} />
    </LazyEdit>
);

It is clear that the block will breaks because there's no REST API in these contexts. In my ideal scenario, when consuming the blocks by default from npm, you should always get the full block with the full npm dependencies but when you consume it on Core, lazy-loading is "enabled" somehow.

On my initial explorations about lazy-loading, I tried solving this issue by introducing the LazyLoad component at build time on Core/Gutenberg using a babel-plugin instead of hard-coding it in the code directly. For instance on my block I'd have

import CoreMirror from 'codemirror';

And on my babel config, I could define what dependencies should be lazy-loaded and what WP handles correspond to these scripts, so I had a config like this

const babelConfig = {
   'coremirror': 'wp-core-mirror'
}

so whenever that dependency was found the LazyLoad component was injected. I abandonned the idea at that time because the WordPress script loader (which was used to power the idea) only supported the default scripts but It seems like the new REST API solves that blocker.

Here's the old PR for this for more context https://github.com/WordPress/gutenberg/pull/8017

That said, maybe it's too complex, how worth it is it to ensure npm consumption of these packages. While I do believe lazy-loading like you propose is very interesting for third-party plugins without the dynamic injection, I think for Core it's more tricky. There's already some expectations that core blocks can be registered in JS without any server.

sarayourfriend commented 4 years ago

Ah, this makes a lot of sense @youknowriad thank you for that explanation. I definitely had not considered the non-server or even non-WordPress setting for Gutenberg for this 🤦‍♀️

The first thing that comes to mind is to implement some sort of strategy pattern of "dependency" provider that is required by the block with the default provider being a passthrough which essentially assumes that the dependency will already exist on the window. Then it is up to the consumer whether they wish to lazily-load the dependency by providing an async provider or to load it some other way.

I think that sort of abstraction could work in the current direction I'm moving in with the LazyLoad component by adding a provider prop to replace cache and that would also implement loadScripts. For example:

const LazyLoad = ( {
    provider,
    scripts,
    children,
    placeholder,
    onLoaded,
    onError,
    a11yLoadingMessage,
    a11yLoadedMessage,
    a11yLoadingFailedMessage,
} ) => {
    const [ loaded, setLoaded ] = useState( false );

    const scriptsToLoad = provider.getScriptsToLoad( scripts );

    useEffect( () => {
        // https://juliangaramendy.dev/use-promise-subscription/
        let isSubscribed = true;

        if ( loaded || scriptsToLoad.length === 0 ) {
            return;
        }

        speak( a11yLoadingMessage );

        provider.loadScripts( scriptsToLoad, cache )
            .then( async () => {
                // ...
            } )
            .catch( ( e ) => {
                // ...
            } );

        return () => ( isSubscribed = false );
    }, [ loaded ] );

    // ...
};

Then a synchronous provider could return an empty array for getScriptsToLoad, implying there is nothing to load.

However, what is not clear to me is how to effectively pararmeterize this provider as a parameter of the block. Is it even possible? Would we need to do some magic to make that work?

sarayourfriend commented 4 years ago

Thinking about this some more, I think rather than introducing a new __experimentalLazyScripts property, it would be more expedient to just use editor_script, considering that will provide the lowest uplift for existing blocks and are scripts that are already registered in a way that we understand.

I don't think we should introduce a new concept for that. I'm a little confused at the moment about how editor_script should work, however. I'm struggling to find in the codebase where these handles actually get enqueued. But according to https://developer.wordpress.org/block-editor/tutorials/block-tutorial/writing-your-first-block-type/ I should just have to register the script and add it as the editor_script to be enqueued right? Is that documentation out of date?

youknowriad commented 4 years ago

So I think that editor_script basically refers to the script that will register the block and that is loaded on the editor. So while it's close to what we're looking for here, If we do want to split things say registration and edit function, it might not serve that purpose right now.

sarayourfriend commented 4 years ago

@youknowriad Do you think it would be worthwhile to revisit your original solution over the one I've proposed? Given what you know of each approach, do you have a sense of whether one will be better off long term? Trying to make sure my energy in this is spent wisely.

I've been thinking more over the weekend and I think if I continue down the path I'm proposing I would go down the strategy pattern path and implement a provider prop that would be passed by the editor itself rather than into LazyLoad. We'd also have LazyLoad automatically wrapping edit functions on blocks that declare themselves __experimentalLazy. Then the lazinessProvider (or some other name) would be passed as part of settings to the editor initialization. Then the WordPressLazinessProvider would be what I'd been working on, and a similar NPMLazinessProvider could be implemented as an option that uses something like an asynchronous import or React's lazy/suspense. The lazinessProvider would get passed to LazyLoad which would wrap the blockType.edit in block-edit when rendered at the end of the function (for lazy blocks).

The only problem with this is that I'm not sure the static analysis required to make something like the NPMLazinessProvider would be possible. It should be possible to provide, for example, a simpler CDNLazinessProvider. Not totally sure what kinks would need to be sorted out to make importing from npm directly possible. It certainly wouldn't happen transparently/automagically as it seemed like your original solution would be able to support.

youknowriad commented 4 years ago

I know it's not helpful but the answer is really I don't know.

NPM registry supposes that everything just works out of the box, which means the dependencies are static there import something from "something". Anything removing that means we're giving up on npm support for blocks and at the moment I don't really see the provider API proposal as a solution for that tbh.

That said, I do wonder how important is it for us to support these use-cases. For WordPress, it is definitely not important but are we ok giving up on tools like asblocks using core blocks, drupal Gutenberg using core blocks...

Maybe others have thoughts here @mcsf @mtias

sarayourfriend commented 4 years ago

I don't really see the provider API proposal as a solution for that tbh

Agreed. I'll take some time this week to revisit your previous attempt and try to revive it.

One thing to note though is that the classic block currently depends silently and heavily on WordPress specific window variables. Would we want to slate work to eventually untie the classic block from WordPress's loading behavior or alternatively should we put the classic block into it's own package?

youknowriad commented 4 years ago

Would we want to slate work to eventually untie the classic block from WordPress's loading behavior or alternatively should we put the classic block into it's own package?

Yes, there are some blocks that are very WP-dependent, and Classic block is one of them. At some point we should try to separate these blocks from generic ones.

kevin940726 commented 3 years ago

The React core team recently shared their top-level overview of how SSR would look like in React 18: https://github.com/reactwg/react-18/discussions/37

It might worth a read, and be taken into consideration when designing our async loading strategy.

dmsnell commented 3 years ago

hi @kevin940726 - thanks for the link. was there something specific in there that you thought was important for this issue?

kevin940726 commented 3 years ago

was there something specific in there that you thought was important for this issue?

Nothing specific, it's just a reminder 😅 . since SSR in React will get a significant update, I just want to make sure that we're leveraging the best practices and not re-inventing the wheels. :)

dmsnell commented 3 years ago

Thanks for sharing @kevin940726 - I don't think these two systems will overlap much. I'm not even sure there's any likely scenario in which we would apply SSR to this stage of Gutenberg loading, or if that would be practical/possible/beneficial.

kevin940726 commented 3 years ago

It's not only related to SSR though, React 18 will also include Concurrent Rendering and Suspense etc. These are not separate features though, they are related to each other and we should pay attention to how we integrate them into Gutenberg.

For instance, we should really use React.lazy and <Suspense> for asynchronously loading components by default, rather than creating our own <LazyLoad> component. We could potentially also get partial hydration working if we do things right.

That said, it's just a reminder. I don't know how much it applies to our architecture yet. But let's just keep an eye on it. Maybe it doesn't have anything to do with this issue, then we can just ignore my comments 😛.

gziolo commented 1 year ago

@tyxla started PR #48315 with an initial experiment that approaches async block loading in a radically simple way.

tyxla commented 1 year ago

Indeed, @gziolo, although that PR still needs some polishing and exploration. It is indeed meant to be a very simple way to deal with the async loading of blocks, likely serving temporarily until we have a better solution.

I think we still should explore using JS modules and import maps as a more robust way to manage scripts and styles through a better long-term solution for WordPress as a whole.

tyxla commented 1 year ago

@jsnajdr is experimenting with async block loading in this PR and has kicked off a discussion about the approaches and decisions there - it could definitely use some additional eyes and feedback.

gziolo commented 1 year ago

Update

@westonruter suggested using a different phrase than "async loading" in https://github.com/WordPress/gutenberg/discussions/53260#discussioncomment-6870404. We concluded that "Lazy Loading" fits best and aligns with how people refer to it in the JavaScript ecosystem. I updated the title to reflect that.

Last week, @jsnajdr landed https://github.com/WordPress/gutenberg/pull/53807 with refactoring to the block registration as a preparation for the follow-up work to allow lazy loading parts of the block definition.

I started looking in https://github.com/WordPress/wordpress-develop/pull/5118 at ways in WordPress Core to automate consuming shared scripts generated in the build pipeline. My main motivation was to figure out a way to fit the concepts of code splitting (runtime chunk, split chunks, dynamic imports) into the existing API shaped around wp_register_script for blocks.