Skip to content

Script Modules API: Add deregister module function #6061

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

Closed
cbravobernal wants to merge 5 commits into from
Closed

Script Modules API: Add deregister module function #6061

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
af0516a
Add deregister module function
cbravobernal Feb 7, 2024
e257287
Add ticket number on test
cbravobernal Feb 7, 2024
11d2ea7
Update suggestions
cbravobernal Feb 8, 2024
e3a254c
Add more tests, indent comments
cbravobernal Feb 12, 2024
a33f12e
Reword test descriptions
cbravobernal Feb 12, 2024
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
12 changes: 12 additions & 0 deletions src/wp-includes/class-wp-script-modules.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -151,6 +151,18 @@ public function dequeue( string $id ) {
unset( $this->enqueued_before_registered[ $id ] );
}

/**
* Removes a registered script module.
*
* @since 6.5.0
*
* @param string $id The identifier of the script module.
*/
public function deregister( string $id ) {
Copy link
Contributor

@youknowriad youknowriad Feb 12, 2024

Choose a reason for hiding this comment

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

Is it "unregister" or "deregister"? It seems on Core, we use "unregister" everywhere except for WP_Scripts and Styles. It seems a bit awkward.

I understand the reasoning about modules being scripts to keep them consistent but wanted to mention.

Copy link
Member

@gziolo gziolo Feb 12, 2024

Choose a reason for hiding this comment

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

Right, that's unfortunate. It's probably better to keep it awkward but similar to scripts. I don't have strong opinion here though 😅

Copy link
Member

@gziolo gziolo Feb 12, 2024

Choose a reason for hiding this comment

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

It's even more interesting:

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

Line 278 in 72bc9c0

function wp_deregister_script( $handle ) {

but:

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

Line 334 in 72bc9c0

wp_scripts()->remove( $handle );

unset( $this->registered[ $id ] );
unset( $this->enqueued_before_registered[ $id ] );
}

/**
* Adds the hooks to print the import map, enqueued script modules and script
* module preloads.
Expand Down
11 changes: 11 additions & 0 deletions src/wp-includes/script-modules.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,3 +112,14 @@ function wp_enqueue_script_module( string $id, string $src = '', array $deps = a
function wp_dequeue_script_module( string $id ) {
wp_script_modules()->dequeue( $id );
}

/**
* Deregisters the script module.
*
* @since 6.5.0
*
* @param string $id The identifier of the script module.
*/
function wp_deregister_script_module( string $id ) {
wp_script_modules()->deregister( $id );
}
73 changes: 73 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 @@ -126,6 +126,79 @@ public function test_wp_dequeue_script_module() {
$this->assertTrue( isset( $enqueued_script_modules['bar'] ) );
}


/**
* Tests that a script module can be deregistered
* after being enqueued, and that will be removed
* from the enqueue list too.
*
* @ticket 60463
*
* @covers ::register()
* @covers ::enqueue()
* @covers ::deregister()
* @covers ::get_enqueued_script_modules()
*/
public function test_wp_deregister_script_module() {
$this->script_modules->register( 'foo', '/foo.js' );
$this->script_modules->register( 'bar', '/bar.js' );
$this->script_modules->enqueue( 'foo' );
$this->script_modules->enqueue( 'bar' );
$this->script_modules->deregister( 'foo' ); // Dequeued.

$enqueued_script_modules = $this->get_enqueued_script_modules();

$this->assertCount( 1, $enqueued_script_modules );
$this->assertFalse( isset( $enqueued_script_modules['foo'] ) );
$this->assertTrue( isset( $enqueued_script_modules['bar'] ) );
}

/**
* Tests that a script module is not deregistered
* if it has not been registered before, causing
* no errors.
*
* @ticket 60463
*
* @covers ::deregister()
* @covers ::get_enqueued_script_modules()
*/
public function test_wp_deregister_unexistent_script_module() {
$this->script_modules->deregister( 'unexistent' );
$enqueued_script_modules = $this->get_enqueued_script_modules();

$this->assertCount( 0, $enqueued_script_modules );
$this->assertFalse( isset( $enqueued_script_modules['unexistent'] ) );
}

/**
* Tests that a script module is not deregistered
* if it has been deregistered previously, causing
* no errors.
*
* @ticket 60463
*
* @covers ::get_enqueued_script_modules()
* @covers ::register()
* @covers ::deregister()
* @covers ::enqueue()
*/
public function test_wp_deregister_already_deregistered_script_module() {
$this->script_modules->register( 'foo', '/foo.js' );
$this->script_modules->enqueue( 'foo' );
$this->script_modules->deregister( 'foo' ); // Dequeued.
$enqueued_script_modules = $this->get_enqueued_script_modules();

$this->assertCount( 0, $enqueued_script_modules );
$this->assertFalse( isset( $enqueued_script_modules['foo'] ) );

$this->script_modules->deregister( 'foo' ); // Dequeued.
$enqueued_script_modules = $this->get_enqueued_script_modules();

$this->assertCount( 0, $enqueued_script_modules );
$this->assertFalse( isset( $enqueued_script_modules['foo'] ) );
}

/**
* Tests that a script module can be enqueued before it is registered, and will
* be handled correctly once registered.
Expand Down