Allow empty string for set_fetchpriority(), return bool, and add tests

src/wp-includes/class-wp-script-modules.php

@@ -108,8 +108,12 @@ public function register( string $id, string $src, array $deps = array(), $versi
 				} else {
 					_doing_it_wrong(
 						__METHOD__,
-						/* translators: %s: Invalid fetchpriority. */
-						sprintf( __( 'Invalid fetchpriority: %s' ), $args['fetchpriority'] ),
+						sprintf(
+							/* translators: 1: $fetchpriority, 2: $id */
+							__( 'Invalid fetchpriority `%1$s` defined for `%2$s` during script registration.' ),
+							is_string( $args['fetchpriority'] ) ? $args['fetchpriority'] : gettype( $args['fetchpriority'] ),
+							$id
+						),
 						'n.e.x.t'
 					);
 				}
@@ -130,7 +134,7 @@ public function register( string $id, string $src, array $deps = array(), $versi
 	 *
 	 * @since n.e.x.t
 	 *
-	 * @param mixed $priority Fetch priority.
+	 * @param string|mixed $priority Fetch priority.
 	 * @return bool Whether valid fetchpriority.
 	 */
 	private function is_valid_fetchpriority( $priority ): bool {
@@ -144,10 +148,15 @@ private function is_valid_fetchpriority( $priority ): bool {
 	 *
 	 * @param string              $id       Script module identifier.
 	 * @param 'auto'|'low'|'high' $priority Fetch priority for the script module.
+	 * @return bool Whether setting the fetchpriority was successful.
 	 */
-	public function set_fetchpriority( string $id, string $priority ) {
+	public function set_fetchpriority( string $id, string $priority ): bool {
 		if ( ! isset( $this->registered[ $id ] ) ) {
-			return;
+			return false;
+		}
+
+		if ( '' === $priority ) {
+			$priority = 'auto';
 		}
 
 		if ( ! $this->is_valid_fetchpriority( $priority ) ) {
@@ -157,10 +166,11 @@ public function set_fetchpriority( string $id, string $priority ) {
 				sprintf( __( 'Invalid fetchpriority: %s' ), $priority ),
 				'n.e.x.t'
 			);
-			return;
+			return false;
 		}
 
 		$this->registered[ $id ]['fetchpriority'] = $priority;
+		return true;
 	}
 
 	/**

tests/phpunit/tests/script-modules/wpScriptModules.php

@@ -188,7 +188,7 @@ public function test_comprehensive_methods( bool $use_global_function, bool $onl
 		// One Dependency.
 		$register( 'b-dep', '/b-dep.js' );
 		$register_and_enqueue( 'b', '/b.js', array( 'b-dep' ) );
-		wp_script_modules()->set_fetchpriority( 'b', 'low' );
+		$this->assertTrue( wp_script_modules()->set_fetchpriority( 'b', 'low' ) );
 
 		// Two dependencies with different formats and a false version.
 		$register( 'c-dep', '/c-static.js', array(), false, array( 'fetchpriority' => 'low' ) );
@@ -434,7 +434,7 @@ public function test_wp_enqueue_script_module() {
 		$this->script_modules->register( 'foo', '/foo.js' );
 		$this->script_modules->register( 'bar', '/bar.js', array(), false, array( 'fetchpriority' => 'high' ) );
 		$this->script_modules->register( 'baz', '/baz.js' );
-		$this->script_modules->set_fetchpriority( 'baz', 'low' );
+		$this->assertTrue( $this->script_modules->set_fetchpriority( 'baz', 'low' ) );
 		$this->script_modules->enqueue( 'foo' );
 		$this->script_modules->enqueue( 'bar' );
 		$this->script_modules->enqueue( 'baz' );
@@ -1253,6 +1253,103 @@ function ( $_ ) use ( $data ) {
 		$this->assertSame( '', $actual );
 	}
 
+	/**
+	 * Data provider for test_fetchpriority_values.
+	 *
+	 * @return array<string, array{fetchpriority: string}>
+	 */
+	public function data_provider_fetchpriority_values(): array {
+		return array(
+			'auto' => array( 'fetchpriority' => 'auto' ),
+			'low'  => array( 'fetchpriority' => 'low' ),
+			'high' => array( 'fetchpriority' => 'high' ),
+		);
+	}
+
+	/**
+	 * Tests that valid fetchpriority values are correctly added to the registered module.
+	 *
+	 * @ticket 61734
+	 *
+	 * @covers WP_Script_Modules::register
+	 * @covers WP_Script_Modules::set_fetchpriority
+	 *
+	 * @dataProvider data_provider_fetchpriority_values
+	 *
+	 * @param string $fetchpriority The fetchpriority value to test.
+	 */
+	public function test_fetchpriority_values( string $fetchpriority ) {
+		$reflection_class    = new ReflectionClass( $this->script_modules );
+		$registered_property = $reflection_class->getProperty( 'registered' );
+
+		$this->script_modules->register( 'test-script', '/test-script.js', array(), null, array( 'fetchpriority' => $fetchpriority ) );
+		$registered_modules = $registered_property->getValue( $this->script_modules );
+		$this->assertSame( $fetchpriority, $registered_modules['test-script']['fetchpriority'] );
+
+		$this->script_modules->register( 'test-script-2', '/test-script-2.js' );
+		$this->assertTrue( $this->script_modules->set_fetchpriority( 'test-script-2', $fetchpriority ) );
+		$registered_modules = $registered_property->getValue( $this->script_modules );
+		$this->assertSame( $fetchpriority, $registered_modules['test-script-2']['fetchpriority'] );
+
+		$this->assertTrue( $this->script_modules->set_fetchpriority( 'test-script-2', '' ) );
+		$registered_modules = $registered_property->getValue( $this->script_modules );
+		$this->assertSame( 'auto', $registered_modules['test-script-2']['fetchpriority'] );
+	}
+
+	/**
+	 * Tests that a script module with an invalid fetchpriority value gets a value of auto.
+	 *
+	 * @ticket 61734
+	 *
+	 * @covers WP_Script_Modules::register
+	 * @expectedIncorrectUsage WP_Script_Modules::register
+	 */
+	public function test_register_script_module_having_fetchpriority_with_invalid_value() {
+		$this->script_modules->register( 'foo', '/foo.js', array(), false, array( 'fetchpriority' => 'silly' ) );
+		$reflection_class    = new ReflectionClass( $this->script_modules );
+		$registered_property = $reflection_class->getProperty( 'registered' );
+		$registered_modules  = $registered_property->getValue( $this->script_modules );
+		$this->assertSame( 'auto', $registered_modules['foo']['fetchpriority'] );
+		$this->assertArrayHasKey( 'WP_Script_Modules::register', $this->caught_doing_it_wrong );
+		$this->assertStringContainsString( 'Invalid fetchpriority `silly`', $this->caught_doing_it_wrong['WP_Script_Modules::register'] );
+	}
+
+	/**
+	 * Tests that a script module with an invalid fetchpriority value type gets a value of auto.
+	 *
+	 * @ticket 61734
+	 *
+	 * @covers WP_Script_Modules::register
+	 * @expectedIncorrectUsage WP_Script_Modules::register
+	 */
+	public function test_register_script_module_having_fetchpriority_with_invalid_value_type() {
+		$this->script_modules->register( 'foo', '/foo.js', array(), false, array( 'fetchpriority' => array( 'WHY AM I NOT A STRING???' ) ) );
+		$reflection_class    = new ReflectionClass( $this->script_modules );
+		$registered_property = $reflection_class->getProperty( 'registered' );
+		$registered_modules  = $registered_property->getValue( $this->script_modules );
+		$this->assertSame( 'auto', $registered_modules['foo']['fetchpriority'] );
+		$this->assertArrayHasKey( 'WP_Script_Modules::register', $this->caught_doing_it_wrong );
+		$this->assertStringContainsString( 'Invalid fetchpriority `array`', $this->caught_doing_it_wrong['WP_Script_Modules::register'] );
+	}
+
+	/**
+	 * Tests that a setting the fetchpriority for script module with an invalid value is ignored so that it remains auto.
+	 *
+	 * @ticket 61734
+	 *
+	 * @covers WP_Script_Modules::register
+	 * @covers WP_Script_Modules::set_fetchpriority
+	 * @expectedIncorrectUsage WP_Script_Modules::set_fetchpriority
+	 */
+	public function test_set_fetchpriority_with_invalid_value() {
+		$this->script_modules->register( 'foo', '/foo.js' );
+		$this->script_modules->set_fetchpriority( 'foo', 'silly' );
+		$reflection_class    = new ReflectionClass( $this->script_modules );
+		$registered_property = $reflection_class->getProperty( 'registered' );
+		$registered_modules  = $registered_property->getValue( $this->script_modules );
+		$this->assertSame( 'auto', $registered_modules['foo']['fetchpriority'] );
+	}
+
 	/**
 	 * Data provider.
 	 *