Script Loader: Fix transitive leaks and improve diagnostics for scoped modules.

sirreal · sirreal · commit 602daa6127e3 · 2026-05-05T20:00:30.000+02:00
Address review feedback for the scoped script modules feature:

- Stop import-map traversal at modules registered with empty scopes so their
  public transitive dependencies don't leak into top-level `imports`.
- Warn when a classic script's `module_dependencies` reference a module
  registered with empty scopes; such a dependency cannot be resolved via bare
  specifier from the classic script.
- Warn when a `module_id` scope resolves to a URL with no path separator
  (which cannot be reduced to a directory).
- Improve register-time validation messages to include the module identifier
  and received type, and avoid PHP syntax in translatable strings.
- Correct the docblock claim that only static dependencies on empty-scoped
  modules fail; declared dependencies (static or dynamic) are treated like
  missing dependencies, mirroring existing missing-dep semantics.
- Tighten test assertions and wrap a filter in `try/finally` to prevent leaks
  between tests; add coverage for the transitive-leak fix, the classic-script
  empty-scoped warning, and the `module_id` no-slash diagnostic.
diff --git a/src/wp-includes/class-wp-script-modules.php b/src/wp-includes/class-wp-script-modules.php
@@ -134,8 +134,9 @@ class WP_Script_Modules {
 	 *                                              When omitted or null, the module is public (top-level `imports`). When an array is
 	 *                                              provided, the module is emitted under the import map's `scopes` keyed by each entry,
 	 *                                              and is not present in top-level `imports`. An empty array means the module is
-	 *                                              registered but cannot be resolved via bare specifier from anywhere; static
-	 *                                              dependencies on such a module are treated like missing dependencies.
+	 *                                              registered but cannot be resolved via bare specifier from anywhere; declared
+	 *                                              dependencies on such a module (static or dynamic) are treated like missing
+	 *                                              dependencies and the dependent will not be emitted.
 	 *                                              Each entry is one of:
 	 *                                              - A non-empty string URL prefix, emitted as-authored. WordPress URL filters such as
 	 *                                                `script_module_loader_src` are NOT applied to string scopes; authors are
@@ -209,7 +210,12 @@ public function register( string $id, string $src, array $deps = array(), $versi
 				if ( ! is_array( $args['scopes'] ) ) {
 					_doing_it_wrong(
 						__METHOD__,
-						__( 'The "scopes" arg must be null or an array.' ),
+						sprintf(
+							/* translators: 1: Module identifier; 2: Received type. */
+							__( 'The "scopes" arg for module "%1$s" must be null or an array; received %2$s.' ),
+							$id,
+							gettype( $args['scopes'] )
+						),
 						'7.1.0'
 					);
 				} else {
@@ -219,7 +225,11 @@ public function register( string $id, string $src, array $deps = array(), $versi
 							if ( '' === $scope_entry ) {
 								_doing_it_wrong(
 									__METHOD__,
-									__( 'Empty string scope entry; entries must be non-empty.' ),
+									sprintf(
+										/* translators: %s: Module identifier. */
+										__( 'A scope entry for module "%s" must be a non-empty string.' ),
+										$id
+									),
 									'7.1.0'
 								);
 								continue;
@@ -236,7 +246,11 @@ public function register( string $id, string $src, array $deps = array(), $versi
 						} else {
 							_doing_it_wrong(
 								__METHOD__,
-								__( 'Each scope entry must be a non-empty string or [ "module_id" => non-empty string ].' ),
+								sprintf(
+									/* translators: %s: Module identifier. */
+									__( 'A scope entry for module "%s" must be a non-empty string or an array with a single "module_id" key whose value is a non-empty string.' ),
+									$id
+								),
 								'7.1.0'
 							);
 						}
@@ -730,7 +744,8 @@ private function get_import_map(): array {
 
 				$module_dependencies = $wp_scripts->get_data( $handle, 'module_dependencies' );
 				if ( is_array( $module_dependencies ) ) {
-					$missing_module_dependencies = array();
+					$missing_module_dependencies     = array();
+					$unreachable_module_dependencies = array();
 					foreach ( $module_dependencies as $module ) {
 						if ( is_string( $module ) ) {
 							$id = $module;
@@ -744,6 +759,11 @@ private function get_import_map(): array {
 
 						if ( ! isset( $this->registered[ $id ] ) ) {
 							$missing_module_dependencies[] = $id;
+						} elseif (
+							isset( $this->registered[ $id ]['scopes'] ) &&
+							array() === $this->registered[ $id ]['scopes']
+						) {
+							$unreachable_module_dependencies[] = $id;
 						} else {
 							$classic_script_module_dependencies[] = $id;
 						}
@@ -762,6 +782,20 @@ private function get_import_map(): array {
 							'7.0.0'
 						);
 					}
+
+					if ( count( $unreachable_module_dependencies ) > 0 ) {
+						_doing_it_wrong(
+							'WP_Scripts::add_data',
+							sprintf(
+								/* translators: 1: Script handle, 2: 'module_dependencies', 3: List of unreachable dependency IDs. */
+								__( 'The script with the handle "%1$s" was enqueued with script module dependencies ("%2$s") that are registered with empty scopes and cannot be resolved via bare specifier: %3$s.' ),
+								$handle,
+								'module_dependencies',
+								implode( wp_get_list_item_separator(), $unreachable_module_dependencies )
+							),
+							'7.1.0'
+						);
+					}
 				}
 
 				foreach ( $wp_scripts->registered[ $handle ]->deps as $dep ) {
@@ -772,11 +806,47 @@ private function get_import_map(): array {
 			}
 		}
 
-		// Note: the script modules in $this->queue are not included in the importmap because they get printed as scripts.
+		/*
+		 * Walk the dependency graph from queue items + classic-script module dependencies.
+		 * Queue items themselves are not added to `$ids` (they are printed as `<script>` tags),
+		 * but they appear via dep edges of other modules — matching prior behavior of
+		 * `get_dependencies()`.
+		 *
+		 * Modules registered with empty scopes (`scopes => array()`) are not traversed:
+		 * their transitive deps are unreachable via bare specifier from any importer, so
+		 * walking through them would leak otherwise-private deps into top-level `imports`.
+		 */
+		$collected_dependencies = array();
+		$id_queue               = array_merge( $this->queue, $classic_script_module_dependencies );
+		while ( ! empty( $id_queue ) ) {
+			$current_id = array_shift( $id_queue );
+			if ( ! isset( $this->registered[ $current_id ] ) ) {
+				continue;
+			}
+
+			// Stop at empty-scoped modules.
+			if (
+				isset( $this->registered[ $current_id ]['scopes'] ) &&
+				array() === $this->registered[ $current_id ]['scopes']
+			) {
+				continue;
+			}
+
+			foreach ( $this->registered[ $current_id ]['dependencies'] as $dependency ) {
+				if (
+					! isset( $collected_dependencies[ $dependency['id'] ] ) &&
+					isset( $this->registered[ $dependency['id'] ] )
+				) {
+					$collected_dependencies[ $dependency['id'] ] = true;
+					$id_queue[]                                  = $dependency['id'];
+				}
+			}
+		}
+
 		$ids = array_unique(
 			array_merge(
 				$classic_script_module_dependencies,
-				array_keys( $this->get_dependencies( array_merge( $this->queue, $classic_script_module_dependencies ) ) )
+				array_keys( $collected_dependencies )
 			)
 		);
 
@@ -1179,6 +1249,17 @@ private function get_scope_keys( string $id ): array {
 			$path       = preg_replace( '~[?#].*$~', '', $other_src );
 			$last_slash = strrpos( (string) $path, '/' );
 			if ( false === $last_slash ) {
+				_doing_it_wrong(
+					__METHOD__,
+					sprintf(
+						/* translators: 1: Scope module id; 2: Owning module id; 3: Resolved URL. */
+						__( 'Scope entry references module "%1$s" whose URL "%3$s" has no "/" path separator and cannot be reduced to a directory for module "%2$s".' ),
+						$other_id,
+						$id,
+						$other_src
+					),
+					'7.1.0'
+				);
 				continue;
 			}
 
diff --git a/src/wp-includes/script-modules.php b/src/wp-includes/script-modules.php
@@ -71,8 +71,9 @@ function wp_script_modules(): WP_Script_Modules {
  *                                              When omitted or null, the module is public. When an array is provided, the module is
  *                                              emitted under the import map's `scopes` keyed by each entry, and is not present in
  *                                              top-level `imports`. An empty array means the module is registered but cannot be
- *                                              resolved via bare specifier from anywhere; static dependencies on such a module
- *                                              are treated like missing dependencies.
+ *                                              resolved via bare specifier from anywhere; declared dependencies on such a
+ *                                              module (static or dynamic) are treated like missing dependencies and the
+ *                                              dependent will not be emitted.
  *                                              Each entry is one of:
  *                                              - A non-empty string URL prefix, emitted as-authored. WordPress URL-rewriting
  *                                                hooks such as `script_module_loader_src` are NOT applied to string scopes;
diff --git a/tests/phpunit/tests/script-modules/wpScriptModules.php b/tests/phpunit/tests/script-modules/wpScriptModules.php
@@ -2956,6 +2956,8 @@ public function test_scopes_string_exact_url_scope() {
 
 		$map = $this->get_full_import_map();
 		$this->assertArrayHasKey( 'https://example.com/exact.js', $map['scopes'] );
+		$this->assertArrayHasKey( 'private', $map['scopes']['https://example.com/exact.js'] );
+		$this->assertArrayNotHasKey( 'private', $map['imports'] );
 	}
 
 	/**
@@ -3029,7 +3031,10 @@ public function test_scopes_module_id_unregistered_drops() {
 
 		$map = $this->get_full_import_map();
 		$this->assertArrayHasKey( 'scopes', $map );
+		$this->assertCount( 1, $map['scopes'] );
 		$this->assertArrayHasKey( '/fallback/', $map['scopes'] );
+		$this->assertArrayHasKey( 'private', $map['scopes']['/fallback/'] );
+		$this->assertArrayNotHasKey( 'private', $map['imports'] );
 	}
 
 	/**
@@ -3282,16 +3287,18 @@ public function test_scopes_script_module_data_emitted_for_scoped() {
 		);
 		$this->script_modules->enqueue( 'consumer', '/consumer.js', array( 'private' ) );
 
-		add_filter(
-			'script_module_data_private',
-			static function () {
-				return array( 'token' => 'abc' );
-			}
-		);
+		$data_filter = static function () {
+			return array( 'token' => 'abc' );
+		};
+		add_filter( 'script_module_data_private', $data_filter );
 
-		$markup = get_echo( array( $this->script_modules, 'print_script_module_data' ) );
-		$this->assertStringContainsString( 'wp-script-module-data-private', $markup );
-		$this->assertStringContainsString( '"token":"abc"', $markup );
+		try {
+			$markup = get_echo( array( $this->script_modules, 'print_script_module_data' ) );
+			$this->assertStringContainsString( 'wp-script-module-data-private', $markup );
+			$this->assertStringContainsString( '"token":"abc"', $markup );
+		} finally {
+			remove_filter( 'script_module_data_private', $data_filter );
+		}
 	}
 
 	/**
@@ -3333,4 +3340,106 @@ public function test_scopes_preloads_include_scoped_static_dep() {
 		$preloads = $this->get_preloaded_script_modules();
 		$this->assertArrayHasKey( 'private', $preloads );
 	}
+
+	/**
+	 * Empty-scoped modules do not leak their public transitive deps into top-level imports.
+	 *
+	 * @covers WP_Script_Modules::get_import_map
+	 */
+	public function test_scopes_empty_does_not_leak_transitive_deps_to_imports() {
+		// Public leaf reachable only through the empty-scoped module.
+		$this->script_modules->register( 'leaf', '/leaf.js' );
+		$this->script_modules->register(
+			'unreachable',
+			'/unreachable.js',
+			array( 'leaf' ),
+			null,
+			array( 'scopes' => array() )
+		);
+		$this->script_modules->enqueue( 'consumer', '/consumer.js', array( 'unreachable' ) );
+
+		$map = $this->get_full_import_map();
+		$this->assertArrayNotHasKey( 'leaf', $map['imports'] ?? array(), 'leaf must not appear in imports — only reachable via empty-scoped module.' );
+		$this->assertArrayNotHasKey( 'unreachable', $map['imports'] ?? array() );
+		$this->assertArrayNotHasKey( 'scopes', $map );
+	}
+
+	/**
+	 * A leaf reachable both via empty-scoped and via a public path is still emitted.
+	 *
+	 * @covers WP_Script_Modules::get_import_map
+	 */
+	public function test_scopes_empty_does_not_hide_dep_reachable_via_public_path() {
+		$this->script_modules->register( 'leaf', '/leaf.js' );
+		$this->script_modules->register(
+			'unreachable',
+			'/unreachable.js',
+			array( 'leaf' ),
+			null,
+			array( 'scopes' => array() )
+		);
+		// Public path also reaches leaf.
+		$this->script_modules->register( 'public-mid', '/public-mid.js', array( 'leaf' ) );
+		$this->script_modules->enqueue( 'consumer-a', '/consumer-a.js', array( 'unreachable' ) );
+		$this->script_modules->enqueue( 'consumer-b', '/consumer-b.js', array( 'public-mid' ) );
+
+		$map = $this->get_full_import_map();
+		$this->assertArrayHasKey( 'leaf', $map['imports'] );
+		$this->assertArrayHasKey( 'public-mid', $map['imports'] );
+	}
+
+	/**
+	 * Classic-script `module_dependencies` referencing an empty-scoped module triggers a warning.
+	 *
+	 * @expectedIncorrectUsage WP_Scripts::add_data
+	 *
+	 * @covers WP_Script_Modules::get_import_map
+	 */
+	public function test_scopes_classic_script_module_dep_on_empty_scoped_warns() {
+		$this->script_modules->register(
+			'unreachable',
+			'/unreachable.js',
+			array(),
+			null,
+			array( 'scopes' => array() )
+		);
+
+		wp_enqueue_script(
+			'classic',
+			'/classic.js',
+			array(),
+			null,
+			array( 'module_dependencies' => array( 'unreachable' ) )
+		);
+
+		// Trigger import-map computation.
+		$map = $this->get_full_import_map();
+		$this->assertArrayNotHasKey( 'unreachable', $map['imports'] ?? array() );
+		$this->assertArrayNotHasKey( 'scopes', $map );
+	}
+
+	/**
+	 * `module_id` resolution to a URL with no slash drops with a warning.
+	 *
+	 * @expectedIncorrectUsage WP_Script_Modules::get_scope_keys
+	 *
+	 * @covers WP_Script_Modules::get_import_map
+	 */
+	public function test_scopes_module_id_no_slash_drops_with_warning() {
+		// A module whose src has no slash (after version stripping). `null` version avoids ?ver.
+		$this->script_modules->register( 'oddurl', 'oddurl', array(), null );
+		$this->script_modules->register(
+			'private',
+			'/private.js',
+			array(),
+			null,
+			array( 'scopes' => array( array( 'module_id' => 'oddurl' ) ) )
+		);
+		$this->script_modules->enqueue( 'consumer', '/consumer.js', array( 'private' ) );
+
+		$map = $this->get_full_import_map();
+		// No scope key derivable; private appears nowhere; map is not emitted.
+		$this->assertArrayNotHasKey( 'scopes', $map );
+		$this->assertArrayNotHasKey( 'private', $map['imports'] ?? array() );
+	}
 }
