Skip to content
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.

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

Script Modules API: Rename wp_module functions to wp_script_module #5869

Conversation

luisherranz
Copy link
Member

@luisherranz luisherranz commented Jan 15, 2024

Trac ticket: https://core.trac.wordpress.org/ticket/56313


Renames all the mentions to "module" with "script module", including function names, comments, and tests.

The list of functions renamed are:

  • wp_module() -> wp_script_module()
  • wp_register_module() -> wp_register_script_module()
  • wp_enqueue_module() -> wp_enqueue_script_module()
  • wp_dequeue_module() -> wp_dequeue_script_module()
  • WP_Script_Modules::prints_enqueued_modules() -> WP_Script_Modules:: print_enqueued_script_modules()
  • WP_Script_Modules::print_module_preloads() -> WP_Script_Modules:: print_script_module_preloads()

It also adds PHP 7 typing to all the functions and improves the types of $dependencies argument of wp_register_script_module and wp_enqueue_script_module using @type.

@luisherranz luisherranz changed the title Rename wp...module functions to wp...script_module Rename wp_module functions to wp_script_module Jan 15, 2024
@luisherranz luisherranz marked this pull request as ready for review January 15, 2024 14:45
@luisherranz luisherranz mentioned this pull request Jan 15, 2024
Copy link

Test using WordPress Playground

The changes in this pull request can previewed and tested using a WordPress Playground instance.

WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser.

Some things to be aware of

  • The Plugin and Theme Directories cannot be accessed within Playground.
  • All changes will be lost when closing a tab with a Playground instance.
  • All changes will be lost when refreshing the page.
  • A fresh instance is created each time the link below is clicked.
  • Every time this pull request is updated, a new ZIP file containing all changes is created. If changes are not reflected in the Playground instance,
    it's possible that the most recent build failed, or has not completed. Check the list of workflow runs to be sure.

For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation.

Test this pull request with WordPress Playground.

Copy link
Member

@felixarntz felixarntz left a comment

Choose a reason for hiding this comment

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

Thanks @luisherranz, LGTM!

Just one point of feedback: Maybe we should also update the function docs to say "script module" instead of just "module"?

@luisherranz
Copy link
Member Author

Sure 🙂

Do you want me to remove the module word from the print methods as well?

  • print_enqueued_modules -> print_enqueued
  • print_module_preloads -> print_preloads

Or maybe change them to script_module instead?

  • print_enqueued_modules -> print_enqueued_script_modules
  • print_module_preloads -> print_script_module_preloads

@felixarntz
Copy link
Member

@luisherranz I think the latter, let's use script_module(s) everywhere.

@luisherranz
Copy link
Member Author

I've renamed all the references to "modules" and replaced them with "script modules".

@luisherranz luisherranz changed the title Rename wp_module functions to wp_script_module Script Modules API: Rename wp_module functions to wp_script_module Jan 16, 2024
Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

Suggested edits to phpdoc and added PHP 7 typing

src/wp-includes/class-wp-script-modules.php Outdated Show resolved Hide resolved
src/wp-includes/script-modules.php Outdated Show resolved Hide resolved
src/wp-includes/script-modules.php Outdated Show resolved Hide resolved
src/wp-includes/class-wp-script-modules.php Outdated Show resolved Hide resolved
src/wp-includes/class-wp-script-modules.php Outdated Show resolved Hide resolved
src/wp-includes/script-modules.php Outdated Show resolved Hide resolved
src/wp-includes/script-modules.php Outdated Show resolved Hide resolved
src/wp-includes/script-modules.php Outdated Show resolved Hide resolved
src/wp-includes/script-modules.php Outdated Show resolved Hide resolved
src/wp-includes/class-wp-script-modules.php Outdated Show resolved Hide resolved
Copy link

@kamranzafar4343 kamranzafar4343 left a comment

Choose a reason for hiding this comment

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

code looks fine to me

luisherranz and others added 2 commits January 18, 2024 08:19
Co-authored-by: Weston Ruter <westonruter@google.com>
@luisherranz
Copy link
Member Author

Suggested edits to phpdoc and added PHP 7 typing

Thanks, @westonruter. I've applied all the necessary changes.

Copy link
Member

@westonruter westonruter left a comment

Choose a reason for hiding this comment

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

Looks good, though my proposal for including the array shape may need to go.

src/wp-includes/class-wp-script-modules.php Outdated Show resolved Hide resolved
src/wp-includes/class-wp-script-modules.php Outdated Show resolved Hide resolved
src/wp-includes/script-modules.php Outdated Show resolved Hide resolved
src/wp-includes/script-modules.php Outdated Show resolved Hide resolved
Co-authored-by: Weston Ruter <westonruter@google.com>
@luisherranz
Copy link
Member Author

Ok, everything looks good now. Thanks for your help, @westonruter 🙂

The PR is ready.

@dmsnell
Copy link
Contributor

dmsnell commented Jan 23, 2024

Merged in [57327]
3c4fbc3

@dmsnell dmsnell closed this Jan 23, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants