Skip to content

Script Modules API: Rename wp_module functions to wp_script_module#5869

Closed
luisherranz wants to merge 8 commits intoWordPress:trunkfrom
luisherranz:rename-module-functions-to-script-modules
Closed

Script Modules API: Rename wp_module functions to wp_script_module#5869
luisherranz wants to merge 8 commits intoWordPress:trunkfrom
luisherranz:rename-module-functions-to-script-modules

Conversation

@luisherranz
Copy link
Copy Markdown
Member

@luisherranz luisherranz commented Jan 15, 2024 •
edited
Loading

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
Closed
@github-actions
Copy link
Copy Markdown

github-actions bot commented Jan 15, 2024

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.

@sirreal sirreal mentioned this pull request Jan 15, 2024
Closed
felixarntz
felixarntz approved these changes Jan 15, 2024
View reviewed changes
Copy link
Copy Markdown
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
Copy Markdown
Member Author

luisherranz commented Jan 15, 2024

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
Copy Markdown
Member

felixarntz commented Jan 15, 2024

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

@luisherranz
Copy link
Copy Markdown
Member Author

luisherranz commented Jan 16, 2024

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

@luisherranz luisherranz mentioned this pull request Jan 16, 2024
Closed
6 tasks
@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
@luisherranz luisherranz mentioned this pull request Jan 16, 2024
Closed
westonruter
westonruter reviewed Jan 16, 2024
View reviewed changes
westonruter
westonruter requested changes Jan 17, 2024
View reviewed changes
Copy link
Copy Markdown
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

kamranzafar4343
kamranzafar4343 reviewed Jan 18, 2024
View reviewed changes
Copy link
Copy Markdown

@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
@luisherranz @westonruter
Indentation and PHP 7 typing improvements
dfcdc6f 
Co-authored-by: Weston Ruter <westonruter@google.com>
@luisherranz
Re-indent
b6a9734
@luisherranz
Copy link
Copy Markdown
Member Author

luisherranz commented Jan 18, 2024

Suggested edits to phpdoc and added PHP 7 typing

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

westonruter
westonruter approved these changes Jan 19, 2024
View reviewed changes
Copy link
Copy Markdown
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.

@luisherranz @westonruter
Remove the array shape
65d91fa 
Co-authored-by: Weston Ruter <westonruter@google.com>
@luisherranz
Copy link
Copy Markdown
Member Author

luisherranz commented Jan 20, 2024

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

The PR is ready.

This was referenced Jan 22, 2024
Merged
Closed
@dmsnell
Copy link
Copy Markdown
Member

dmsnell commented Jan 23, 2024

Merged in [57327]
3c4fbc3

@dmsnell dmsnell closed this Jan 23, 2024
@luisherranz luisherranz mentioned this pull request Jan 25, 2024
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@swissspidy swissspidy swissspidy left review comments

@felixarntz felixarntz felixarntz approved these changes

@westonruter westonruter westonruter approved these changes

+1 more reviewer

@kamranzafar4343 kamranzafar4343 kamranzafar4343 left review comments

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

6 participants

@luisherranz @felixarntz @dmsnell @westonruter @swissspidy @kamranzafar4343