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.

Sign up for GitHub

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: Add method to include modules in importmap #8009

Closed
wants to merge 7 commits into from
Closed

Script Modules: Add method to include modules in importmap #8009

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
77f1193
Add "expose" logic to script modules
sirreal Oct 8, 2024
01cc5c4
Make get_src return nullable
sirreal Oct 8, 2024
a7ea9a9
Rework, rename, add tests
sirreal Dec 16, 2024
31ec071
Remove uniqueness checks
sirreal Dec 17, 2024
539e38b
Replace excessively prescriptive comment
sirreal Dec 17, 2024
751887b
Add test for correct merge with regular dependencies
sirreal Dec 17, 2024
d30c9de
Fix overwriting array keys
sirreal Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
72 changes: 62 additions & 10 deletions src/wp-includes/class-wp-script-modules.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,18 @@ class WP_Script_Modules {
*/
private $enqueued_before_registered = array();

/**
* Holds script module identifiers that have been marked for inclusion in the import map.
*
* A script module that appears here should be include in the import map regardless of
* whether it is in the dependency graph of enqueued script modules.
*
* @since 6.8.0
*
* @var string[]
*/
private $marked_for_inclusion = array();

/**
* Tracks whether the @wordpress/a11y script module is available.
*
Expand Down Expand Up @@ -148,6 +160,21 @@ public function enqueue( string $id, string $src = '', array $deps = array(), $v
}
}

/**
* Marks the script module for inclusion in the import map.
*
* This method is intended for use outside of the script module dependency system.
* It's recommended that script modules rely on the script module dependency system
* to manage the import map.
*
* @since 6.8.0
*
* @param string $id The identifier of the script module.
*/
public function include_in_import_map( string $id ) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if this method name is too specific and tied to the implementation.

Originally I had used expose() and was also considering make_available(). I'm wondering about a future where this may add some option arguments for preload, etc.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My first thought was add_to_import_map()

$this->marked_for_inclusion[] = $id;
}

/**
* Unmarks the script module so it will no longer be enqueued in the page.
*
Expand Down Expand Up @@ -208,10 +235,15 @@ public function add_hooks() {
*/
public function print_enqueued_script_modules() {
foreach ( $this->get_marked_for_enqueue() as $id => $script_module ) {
$src = $this->get_src( $id );
if ( null === $src ) {
continue;
}

wp_print_script_tag(
array(
'type' => 'module',
'src' => $this->get_src( $id ),
'src' => $src,
'id' => $id . '-js-module',
)
);
Expand All @@ -228,11 +260,16 @@ public function print_enqueued_script_modules() {
*/
public function print_script_module_preloads() {
foreach ( $this->get_dependencies( array_keys( $this->get_marked_for_enqueue() ), array( 'static' ) ) as $id => $script_module ) {
$src = $this->get_src( $id );
if ( null === $src ) {
continue;
}

// Don't preload if it's marked for enqueue.
if ( true !== $script_module['enqueue'] ) {
echo sprintf(
'<link rel="modulepreload" href="%s" id="%s">',
esc_url( $this->get_src( $id ) ),
esc_url( $src ),
esc_attr( $id . '-js-modulepreload' )
);
}
Expand Down Expand Up @@ -262,14 +299,29 @@ public function print_import_map() {
*
* @since 6.5.0
*
* @return array Array with an `imports` key mapping to an array of script module identifiers and their respective
* URLs, including the version query.
* @return array Array with an `imports` key mapping to an array of script
* module identifiers and their respective URLs, including
* the version query.
*/
private function get_import_map(): array {
$imports = array();
foreach ( $this->get_dependencies( array_keys( $this->get_marked_for_enqueue() ) ) as $id => $script_module ) {
$imports[ $id ] = $this->get_src( $id );
$imports = array();
$script_module_ids = array_merge( $this->marked_for_inclusion, array_keys( $this->get_marked_for_enqueue() ) );

foreach ( $this->get_dependencies( $script_module_ids ) as $id => $script_module ) {
$src = $this->get_src( $id );
if ( null === $src ) {
continue;
}
$imports[ $id ] = $src;
}
foreach ( $this->marked_for_inclusion as $id ) {
$src = $this->get_src( $id );
if ( null === $src ) {
continue;
}
$imports[ $id ] = $src;
}

return array( 'imports' => $imports );
}

Expand Down Expand Up @@ -335,11 +387,11 @@ function ( $dependency_script_modules, $id ) use ( $import_types ) {
* @since 6.5.0
*
* @param string $id The script module identifier.
* @return string The script module src with a version if relevant.
* @return string|null The script module src with a version if relevant.
*/
private function get_src( string $id ): string {
private function get_src( string $id ): ?string {
if ( ! isset( $this->registered[ $id ] ) ) {
return '';
return null;
}

$script_module = $this->registered[ $id ];
Expand Down
78 changes: 74 additions & 4 deletions tests/phpunit/tests/script-modules/wpScriptModules.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public function set_up() {
*
* @return array Enqueued script module URLs, keyed by script module identifier.
*/
public function get_enqueued_script_modules() {
private function get_enqueued_script_modules() {
$script_modules_markup = get_echo( array( $this->script_modules, 'print_enqueued_script_modules' ) );
$p = new WP_HTML_Tag_Processor( $script_modules_markup );
$enqueued_script_modules = array();
Expand All @@ -54,18 +54,20 @@ public function get_enqueued_script_modules() {
*
* @return array Import map entry URLs, keyed by script module identifier.
*/
public function get_import_map() {
private function get_import_map() {
$import_map_markup = get_echo( array( $this->script_modules, 'print_import_map' ) );
preg_match( '/<script type="importmap" id="wp-importmap">.*?(\{.*\}).*?<\/script>/s', $import_map_markup, $import_map_string );
return json_decode( $import_map_string[1], true )['imports'];
return isset( $import_map_string[1] )
? json_decode( $import_map_string[1], true )['imports']
: array();
}

/**
* Gets a list of preloaded script modules.
*
* @return array Preloaded script module URLs, keyed by script module identifier.
*/
public function get_preloaded_script_modules() {
private function get_preloaded_script_modules() {
$preloaded_markup = get_echo( array( $this->script_modules, 'print_script_module_preloads' ) );
$p = new WP_HTML_Tag_Processor( $preloaded_markup );
$preloaded_script_modules = array();
Expand Down Expand Up @@ -906,4 +908,72 @@ public static function data_invalid_script_module_data(): array {
'string' => array( 'string' ),
);
}

/**
* @ticket 61500
*/
public function test_included_module_appears_in_importmap() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In core, the best practice when using multiple assertions in a test is for each assertion to have a custom error message. Makes debugging easier.

$this->script_modules->register( 'dependency', '/dep.js' );
$this->script_modules->register( 'example', '/example.js', array( 'dependency' ) );

// Nothing printed now.
$this->assertSame( array(), $this->get_enqueued_script_modules() );
$this->assertSame( array(), $this->get_preloaded_script_modules() );
$this->assertSame( array(), $this->get_import_map() );

// After including, the importmap should be populated.
$this->script_modules->include_in_import_map( 'example' );
$this->assertSame( array(), $this->get_enqueued_script_modules() );
$this->assertSame( array(), $this->get_preloaded_script_modules() );

$import_map = $this->get_import_map();
$this->assertCount( 2, $import_map );
$this->assertArrayHasKey( 'example', $import_map );
$this->assertArrayHasKey( 'dependency', $import_map );
}

/**
* @ticket 61500
*/
public function test_included_modules_concat_With_enqueued_dependencies() {
$this->script_modules->register( 'dependency-enqueued', '/dep.js' );
$this->script_modules->register(
'enqueued',
'/example.js',
array(
array(
'id' => 'dependency-enqueued',
'import' => 'dynamic',
),
)
);
$this->script_modules->enqueue( 'enqueued' );

$this->script_modules->register( 'dependency', '/dep.js' );
$this->script_modules->register( 'example', '/example.js', array( 'dependency' ) );

// Only dependency-enqueued should be printed.
$enqueued = $this->get_enqueued_script_modules();
$this->assertCount( 1, $enqueued );
$this->assertArrayHasKey( 'enqueued', $enqueued );
$this->assertSame( array(), $this->get_preloaded_script_modules() );

$import_map = $this->get_import_map();
$this->assertCount( 1, $import_map );
$this->assertArrayHasKey( 'dependency-enqueued', $import_map );

// After including, the importmap should be populated.
$this->script_modules->include_in_import_map( 'example' );

$enqueued = $this->get_enqueued_script_modules();
$this->assertCount( 1, $enqueued );
$this->assertArrayHasKey( 'enqueued', $enqueued );
$this->assertSame( array(), $this->get_preloaded_script_modules() );

$import_map = $this->get_import_map();
$this->assertCount( 3, $import_map );
$this->assertArrayHasKey( 'dependency-enqueued', $import_map );
$this->assertArrayHasKey( 'dependency', $import_map );
$this->assertArrayHasKey( 'example', $import_map );
}
}
Loading