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

[WIP] Handle All Asset Registration and Enqueueing For Blocks #5084

Closed
wants to merge 1 commit into from

Conversation

Shelob9
Copy link
Contributor

@Shelob9 Shelob9 commented Feb 15, 2018

Related: #2751 #4514 #2756 #4841

Enhancement Overview

After my conversation with @youknowriad and @davidbhayes I really think that block registration should take the handle and url for assets. The block registration system (not WP_Block_Type_Registry, which as several concerns already) should be responsible for registering and enqueuing CSS and JavaScript for blocks.

Description

This PR, if completed, will allow core or plugins or hosts to take alternative approaches in the future to dealing with problems such as:

  • Multiple plugins importing the same library (Vue, Axios, etc) into their wepback bundles.
  • Multiple plugins, loading multiple blocks, each block adding 1-4 HTTP requests. Even with HTTP/2 this can become a performance issue. Also, WordPress doesn't require HTTPS, so this is a major problem for many users.
  • Async loading of block assets.

I've prepared a quick PR of psuedo-code for what I'd like to propose. This isn't tested, I did not test it at all, or even include the files (sorry so used to autoloaders,) this is just a concept for discussion. I'll happily make it work, and provide unit tests and run PHP_CS on it if there is interest in this PR.

Proposed Change to register_block_type()

In the handbook's example:

    wp_register_script(
        'gutenberg-boilerplate-es5-step01',
        plugins_url('step-01/block.js', __FILE__),
        array('wp-blocks', 'wp-element')
    );
    
    register_block_type('gutenberg-boilerplate-es5/hello-world-step-01', array(
        'editor_script' => 'gutenberg-boilerplate-es5-step01',
    ));

Would change to:

    register_block_type('gutenberg-boilerplate-es5/hello-world-step-01', array(
        'editor_script' => array(
            //Handle for block JavaScript that is editor only.
            'handle' => 'gutenberg-boilerplate-es5-step01',
            //URL for block JavaScript that is editor only.
            'url' => plugins_url('step-01/block.js', __FILE__)
        ),
        'editor_style' => array(
            //Handle for block CSS that is editor only.
            'handle' => 'gutenberg-boilerplate-es5-step01',
            'url' => plugins_url('step-01/block.js', __FILE__)
        ),
        'script' => array(
            //Handle for block JavaScript that is for front-end and editor
            'handle' => 'gutenberg-boilerplate-es5-step01-front-end',
            //URL for block JavaScript that is for front-end and editor
            'url' => plugins_url('step-01/front-end.js', __FILE__)
        ),
        'style' => array(
            //Handle for block CSS that is for front-end and editor
            'handle' => 'gutenberg-boilerplate-es5-step01-front-end',
            //URL for block CSS that is for front-end and editor
            'url' => plugins_url('step-01/front-end.js', __FILE__)
        ),
    
    ));
) );

Goals:

  • Make core entirely responsible for handling block asset loading for non-core blocks.
    • Less code repetition. 3rd-party developers currently have to write a lot of PHP boilerplate for asset loading. This is inefficient.
    • Can be improved without worrying about backwards-compat. The current documented pattern/ best practice can't change without causing a major disruption for a lot of plugins and themes.
  • Make the interaction with WP_Dependency automatic by default, and totally extensible.
    • Sensible default by core, using existing APIs.
    • A plugin/ site/ host/ future version of WordPress should be able to provide alternative implementations. For example, a plugin could minfify CSS and JavaScript automatically, or create custom bundles, or integrate with a CDN, or do nothing and let a clever use of webpack solve these problems a totally different way.

What I've Done

  • Created a class for registering assets:
    • WP_Block_Assets_Enqueue handles registering the block's assets.
    • It implements WP_Registers_Block_Assets this interface is important as the factory is written so you can provide own WP_Registers_Block_Assets.
    • It's responsibility is to call wp_register_script/style at the right time.
    • A plugin may provide an object that implements WP_Registers_Block_Assets on the pre_register_block_assets filter if they wish to optimize the process in some way.
  • Created a class for registering assets:
    • WP_Block_Assets_Enqueue handles registering the block's assets.
    • It implements WP_Enqueues_Block_Assets this interface is important as the factory is written so you can provide your own WP_Enqueues_Block_Assets.
    • It's responsibility is to call wp_enqueue_script/style at the right time.
    • A plugin may provide an object that implements WP_Enqueues_Block_Assets on the pre_enqueue_block_assets filter if they wish to optimize the process in some way.
  • In WP_Block_Type_Registry I've implemented this new system, so that all hooks are added, by default.

Everything?

This is the current best practice:

    wp_register_script(
        'gutenberg-boilerplate-es5-step01',
        plugins_url('step-01/block.js', __FILE__),
        array('wp-blocks', 'wp-element')
    );
    
    register_block_type('gutenberg-boilerplate-es5/hello-world-step-01', array(
        'editor_script' => 'gutenberg-boilerplate-es5-step01',
    ));

With this Gutenberg has everything it needs to call wp_enqueue_script at the right time, with the right handle. So, you could argue this is all we really need. So why did I make this new system do everything? I think WP_Dependency is deficient for this task. Yes, I'm using it here, but I'd like to have a layer above it, so plugin developers can use a system that includes the following concepts:

  • Treeshacking
  • A tool to scan all blocks package.json to provide the most optimized builds, detect conflicts, and create 1 single entry point for all blocks, custom or not or something, I don't know webpack that well.
  • JavaScript modules and imports
  • Asynchronous asset loading
  • ServiceWorkers

It's probably best not to use wp_register_script but instead, create something totally new. By wrapping the current dependency Gutenberg is creating - that it has no way to modify later - in WP_Dependency we're not tied long-term to a system that was designed way back when we wrote JavaScript by opening up a file and writing JavaScript and then saving it. And people still do that, and that's fine, WP_Dependency is fine for that.

Discussion Questions

  • Is there any interest in this at all?
  • We're putting Redux and webpack in core, but we don't have a PHP autoloader?
  • Should WP_Block_Type_Registry track the factory and objects the factory creates? This is needed for making hooks removable,

How Has This Been Tested?

Not at all. This code breaks things as-is, just wanted to discuss.

Checklist:

  • My code is tested.
  • My code follows the WordPress code style.
  • My code has proper inline documentation.

@Shelob9
Copy link
Contributor Author

Shelob9 commented Feb 15, 2018

Pinging @aduth and @matias for feedback

Also, related WP-CLI issue: wp-cli/scaffold-command#106

@Shelob9
Copy link
Contributor Author

Shelob9 commented Feb 16, 2018

Proposal to add service worker registration to core: https://core.trac.wordpress.org/ticket/36995#comment:7

@aduth
Copy link
Member

aduth commented Feb 19, 2018

I'm having a bit of trouble following the benefits provided by the proposal here. Following the discussion in "Everything?", I still don't see what's not possible about implementing asynchronous loading for blocks and block dependencies using the built-in script loader dependency graph. There will certainly need to be some more tooling around this, but it seems very much within the realm of reasonable for the client to request a given script handle and know how to load it (including which dependencies are required, already present, etc).

The proposed changes to register_block_type seem to reinvent the wheel. It's also not clear how one would define additional dependencies? The wp_register_script implementation might be slightly more verbose (in that it requires separate function calls), but is familiar and accommodates all of these requirements already.

@danielbachhuber
Copy link
Member

Closing for now. Let's continue discussion on an issue if this is still relevant.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants