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

Asynchronously load TinyMCE (when possible) #23068

Closed
wants to merge 9 commits into from
Closed

Asynchronously load TinyMCE (when possible) #23068

wants to merge 9 commits into from

Conversation

sarayourfriend
Copy link
Contributor

@sarayourfriend sarayourfriend commented Jun 10, 2020

The problem

TinyMCE is a huge dependency that is used rarely in the context of the block editor. Primarily it is used to support the classic block and some meta boxes. In total (TinyMCE core and plugins) it costs 272.274KB in compressed trasferred JS and 1,007.209KB parsed (that's over a MB in parse!). This is a big cost to pay for something that many users will never interact with.

The solution I'm exploring in this PR is to offset loading TinyMCE as often as possible until it is needed. Given certain edge cases and exclusions, this would offset the cost of loading TinyMCE except in the following case, which are the minority of use cases:

  1. We cannot do this when non-backwards compat meta boxes are installed as they may depend on TinyMCE (as in the case of Advanced Custom Fields, for example).
  2. We cannot do this when a classic block is already being used on a post (for reasons described in this comment Block and Component Lazy Loading #2768 (comment) in the "Problems with blocks already used on a post" section).

Otherwise, when the classic block has not already been added to a post or when custom meta boxes aren't installed, we can offset the TinyMCE dependency until the block's edit is run. Indeed, in this case, if someone never uses the classic block, they will not pay the penalty for having it.

There was an existing draft PR for asynchronously loading TinyMCE that built upon an existing PR in Gutenberg that introduced a REST API for retrieving a list of dependencies. This REST API isn't technically necessary for asynchronously loading TinyMCE for the classic block, it was included in the draft PR as a way of looking forward to enabling similar async behavior in other blocks.

However, the REST API itself presents some complexities and concerns:

  1. REST calls to WP are expensive as all of WP needs to spin up, and in the current implementation it could require more than one call if someone ran with the experimental version.
  2. While the REST API does return inline scripts, TinyMCE’s inline scripts are complex and varied (there are three, not all are "registered" in WP_Scripts).

Because we can get the URL for TinyMCE without using a REST API (partially it's already available in the tinyMCEPreInit object but this won't be sufficient as I describe below) I think we can shortcut getting a win against TinyMCE around the REST API.

To accomplish this async TinyMCE project, I propose the following:

  1. Creat a LazyLoadTinyMCE component to wrap the ClassicEdit component.
    • This LazyLoadTinyMCE component is basically a TinyMCE specific version of the the solution described in this PR.
  2. Cutting TinyMCE from the initial pageload of the block editor whenever possible.

TinyMCE URLs

There is one caveat about using tinyMCEPreInit.baseURL is that we need to decide which TinyMCE script to use as different scripts are used for different environments and compression settings. I’m not totally sure how to get this information to the frontend. One potential solution is to use $wp_scripts->registered['wp-tinymce']->deps and then follow a strategy similar to get_url in the REST API PR to retrieve the URI for the wp-tinymce scripts and then add those to an array the frontend can use. Adding functionality like get_url to WP_Scripts directly would be good for making the functionality available generally. It could alternatively be added as a utility function in Gutenberg's plugin, but in any case, it would need to be available outside the REST API.

Stopping WordPress from equeueing TinyMCE

Along with that, we also need to be enable Gutenberg to prevent core from enqueuing the TinyMCE scripts. To do that we ought to move the printing of the editor scripts into an action that can be overwritten by the plugin. This trac ticket proposes that change: https://core.trac.wordpress.org/ticket/49964

Edge cases/exclusions

Meta boxes

As stated above, there are some exceptions to when we can do this. Meta boxes present a backwards compatability issue as some of them depend directly on TinyMCE. Meta box development hasn’t stopped and widely used plugins like Advanced Custom Fields depend on them. Preserving the ability for meta boxes that depend on TinyMCE to continue to work is part of the work for v1. We’ll do this by disabling async TinyMCE when we detect that custom meta boxes are being used.

From what I understand at the moment edit-form-blocks.php enqueues the editor scripts before processing metabox data. Ideally we would like to have already run through processing meta boxes before we render scripts for the editor so that we have some basis for deciding whether meta boxes are really being used. register_and_do_post_meta_boxes takes care of processing meta boxes for a post. I think we can move this logic before the call to wp_enqueue_editor in edit-form-blocks.php without consequence and then look into the global $wp_meta_boxes in the action we'll add in Gutenberg to override TinyMCE script printing.

When the classic block is already used on a post

When a post is first loaded into the block editor, the edit for each block gets run. This means that when a classic block exists on a post, TinyMCE will be needed immediately on page load. Asynchronously loading TinyMCE here isn't possible because we need to render the block's contents into the editor. I think we should solve this by cutting this from the scope of the async TinyMCE project and instead solve it as part of the wider project to async all block dependencies whenever possible (if indeed it is a solvable problem).

We'll need to introspect into a posts post_content and decide whether we think the classic block is being used. There already exists a function has_blocks, however it relies on the block's boundaries being renderedinto the post_content and unfortunately, the classic block doesn't render block markup comments. So the only way I can think of right now to do it is to run the post through parse_blocks and then walk the block tree and see if a core/freeform block exists.

Summary of changes

To summarize, we need to make changes to WordPress core and Gutenberg.

  • Core
    1. Devise a way to detect whether meta boxes that use TinyMCE are present that would be usable the Gutenberg plugin so that it can decide when to follow the default path of eagerly loading TinyMCE.
    2. Start injecting the TinyMCE dependency URIs into the frontend.
    3. Move printing editor scripts into an action overridable by the Gutenberg plugin so that we can prevent TinyMCE scripts from being injected when meta boxes are not present and when the classic block is not already being used for the post being edited.
  • Gutenberg
    1. Create a version of the LazyLoad component that is able to load dependencies from URIs and that does not use the REST API.
    2. Prevent TinyMCE from being loaded by overriding the action we created in core. Also print translation initialization into a JS function to be executed by LazyLoadTinyMCE once the dependency is loaded.
    3. Wrap the classic block’s edit component with LazyLoadTinyMCE.

Completing the above will deliver a significant decrease in JavaScript download and parse times for most Gutenberg users.

Alternatives

We could move TinyMCE into the block directory, however then every post would pay the cost of the dependency, regardless of whether it was going to be used, so I don't think this is an adequate solution.

Alternatively, we could move it into the block directory and then have the block directory offset loading a block's dependencies until it is edited... but that's just the other more general solution documented in #2768.

What this PR actually does

This PR adds a LazyLoadTinyMCE component that wraps the classic block. This component delays the rendering of the classic block's edit component until TinyMCE has been loaded.

The PR depends on changes proposed in WordPress/wordpress-develop#232.

On a clean WordPress and Gutenberg install, this offsets approximately 272.274KB in transfer and 1,007.209KB parsed (that's over a MB in parse!) after TinyMCE itself and all the various plugins.

How has this been tested?

Using a WP installation running the changes in WordPress/wordpress-develop#232:

  1. Load the editor for both a post that does and does not have a classic block on it and verify that TinyMCE scripts were not requested on the initial page load by checking the network tab in the browser's development tools
  2. Add a classic block to a post that does not already have a classic block. Verify that when this is done that:
    1. Request(s) for the TinyMCE script(s) are made
    2. window.tinymce exists
    3. The classic block is now editable
    4. Aside from the delay in the block being editable, that there are no differences in behavior from master
    5. Changes to the classic block's content are preserved and saved
    6. Subsequent classic blocks added or edited to no re-request the TinyMCE scripts
  3. Open a post that already has a classic block.
    1. Verify the classic block rendering matches master's behavior (the "Classic" bar appears above the content of the classic block)
    2. Verify you can edit the classic block.
    3. Verify you can add new classic blocks.
    4. Verify that there are no extra requests to retrieve TinyMCE aside from the initial page load.
  4. Add a meta box that does not declare __back_compat_meta_box in its args (using ACF for example) for a specific post type. Edit and create a new post of that post type and verify that TinyMCE is loaded on page load and not re-loaded when a classic block is added or edited.
  5. Add a meta box as described in (4) but then edit a post type that does not have the custom meta box. Verify that TinyMCE is still asynchronously loaded and that the criteria from (2) still pass.

Types of changes

This is a new feature. The only user-facing change in behavior will be a delay between when a classic block is "edited" and when it is actually "editable".

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code follows the accessibility standards.
  • My code has proper inline documentation.
  • I've included developer documentation if appropriate.
  • I've updated all React Native files affected by any refactorings/renamings in this PR.

This component allows us to wrap existing components
and delay them until required 3rd party scripts and
styles are asynchronously loaded. It also provides a
hook for post-load setup that resolves before the
children are rendered, guaranteeing that the children
aren't active until the world around them is set up
correctly.
Later on we'll be able to expand this pattern generically, but for
now we'll keep it focused on TinyMCE.
Meta boxes _may_ depend on TinyMCE already being initialized.
Because of this, we cannot utilize asynchronous TinyMCE when
any "non backwards compat only" meta boxes are detected for the
current post type.
@sarayourfriend sarayourfriend changed the title Asynchronously load TinyMCE Asynchronously load TinyMCE (when possible) Jun 10, 2020
@sarayourfriend sarayourfriend marked this pull request as draft June 10, 2020 22:58
@diegohaz
Copy link
Member

diegohaz commented Jun 11, 2020

With respect to accessibility: I'm not sure what the implications are here. My gut says that whatever ends up being used for the placeholder needs to indicate that the block is being loaded, but I'm not sure how to do that. @diegohaz, would you have any suggestions about what kind of placeholder and what attributes it would need to have to make this accessibile?

One option would be setting aria-busy on the block wrapper component. I know that on role="group" elements, Safari/VoiceOver will announce loading when the attribute is set to true and loaded when it's set to false (or removed). But it doesn't seem to work on NVDA and JAWS.

So, I guess the safest solution here is to use the speak utility to announce the loading/loaded state.

We cannot delay loading TinyMCE in this case because when the editor
first boots up it renders each block's `edit` component. Because
this is expected to be synchronous, the classic block never gets a
chance to finish rendering its content and you end up with just the
placeholder block.
@sarayourfriend
Copy link
Contributor Author

Thanks for the tip @diegohaz! I took a shot at writing something appropriate, hopefully someone can confirm whether that kind of message is appropriate or if something else would be better.

@diegohaz diegohaz added [Block] Classic Affects the Classic Editor Block [Package] Block library /packages/block-library [Type] Performance Related to performance efforts labels Jun 29, 2020
@diegohaz diegohaz self-requested a review June 29, 2020 16:02
Base automatically changed from master to trunk March 1, 2021 15:43
@sarayourfriend
Copy link
Contributor Author

Closing in favor of #29681

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
[Block] Classic Affects the Classic Editor Block [Package] Block library /packages/block-library [Type] Performance Related to performance efforts
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants