Use global variable in wp_script_modules()

[swissspidy]

Only noticed this while looking at #5888

wp_script_modules() is an odd function because a) it doesn't use a get_instance() method like elsewhere in core, b) it does more than just returning an instance. The first time you call it, it adds hooks.

Curious to hear thoughts on this alternative suggestion.

cc @luisherranz

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

---

This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.

[github-actions]

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.

[luisherranz]

The idea of wp_script_modules() came from @felixarntz in these comments of the original PR:

I would recommend to make everything here non-static and instead introduce a simple wp_modules() function that encapsulates the default/main instance of the class, similar to how the existing wp_scripts() and wp_styles() functions do.

The idea to avoid loading/instantiating the class early in the WordPress bootstrap also came from Felix:

It would be great if you could introduce wrapper functions for these 3 as well (i.e. wp_print_import_map() etc.). This way you can reference the functions here, which avoids loading/instantiating the class early in the WordPress bootstrap process where it may not be needed.

Not a big deal, but an easy enough optimization IMO worth making.

Taking into account what Felix said, I thought it would be even nicer to add the hooks inside the first call to the wp_script_modules() function. That not only prevents the hooks from being added early in the WordPress bootstrap, but doesn't add them at all if nobody is using modules.

Again, as Felix said, it's probably not a big deal, just an optimization.

I don't have strong preferences over any of the options, though, so feel free to change it if you want 🙂

[swissspidy]

Hmm I see.

Well in that case I suppose keeping the function is fine, but not a fan of adding the hooks in there as well.

[swissspidy]

Committed in https://core.trac.wordpress.org/changeset/57503

[luisherranz]

Could you please take a look at the class initialization at #5953? I'm doing something similar there to what I did here, and I don't know what the correct way to do it would be.

Thanks 🙏