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

Script Modules API: Move the script modules to the footer in classic themes #5931

Conversation

luisherranz
Copy link
Member

@luisherranz luisherranz commented Jan 23, 2024

Trac ticket: https://core.trac.wordpress.org/ticket/60240


What

Moves the script module prints to the footer in classic themes. It also removes the ability to call the print functions more than once because it's now unnecessary.

Why

In the ticket that introduced the Script Modules API (#56313), I added a logic that prints as many enqueued and preloads as possible in the head, then prints the remaining ones in the footer and includes the import map. I explain my reasoning ​in this comment.

But @cbravobernal realized that import maps fail if they appear after the modules. That means that there's no other way but printing the import map first, and then all the modules and preloads (yes, preloads also fail!). So we need to check if we are on a classic or block theme, and if we are in a classic theme, move everything to the footer.

How

By leveraging wp_is_block_theme() and moving everything to the footer in classic themes:

public function add_hooks() {
  $position = wp_is_block_theme() ? 'wp_head' : 'wp_footer';
  add_action( $position, array( $this, 'print_import_map' ) );
  add_action( $position, array( $this, 'print_enqueued_modules' ) );
  add_action( $position, array( $this, 'print_module_preloads' ) );
}

Additional comments

@westonruter mentioned that we could optimize this in the future if we have a way to filter the output before sending the response to the client:

Testing instructions

To check that this fixes the order of the import map:

  1. Create a new plugin with three files:
    // test-plugin/test.php
    <?php
    /*
    Plugin Name: Test Script Modules
    Version: 1.0.0
    */
    
    wp_register_script_module(
      'bar',
      plugins_url( '/bar.js', __FILE__ )
    );
    
    wp_enqueue_script_module(
      'foo',
      plugins_url( '/foo.js', __FILE__ ),
      array( 'bar' )
    );
    // test-plugin/foo.js
    import bar from "bar";
    
    bar();
    // test-plugin/bar.js
    export default function bar() {
      console.log("bar");
    }
  2. Activate the plugin.
  3. Open the site (frontend).
  4. Check that "bar" was printed in the console.

To check that this fixes the positioning of the scripts/link in the classic themes:

  1. Load a block theme.
  2. Check that the scripts with type="importmap" and type="module", and the link with rel="modulepreload" are printed in the head.
  3. Load a classic theme.
  4. Check that the scripts with type="importmap" and type="module", and the link with rel="modulepreload" are printed in the footer.

@luisherranz luisherranz changed the title Move all hooks to the footer in classic themes and remove multiple prints functionality Script Modules API: Move the script modules to the footer in classic themes Jan 23, 2024
Copy link

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

@luisherranz
Copy link
Member Author

FWIW, as I removed the ability to call the print functions more than once because it's now unnecessary, I also removed the test_print_enqueued_script_modules_can_be_called_multiple_times and test_print_preloaded_script_modules_can_be_called_multiple_times tests.

@youknowriad
Copy link
Contributor

@cbravobernal
Copy link
Contributor

I tested and is working as expected.

Trunk screenshots:
Screenshot 2024-01-24 at 11 42 32
Screenshot 2024-01-24 at 11 42 51

  • Is bar in console log ❌
  • There are errors on console ❌
  • The scripts are in the footer in a classic theme. ❌

PR applied screenshots:
Screenshot 2024-01-24 at 11 46 18
Screenshot 2024-01-24 at 11 46 12

  • Is bar in console log ✅
  • There are no errors on console ✅
  • The scripts are in the footer in a classic theme. ✅

@luisherranz
Copy link
Member Author

Thanks, folks!

cbravobernal added a commit to WordPress/gutenberg that referenced this pull request Jan 24, 2024
…ss-6.5 folder (#57778)

* Move modules api to 6.5 folder

* Update functions to use wp_modules instead of gutenberg_modules

* Prevent class redeclaration

* Add classic themes conditional

* Yet another script position movement

* Add gutenberg_*_module deprecations

* Add correct version deprecations

* Add correct version deprecations

* Use correct order for module hooks

* Rename module functions to script_module

* Rename module functions to script_module

* Update files from WP Core

Includes this PR WordPress/wordpress-develop#5931

* Fix deprecated version and rename experimental file

* Add warning to upcoming deleted file

* Refactor file to use only modules

* Refactor search block to use only modules

* Refactor query block to use only modules

* Refactor image block to use only modules

* Refactor navigation block to use only modules

* Fix some params and add missing DocBlock

* Update DEWP readme

* Remove tests

This code is now tested in WordPress Core.

* Update enqueue in new e2e tests

* Fix wrong print function names

* Remove unnecessary extra argument

* Add missing function

* Fix redeclared class problem

Fixes an issue in PHP 7.0 where we'd see a redeclared class fatal error.

* Remove we from docs

---------

Co-authored-by: Luis Herranz <luisherranz@gmail.com>
Co-authored-by: Jon Surrell <sirreal@users.noreply.github.com>
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