Script Loader: Warn when classic scripts with module dependencies lack footer/defer.

A classic script with `module_dependencies` may be evaluated before the script modules import map is printed if it loads blocking in the document head, causing a "Failed to resolve module specifier" error on dynamic imports.

* Trigger `_doing_it_wrong()` from `_wp_scripts_add_args_data()` when a classic script provides `module_dependencies` without setting `in_footer` to `true` or using a `defer` loading `strategy`, and document this requirement in the `wp_register_script()` and `wp_enqueue_script()` docblocks.
* Remove the `module_dependencies` arg from the `wp-codemirror` script registration in favor of passing the espree module URL directly through `wp_get_code_editor_settings()`. This avoids registering `espree` as a publicly-available script module when it is only ever used internally as a private implementation detail of the code editor.
* Add a `console.warn()` in `wp.codeEditor.initialize()` when invoked before `DOMContentLoaded`, so developers are alerted if the function is called too early for the import map to have been parsed.
* Add PHPStan types which were missing when `module_dependencies` were initially introduced.
* Harden `WP_Scripts::add_data()` against non-string `strategy` values being passed to `sprintf()`.

Developed in WordPress#11788

Follow-up to r61587.

Props khokansardar, westonruter, jonsurrell, jorbin.
See #61500, #64238.
Fixes #65165.


git-svn-id: https://develop.svn.wordpress.org/trunk@62368 602fd350-edb4-49c9-b593-d223f7449a82

Author: westonruter

src/js/_enqueues/lib/codemirror/javascript-lint.js

@@ -30,6 +30,7 @@ import CodeMirror from 'codemirror';
  * @property {boolean} [es3] - "This option tells JSHint that your code needs to adhere to ECMAScript 3 specification. Use this option if you need your program to be executable in older browsers—such as Internet Explorer 6/7/8/9—and other legacy JavaScript environments."
  * @property {boolean} [module] - "This option informs JSHint that the input code describes an ECMAScript 6 module. All module code is interpreted as strict mode code."
  * @property {'implied'} [strict] - "This option requires the code to run in ECMAScript 5's strict mode."
+ * @property {string} [espreeModuleUrl] - The URL to the espree script module.
  */
 
 /**
@@ -42,9 +43,13 @@ import CodeMirror from 'codemirror';
  * @returns {Promise<CodeMirrorLintError[]>}
  */
 async function validator( text, options ) {
+	if ( ! options.espreeModuleUrl ) {
+		return [];
+	}
+
 	const errors = /** @type {CodeMirrorLintError[]} */ [];
 	try {
-		const espree = await import( /* webpackIgnore: true */ 'espree' );
+		const espree = await import( /* webpackIgnore: true */ options.espreeModuleUrl );
 		espree.parse( text, {
 			...getEspreeOptions( options ),
 			loc: true,

src/js/_enqueues/wp/code-editor.js

@@ -2,6 +2,8 @@
  * @output wp-admin/js/code-editor.js
  */
 
+/* global console */
+
 /* eslint-env es2020 */
 
 if ( 'undefined' === typeof window.wp ) {
@@ -412,6 +414,10 @@ if ( 'undefined' === typeof window.wp.codeEditor ) {
 	 * @return {CodeEditorInstance} Instance.
 	 */
 	wp.codeEditor.initialize = function initialize( textarea, settings ) {
+		if ( document.readyState === 'loading' ) {
+			console.warn( 'wp.codeEditor.initialize() ran too early. Invoke this function in a `DOMContentLoaded` event listener.' );
+		}
+
 		let $textarea;
 		if ( 'string' === typeof textarea ) {
 			$textarea = $( '#' + textarea );

src/wp-includes/class-wp-scripts.php

@@ -885,7 +885,7 @@ public function add_data( $handle, $key, $value ) {
 					sprintf(
 						/* translators: 1: $strategy, 2: $handle */
 						__( 'Invalid strategy `%1$s` defined for `%2$s` during script registration.' ),
-						$value,
+						is_string( $value ) ? $value : gettype( $value ),
 						$handle
 					),
 					'6.3.0'
@@ -897,7 +897,7 @@ public function add_data( $handle, $key, $value ) {
 					sprintf(
 						/* translators: 1: $strategy, 2: $handle */
 						__( 'Cannot supply a strategy `%1$s` for script `%2$s` because it is an alias (it lacks a `src` value).' ),
-						$value,
+						is_string( $value ) ? $value : gettype( $value ),
 						$handle
 					),
 					'6.3.0'

src/wp-includes/functions.wp-scripts.php

@@ -71,19 +71,30 @@ function _wp_scripts_maybe_doing_it_wrong( $function_name, $handle = '' ) {
 /**
  * Adds the data for the recognized args and warns for unrecognized args.
  *
+ * @see wp_enqueue_script()
+ * @see wp_register_script()
+ *
  * @ignore
  * @since 7.0.0
  *
  * @param WP_Scripts $wp_scripts WP_Scripts instance.
  * @param string     $handle     Script handle.
  * @param array      $args       Array of extra args for the script.
+ *
+ * @phpstan-param non-empty-string $handle
+ * @phpstan-param array{
+ *     in_footer?: bool,
+ *     strategy?: 'async'|'defer',
+ *     fetchpriority?: 'low'|'auto'|'high',
+ *     module_dependencies?: array<non-empty-string|array{ id: non-empty-string, ... }>,
+ * } $args
  */
-function _wp_scripts_add_args_data( WP_Scripts $wp_scripts, string $handle, array $args ) {
+function _wp_scripts_add_args_data( WP_Scripts $wp_scripts, string $handle, array $args ): void {
 	$allowed_keys = array( 'strategy', 'in_footer', 'fetchpriority', 'module_dependencies' );
 	$unknown_keys = array_diff( array_keys( $args ), $allowed_keys );
 	if ( ! empty( $unknown_keys ) ) {
 		$trace         = debug_backtrace( DEBUG_BACKTRACE_IGNORE_ARGS, 2 );
-		$function_name = ( $trace[1]['class'] ?? '' ) . ( $trace[1]['type'] ?? '' ) . $trace[1]['function'];
+		$function_name = ( $trace[1]['class'] ?? '' ) . ( $trace[1]['type'] ?? '' ) . ( $trace[1]['function'] ?? __FUNCTION__ );
 		_doing_it_wrong(
 			$function_name,
 			sprintf(
@@ -97,7 +108,8 @@ function _wp_scripts_add_args_data( WP_Scripts $wp_scripts, string $handle, arra
 		);
 	}
 
-	if ( ! empty( $args['in_footer'] ) ) {
+	$in_footer = ! empty( $args['in_footer'] );
+	if ( $in_footer ) {
 		$wp_scripts->add_data( $handle, 'group', 1 );
 	}
 	if ( ! empty( $args['strategy'] ) ) {
@@ -108,6 +120,31 @@ 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_deferred = 'defer' === ( $args['strategy'] ?? null );
+		if ( ! $in_footer && ! $is_deferred ) {
+			$trace         = debug_backtrace( DEBUG_BACKTRACE_IGNORE_ARGS, 2 );
+			$function_name = ( $trace[1]['class'] ?? '' ) . ( $trace[1]['type'] ?? '' ) . ( $trace[1]['function'] ?? __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'
+			);
+		}
 	}
 }
 
@@ -204,25 +241,39 @@ function wp_add_inline_script( $handle, $data, $position = 'after' ) {
  * @since 6.9.0 The $fetchpriority parameter of type string was added to the $args parameter of type array.
  * @since 7.0.0 The $module_dependencies parameter of type string[] was added to the $args parameter of type array.
  *
- * @param string                                                              $handle Name of the script. Should be unique.
- * @param string|false                                                        $src    Full URL of the script, or path of the script relative to the WordPress root directory.
- *                                                                                    If source is set to false, script is an alias of other scripts it depends on.
- * @param string[]                                                            $deps   Optional. An array of registered script handles this script depends on. Default empty array.
- * @param string|bool|null                                                    $ver    Optional. String specifying script version number, if it has one, which is added to the URL
- *                                                                                    as a query string for cache busting purposes. If version is set to false, a version
- *                                                                                    number is automatically added equal to current installed WordPress version.
- *                                                                                    If set to null, no version is added.
- * @param array<string, string|bool|array<string|array<string, string>>>|bool $args   {
+ * @param string           $handle Name of the script. Should be unique.
+ * @param string|false     $src    Full URL of the script, or path of the script relative to the WordPress root directory.
+ *                                 If source is set to false, script is an alias of other scripts it depends on.
+ * @param string[]         $deps   Optional. An array of registered script handles this script depends on. Default empty array.
+ * @param string|bool|null $ver    Optional. String specifying script version number, if it has one, which is added to the URL
+ *                                 as a query string for cache busting purposes. If version is set to false, a version
+ *                                 number is automatically added equal to current installed WordPress version.
+ *                                 If set to null, no version is added.
+ * @param array|bool       $args   {
  *     Optional. An array of extra args for the script. Default empty array.
  *     Otherwise, it may be a boolean in which case it determines whether the script is printed in the footer. Default false.
  *
- *     @type string                              $strategy            Optional. If provided, may be either 'defer' or 'async'.
- *     @type bool                                $in_footer           Optional. Whether to print the script in the footer. Default 'false'.
- *     @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.
+ *     @type string $strategy            Optional. If provided, may be either 'defer' or 'async'.
+ *     @type bool   $in_footer           Optional. Whether to print the script in the footer. Default 'false'.
+ *     @type string $fetchpriority       Optional. The fetch priority for the script. Default 'auto'.
+ *     @type array  $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.
+ *
+ * @phpstan-param non-empty-string $handle
+ * @phpstan-param non-empty-string|false $src
+ * @phpstan-param non-empty-string[] $deps
+ * @phpstan-param array{
+ *     in_footer?: bool,
+ *     strategy?: 'async'|'defer',
+ *     fetchpriority?: 'low'|'auto'|'high',
+ *     module_dependencies?: array<non-empty-string|array{ id: non-empty-string, ... }>,
+ * }|bool $args
  */
 function wp_register_script( $handle, $src, $deps = array(), $ver = false, $args = array() ) {
 	if ( ! is_array( $args ) ) {
@@ -386,31 +437,46 @@ function wp_deregister_script( $handle ) {
  * @since 6.9.0 The $fetchpriority parameter of type string was added to the $args parameter of type array.
  * @since 7.0.0 The $module_dependencies parameter of type string[] was added to the $args parameter of type array.
  *
- * @param string                                                              $handle Name of the script. Should be unique.
- * @param string                                                              $src    Full URL of the script, or path of the script relative to the WordPress root directory.
- *                                                                                    Default empty.
- * @param string[]                                                            $deps   Optional. An array of registered script handles this script depends on. Default empty array.
- * @param string|bool|null                                                    $ver    Optional. String specifying script version number, if it has one, which is added to the URL
- *                                                                                    as a query string for cache busting purposes. If version is set to false, a version
- *                                                                                    number is automatically added equal to current installed WordPress version.
- *                                                                                    If set to null, no version is added.
- * @param array<string, string|bool|array<string|array<string, string>>>|bool $args {
+ * @param string           $handle Name of the script. Should be unique.
+ * @param string           $src    Full URL of the script, or path of the script relative to the WordPress root directory.
+ *                                 Default empty.
+ * @param string[]         $deps   Optional. An array of registered script handles this script depends on. Default empty array.
+ * @param string|bool|null $ver    Optional. String specifying script version number, if it has one, which is added to the URL
+ *                                 as a query string for cache busting purposes. If version is set to false, a version
+ *                                 number is automatically added equal to current installed WordPress version.
+ *                                 If set to null, no version is added.
+ * @param array|bool $args {
  *     Optional. An array of extra args for the script. Default empty array.
  *     Otherwise, it may be a boolean in which case it determines whether the script is printed in the footer. Default false.
  *
- *     @type string                              $strategy            Optional. If provided, may be either 'defer' or 'async'.
- *     @type bool                                $in_footer           Optional. Whether to print the script in the footer. Default 'false'.
- *     @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()}.
+ *     @type string $strategy            Optional. If provided, may be either 'defer' or 'async'.
+ *     @type bool   $in_footer           Optional. Whether to print the script in the footer. Default 'false'.
+ *     @type string $fetchpriority       Optional. The fetch priority for the script. Default 'auto'.
+ *     @type array  $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.
  * }
+ *
+ * @phpstan-param non-empty-string $handle
+ * @phpstan-param string $src
+ * @phpstan-param non-empty-string[] $deps
+ * @phpstan-param array{
+ *     in_footer?: bool,
+ *     strategy?: 'async'|'defer',
+ *     fetchpriority?: 'low'|'auto'|'high',
+ *     module_dependencies?: array<non-empty-string|array{ id: non-empty-string, ... }>,
+ * }|bool $args
  */
 function wp_enqueue_script( $handle, $src = '', $deps = array(), $ver = false, $args = array() ) {
 	_wp_scripts_maybe_doing_it_wrong( __FUNCTION__, $handle );
 
 	$wp_scripts = wp_scripts();
 
 	if ( $src || ! empty( $args ) ) {
+		/** @var array{ 0: non-empty-string, 1?: string } $_handle */
 		$_handle = explode( '?', $handle );
 		if ( ! is_array( $args ) ) {
 			$args = array(

src/wp-includes/general-template.php

@@ -4155,27 +4155,31 @@ function wp_get_code_editor_settings( $args ) {
 			'outline-none'              => true,
 		),
 		'jshint'     => array(
-			'esversion' => 11,
-			'module'    => str_ends_with( $args['file'] ?? '', '.mjs' ),
+			'esversion'       => 11,
+			'module'          => str_ends_with( $args['file'] ?? '', '.mjs' ),
+
+			// This script module URL is intentionally referenced here instead of registering an espree script module
+			// in wp_default_script_modules(). This is a first stab at a core-only private module.
+			'espreeModuleUrl' => add_query_arg( 'ver', '9.6.1', includes_url( 'js/codemirror/espree.min.js' ) ),
 
 			// The following JSHint *linting rule* options are copied from
 			// <https://github.com/WordPress/wordpress-develop/blob/6.9.0/.jshintrc>.
 			// Parsing-related options such as `esversion` (and, in other contexts, `es5`, `es3`, `module`, `strict`)
 			// are honored by the Espree-based integration, but these linting-rule options are not interpreted by Espree
 			// and are kept only for compatibility/documentation with the original JSHint configuration.
-			'boss'      => true,
-			'curly'     => true,
-			'eqeqeq'    => true,
-			'eqnull'    => true,
-			'expr'      => true,
-			'immed'     => true,
-			'noarg'     => true,
-			'nonbsp'    => true,
-			'quotmark'  => 'single',
-			'undef'     => true,
-			'unused'    => true,
-			'browser'   => true,
-			'globals'   => array(
+			'boss'            => true,
+			'curly'           => true,
+			'eqeqeq'          => true,
+			'eqnull'          => true,
+			'expr'            => true,
+			'immed'           => true,
+			'noarg'           => true,
+			'nonbsp'          => true,
+			'quotmark'        => 'single',
+			'undef'           => true,
+			'unused'          => true,
+			'browser'         => true,
+			'globals'         => array(
 				'_'                 => false,
 				'Backbone'          => false,
 				'jQuery'            => false,

src/wp-includes/script-loader.php

@@ -1200,9 +1200,8 @@ function wp_default_scripts( $scripts ) {
 	);
 
 	$scripts->add( 'wp-codemirror', '/wp-includes/js/codemirror/codemirror.min.js', array(), '5.65.20' );
-	did_action( 'init' ) && $scripts->add_data( 'wp-codemirror', 'module_dependencies', array( 'espree' ) );
 	$scripts->add( 'csslint', '/wp-includes/js/codemirror/csslint.js', array(), '1.0.5' );
-	$scripts->add( 'esprima', '/wp-includes/js/codemirror/esprima.js', array(), '4.0.1' ); // Deprecated. Use 'espree' script module.
+	$scripts->add( 'esprima', '/wp-includes/js/codemirror/esprima.js', array(), '4.0.1' ); // Deprecated.
 	$scripts->add( 'jshint', '/wp-includes/js/codemirror/fakejshint.js', array( 'esprima' ), '2.9.5' ); // Deprecated.
 	$scripts->add( 'jsonlint', '/wp-includes/js/codemirror/jsonlint.js', array(), '1.6.3' );
 	$scripts->add( 'htmlhint', '/wp-includes/js/codemirror/htmlhint.js', array(), '1.8.0' );

src/wp-includes/script-modules.php

@@ -219,13 +219,6 @@ function wp_default_script_modules() {
 		$module_deps = $script_module_data['module_dependencies'] ?? array();
 		wp_register_script_module( $script_module_id, $path, $module_deps, $script_module_data['version'], $args );
 	}
-
-	wp_register_script_module(
-		'espree',
-		includes_url( 'js/codemirror/espree.min.js' ),
-		array(),
-		'9.6.1'
-	);
 }
 
 /**

tests/phpunit/includes/abstract-testcase.php

@@ -18,7 +18,9 @@ abstract class WP_UnitTestCase_Base extends PHPUnit_Adapter_TestCase {
 	protected $expected_deprecated     = array();
 	protected $caught_deprecated       = array();
 	protected $expected_doing_it_wrong = array();
-	protected $caught_doing_it_wrong   = array();
+
+	/** @var non-empty-string[] */
+	protected $caught_doing_it_wrong = array();
 
 	protected static $hooks_saved = array();
 	protected static $ignore_files;