Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 16 additions & 0 deletions src/wp-includes/class-wp-script-modules.php
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
<?php

/**
* Script Modules API: WP_Script_Modules class.
*
Expand All @@ -14,6 +15,7 @@
* @since 6.5.0
*/
class WP_Script_Modules {

/**
* Holds the registered script modules, keyed by script module identifier.
*
Expand Down Expand Up @@ -343,6 +345,20 @@ public function print_enqueued_script_modules() {
if ( $fetchpriority !== $script_module['fetchpriority'] ) {
$args['data-wp-fetchpriority'] = $script_module['fetchpriority'];
}

/**
* Filters the attributes for a script module tag.
*
* @since 6.9.0
*
* @param array $args Key-value pairs representing `<script>` tag attributes.
* Only the attribute name is added to the `<script>` tag for
* entries with a boolean value, and that are true.
* @param string $id The script module identifier.
* @param array $script_module The script module data.
*/
$args = apply_filters( 'wp_script_module_attributes', $args, $id, $script_module );
Copy link
Member

Choose a reason for hiding this comment

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

Can't the existing filter be used? I don't think we really need a new filter.

Copy link
Member

Choose a reason for hiding this comment

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

Couldn't that have unexpected consequences? Someone who is using the wp_script_attributes filter today only expects to be modifying the scripts, not the script modules.

For example, if someone is using that filter to add defer to some scripts with a conditional that is also true for some script modules, it would generate invalid HTML for them:

Image

Copy link
Member

Choose a reason for hiding this comment

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

No, because as you see below, it is using wp_print_script_tag(). This function is used for printing any script, including classic scripts, script modules, speculationrules. This function calls wp_get_script_tag() and that function is what applies the wp_script_attributes filters.

Copy link
Member

Choose a reason for hiding this comment

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

Ohhh, it's already there. We didn't notice that!

Thanks, Weston. Closing this then, as it's not needed. We'll update GB as well 🙂

Copy link
Member

Choose a reason for hiding this comment

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

I've drafted a PR as a starting point: WordPress/gutenberg#72449

I'm guessing lib/compat/wordpress-6.9/script-modules.php can be removed as well.

Copy link
Member

Choose a reason for hiding this comment

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

I'd make the arguments match the existing filter for scripts (wp_script_attributes):

  • Rename $args to $attributes
  • Remove $id, which can be obtained from the $attributes['id'] anyway
  • Eliminate $script_module, whose schema is not publicly exposed for now, and this exposure would mean that backward compatibility would have to be maintained from now on.
Suggested change
$args = apply_filters( 'wp_script_module_attributes', $args, $id, $script_module );
$args = apply_filters( 'wp_script_module_attributes', $attributes );


wp_print_script_tag( $args );
}
}
Expand Down
34 changes: 34 additions & 0 deletions tests/phpunit/tests/script-modules/wpScriptModules.php
Original file line number Diff line number Diff line change
Expand Up @@ -1730,4 +1730,38 @@ public static function data_invalid_script_module_data(): array {
'string' => array( 'string' ),
);
}

/**
* Tests that the wp_script_module_attributes filter can modify script module attributes.
*
* @ticket 62525
*
* @covers WP_Script_Modules::print_enqueued_script_modules()
*/
public function test_wp_script_module_attributes_filter() {
$this->script_modules->register( 'foo', '/foo.js' );
$this->script_modules->enqueue( 'foo' );

add_filter(
'wp_script_module_attributes',
function ( $args, $id ) {
if ( 'foo' === $id ) {
$args['data-custom'] = 'test-value';
$args['crossorigin'] = 'anonymous';
}
return $args;
},
10,
2
);

$enqueued_modules = $this->get_enqueued_script_modules();

$this->assertArrayHasKey( 'foo', $enqueued_modules );
$this->assertSame( 'test-value', $enqueued_modules['foo']['data-custom'] );

// Check the full output to verify crossorigin attribute
$output = get_echo( array( $this->script_modules, 'print_enqueued_script_modules' ) );
$this->assertStringContainsString( 'crossorigin="anonymous"', $output );
}
}
Loading