Make WordPress Core

Opened 3 months ago

Closed 3 months ago

Last modified 3 months ago

#60240 closed defect (bug) (fixed)

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

Reported by: luisherranz's profile luisherranz Owned by: youknowriad's profile youknowriad
Milestone: 6.5 Priority: normal
Severity: major Version:
Component: Script Loader Keywords: has-patch has-unit-tests needs-testing-info
Focuses: javascript Cc:

Description

In the ticket that introduced the 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.

Change History (13)

#1 @luisherranz
3 months ago

  • Summary changed from Modules API: Move the modules to the footer in classic themes to Script Modules API: Move the modules to the footer in classic themes

#2 @luisherranz
3 months ago

I've prepared this PR, but it's built on top of the Script Modules API: Rename `wp_module` functions to `wp_script_module` one, so I'm waiting until that one is merged to move it to /WordPress/wordpress-develop:

This ticket was mentioned in PR #5931 on WordPress/wordpress-develop by @luisherranz.


3 months ago
#3

  • Keywords has-patch has-unit-tests added

---

## 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: enhancement: Add the ability to handle ES Modules and Import Maps (reopened #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:

This ticket was mentioned in Slack in #core-test by ankit-k-gupta. View the logs.


3 months ago

#5 @oglekler
3 months ago

  • Keywords needs-testing-info added
  • Type changed from defect (bug) to enhancement

This ticket was discussed during but scrub.

This looks like an enhancement, so, I am adjusting ticket's type.

@luisherranz can you, please, provide testing instructions? It will be much easier for testers. Thank you in advance 🙏

Add props to @ankit-k-gupta

#6 @luisherranz
3 months ago

Oh, sure.

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.
2- Check that the scripts with type="importmap" and type="module", and the link with rel="modulepreload" are printed in the footer.

@luisherranz commented on PR #5931:


3 months ago
#7

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.

#8 @youknowriad
3 months ago

  • Owner set to youknowriad
  • Resolution set to fixed
  • Status changed from new to closed

In 57345:

Script Loader: Load the modules to the footer in classic themes

Incremental import maps fail if the import map is printed after the module scripts.
This means, we should always render import maps first. This means that for classic themes, we need to move the import map and modules to the footer because we can't know before that which modules are needed.

Props luisherranz, cbravobernal.
Fixes #60240.

#9 @youknowriad
3 months ago

  • Milestone changed from Awaiting Review to 6.5

#11 @youknowriad
3 months ago

  • Type changed from enhancement to defect (bug)

This ticket was discussed during but scrub.

This is a bug fix for me as otherwise modules don't work correctly if loaded after the import map.

@cbravobernal commented on PR #5931:


3 months ago
#12

I tested and is working as expected.

Trunk screenshots:
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/37012961/4ae1bddb-ae96-4aff-80f8-6c92282fd629
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/37012961/dc7d07f6-8659-417a-9d7e-9f0c6840eb51

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

PR applied screenshots:
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/37012961/cb6d89c3-ced6-496f-b392-c09218e32a7c
https://i0.wp.com/github.com/WordPress/wordpress-develop/assets/37012961/8a8a2d34-3e07-4053-ae9a-627011cc5f48

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

@luisherranz commented on PR #5931:


3 months ago
#13

Thanks, folks!

Note: See TracTickets for help on using tickets.