Skip to content

Script Loader: Warn when classic scripts depend on modules without footer/defer #11788

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

Open
itzmekhokan wants to merge 1 commit into
base: trunk
Choose a base branch
from
+235 −0
Open

Script Loader: Warn when classic scripts depend on modules without footer/defer #11788

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
34 changes: 34 additions & 0 deletions src/wp-includes/functions.wp-scripts.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,6 +108,32 @@ function _wp_scripts_add_args_data( WP_Scripts $wp_scripts, string $handle, arra
}
if ( ! empty( $args['module_dependencies'] ) ) {
$wp_scripts->add_data( $handle, 'module_dependencies', $args['module_dependencies'] );

/*
* A classic script with module dependencies must either be printed in the
* footer or use the 'defer' loading strategy. Otherwise, the script may be
* evaluated before the script modules import map is printed, causing
* dynamic imports to fail with a "Failed to resolve module specifier" error.
*/
$is_in_footer = ! empty( $args['in_footer'] );
$is_deferred = isset( $args['strategy'] ) && 'defer' === $args['strategy'];
if ( ! $is_in_footer && ! $is_deferred ) {
$trace = debug_backtrace( DEBUG_BACKTRACE_IGNORE_ARGS, 2 );
$function_name = ( $trace[1]['class'] ?? '' ) . ( $trace[1]['type'] ?? '' ) . $trace[1]['function'];
_doing_it_wrong(
$function_name,
sprintf(
/* translators: 1: 'module_dependencies', 2: Script handle, 3: 'in_footer', 4: 'strategy', 5: 'defer'. */
__( 'When the %1$s arg is provided, the "%2$s" script must either be printed in the footer (%3$s set to true) or use a deferred loading %4$s (%5$s) so that the import map is printed before the script is evaluated.' ),
'<code>module_dependencies</code>',
$handle,
'<code>in_footer</code>',
'<code>strategy</code>',
'<code>defer</code>'
),
'7.0.0'
);
}
}
}

Expand Down Expand Up @@ -221,6 +247,10 @@ function wp_add_inline_script( $handle, $data, $position = 'after' ) {
* @type string $fetchpriority Optional. The fetch priority for the script. Default 'auto'.
* @type array<string|array<string, string>> $module_dependencies Optional. IDs for module dependencies loaded via dynamic import. Default empty array.
* For the full data format, see the `$deps` param of {@see wp_register_script_module()}.
* When provided, the script must either be printed in the footer (with
* `in_footer` set to true) or use a deferred loading `strategy` (`defer`),
* so that the script modules import map is printed before the script
* is evaluated. Otherwise dynamic imports may fail to resolve.
* }
* @return bool Whether the script has been registered. True on success, false on failure.
*/
Expand Down Expand Up @@ -403,6 +433,10 @@ function wp_deregister_script( $handle ) {
* @type string $fetchpriority Optional. The fetch priority for the script. Default 'auto'.
* @type array<string|array<string, string>> $module_dependencies Optional. IDs for module dependencies loaded via dynamic import. Default empty array.
* For the full data format, see the `$deps` param of {@see wp_register_script_module()}.
* When provided, the script must either be printed in the footer (with
* `in_footer` set to true) or use a deferred loading `strategy` (`defer`),
* so that the script modules import map is printed before the script
* is evaluated. Otherwise dynamic imports may fail to resolve.
* }
*/
function wp_enqueue_script( $handle, $src = '', $deps = array(), $ver = false, $args = array() ) {
Expand Down
197 changes: 197 additions & 0 deletions tests/phpunit/tests/dependencies/scripts.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1396,6 +1396,203 @@
);
}

/**
* Tests that registering a script with `module_dependencies` triggers `_doing_it_wrong`
* when the script is not printed in the footer and does not use the `defer` strategy.
*
* @ticket 65165
*
* @covers ::wp_register_script
* @covers ::wp_enqueue_script
* @covers ::_wp_scripts_add_args_data
*
* @dataProvider data_module_dependencies_require_footer_or_defer
*
* @param string $function_name Function name to call.
* @param array $args Arguments to pass to the function.
* @param bool $should_warn Whether the call is expected to trigger a `_doing_it_wrong` warning.
*/
public function test_module_dependencies_require_footer_or_defer( string $function_name, array $args, bool $should_warn ) {
if ( $should_warn ) {
$this->setExpectedIncorrectUsage( $function_name );
}

call_user_func_array( $function_name, $args );

if ( $should_warn ) {
$this->assertStringContainsString(
'module_dependencies',
$this->caught_doing_it_wrong[ $function_name ],
'The _doing_it_wrong message should reference module_dependencies.'
);
$this->assertStringContainsString(
'in_footer',
$this->caught_doing_it_wrong[ $function_name ],
'The _doing_it_wrong message should reference the in_footer requirement.'
);
$this->assertStringContainsString(
'defer',
$this->caught_doing_it_wrong[ $function_name ],
'The _doing_it_wrong message should reference the defer strategy.'
);
} else {
$this->assertArrayNotHasKey(
$function_name,
$this->caught_doing_it_wrong,
'No _doing_it_wrong warning should be triggered when in_footer is true or strategy is defer.'
);
}
}

/**
* Data provider for test_module_dependencies_require_footer_or_defer.
*
* @return array<string, array{function_name: string, args: array, should_warn: bool}>
*/
public function data_module_dependencies_require_footer_or_defer(): array {
$base_args = array(
'/script.js',
array(),
null,
);

return array(
'register_blocking_warns' => array(

Check warning on line 1460 in tests/phpunit/tests/dependencies/scripts.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 12 space(s) between "'register_blocking_warns'" and double arrow, but found 13.
'function_name' => 'wp_register_script',
'args' => array_merge(
array( 'module-deps-blocking-register' ),
$base_args,
array(
array(
'module_dependencies' => array( 'foo' ),
),
)
),
'should_warn' => true,
),
'enqueue_blocking_warns' => array(

Check warning on line 1473 in tests/phpunit/tests/dependencies/scripts.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 13 space(s) between "'enqueue_blocking_warns'" and double arrow, but found 14.
'function_name' => 'wp_enqueue_script',
'args' => array_merge(
array( 'module-deps-blocking-enqueue' ),
$base_args,
array(
array(
'module_dependencies' => array( 'foo' ),
),
)
),
'should_warn' => true,
),
'register_async_warns' => array(

Check warning on line 1486 in tests/phpunit/tests/dependencies/scripts.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 15 space(s) between "'register_async_warns'" and double arrow, but found 16.
'function_name' => 'wp_register_script',
'args' => array_merge(
array( 'module-deps-async-register' ),
$base_args,
array(
array(
'module_dependencies' => array( 'foo' ),
'strategy' => 'async',
),
)
),
'should_warn' => true,
),
'register_in_footer_does_not_warn' => array(

Check warning on line 1500 in tests/phpunit/tests/dependencies/scripts.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 3 space(s) between "'register_in_footer_does_not_warn'" and double arrow, but found 4.
'function_name' => 'wp_register_script',
'args' => array_merge(
array( 'module-deps-footer-register' ),
$base_args,
array(
array(
'module_dependencies' => array( 'foo' ),
'in_footer' => true,
),
)
),
'should_warn' => false,
),
'enqueue_in_footer_does_not_warn' => array(

Check warning on line 1514 in tests/phpunit/tests/dependencies/scripts.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 4 space(s) between "'enqueue_in_footer_does_not_warn'" and double arrow, but found 5.
'function_name' => 'wp_enqueue_script',
'args' => array_merge(
array( 'module-deps-footer-enqueue' ),
$base_args,
array(
array(
'module_dependencies' => array( 'foo' ),
'in_footer' => true,
),
)
),
'should_warn' => false,
),
'register_defer_does_not_warn' => array(

Check warning on line 1528 in tests/phpunit/tests/dependencies/scripts.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 7 space(s) between "'register_defer_does_not_warn'" and double arrow, but found 8.
'function_name' => 'wp_register_script',
'args' => array_merge(
array( 'module-deps-defer-register' ),
$base_args,
array(
array(
'module_dependencies' => array( 'foo' ),
'strategy' => 'defer',
),
)
),
'should_warn' => false,
),
'enqueue_defer_does_not_warn' => array(

Check warning on line 1542 in tests/phpunit/tests/dependencies/scripts.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 8 space(s) between "'enqueue_defer_does_not_warn'" and double arrow, but found 9.
'function_name' => 'wp_enqueue_script',
'args' => array_merge(
array( 'module-deps-defer-enqueue' ),
$base_args,
array(
array(
'module_dependencies' => array( 'foo' ),
'strategy' => 'defer',
),
)
),
'should_warn' => false,
),
'register_footer_and_defer_no_warn' => array(

Check warning on line 1556 in tests/phpunit/tests/dependencies/scripts.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 2 space(s) between "'register_footer_and_defer_no_warn'" and double arrow, but found 3.
'function_name' => 'wp_register_script',
'args' => array_merge(
array( 'module-deps-footer-defer-register' ),
$base_args,
array(
array(
'module_dependencies' => array( 'foo' ),
'in_footer' => true,
'strategy' => 'defer',
),
)
),
'should_warn' => false,
),
'register_no_module_deps_no_warn' => array(

Check warning on line 1571 in tests/phpunit/tests/dependencies/scripts.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 4 space(s) between "'register_no_module_deps_no_warn'" and double arrow, but found 5.
'function_name' => 'wp_register_script',
'args' => array_merge(
array( 'no-module-deps-register' ),
$base_args,
array( array() )
),
'should_warn' => false,
),
'register_empty_module_deps_no_warn' => array(

Check warning on line 1580 in tests/phpunit/tests/dependencies/scripts.php

View workflow job for this annotation

GitHub Actions / Coding standards / PHP checks 

Array double arrow not aligned correctly; expected 1 space(s) between "'register_empty_module_deps_no_warn'" and double arrow, but found 2.
'function_name' => 'wp_register_script',
'args' => array_merge(
array( 'empty-module-deps-register' ),
$base_args,
array(
array(
'module_dependencies' => array(),
),
)
),
'should_warn' => false,
),
);
}

/**
* Data provider.
*
Expand Down
4 changes: 4 additions & 0 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 @@ -2041,6 +2041,7 @@ public function test_included_module_appears_in_importmap() {
array( 'classic-dependency' ),
false,
array(
'in_footer' => true,
'module_dependencies' => array(
'example',
array(
Expand Down Expand Up @@ -2109,6 +2110,7 @@ public function test_import_map_includes_dependencies_of_classic_scripts_recursi
array(),
false,
array(
'in_footer' => true,
'module_dependencies' => array( 'classic-transitive-dependency' ),
)
);
Expand All @@ -2118,6 +2120,7 @@ public function test_import_map_includes_dependencies_of_classic_scripts_recursi
array( 'classic-transitive-dep' ),
false,
array(
'in_footer' => true,
'module_dependencies' => array( 'not-enqueued' ),
)
);
Expand Down Expand Up @@ -2153,6 +2156,7 @@ public function test_wp_scripts_doing_it_wrong_for_missing_script_module_depende
array(),
null,
array(
'in_footer' => true,
'module_dependencies' => array( 'does-not-exist' ),
)
);
Expand Down
Loading