Make WordPress Core

Opened 3 years ago

Last modified 7 weeks ago

#60597 assigned enhancement

Script Modules API: Allow list of enqueued module data to be exposed

Reported by: johnbillion Owned by: westonruter
Priority: normal Milestone: 7.2
Component: Script Loader Version: 6.5
Severity: normal Keywords: has-patch has-unit-tests
Cc: Focuses:

Description

There's no means for a plugin to fetch a list of registered or enqueued script modules for debugging purposes from the Script Modules API as there is from the existing Scripts Dependencies API.

Most likely the get_import_map(), get_marked_for_enqueue(), get_dependencies(), and get_versioned_src() methods of the WP_Script_Modules should be made public to facilitate this.

Change History (30)

This ticket was mentioned in ​PR #6155 on ​WordPress/wordpress-develop by ​@johnbillion.


3 years ago
#1

  • Keywords has-patch added; needs-patch removed

#2 @johnbillion
3 years ago

  • Milestone Awaiting Review → 6.5
  • Type enhancement → task (blessed)

#3 @swissspidy
3 years ago

Are we comfortable with making all of these public and maintaining backward compatibility going forward?

Now would be the last chance to change anything about the function signature etc.

#4 @swissspidy
3 years ago

cc @luisherranz @westonruter

#5 @luisherranz
3 years ago

I think it's too soon to expose the internals.

I'd rather expose another function that only returns the list of enqueued/registered module identifiers.

#6 @johnbillion
3 years ago

I don't think this can really be considered "internals". The public print_import_map() method mostly just prints a JSON encoded representation of the data from the get_import_map() method which in turn constructs its data from the methods that I'm proposing should be public.

In order for my Query Monitor debugging plugin to show info about script modules in the same way that it does for enqueued scripts and styles, it needs to be able to fetch a list of enqueued modules (get_marked_for_enqueue()), their dependencies (get_dependencies()), and the full source URL (get_versioned_src()).

#7 @luisherranz
3 years ago

get_marked_for_enqueue and get_dependencies return a list of script module arrays, not a list of script module identifiers (strings). Those script module arrays are the arrays stored in $registered, and contain all the internal information about a script module.

So instead of making the $registered arrays public, which would mean they can't be refactored in the future, I would expose functions that return the script module identifiers (strings) only. If you need more information than that, like the corresponding src or dependencies of a script module identifier, I would create additional public functions that return a single piece of information (the src, the list of dependencies, etc).

That's just my opinion, though.

#8 @swissspidy
3 years ago

Tough one! I understand both sides, though I am probably leaning towards creating dedicated methods for this as suggested by Luis. This would be cleaner and wouldn't put us into a bad spot regarding BC. That would be an enhancement for early 6.6 though in my eyes, given how late in the release cycle we are. It's not ideal for plugins such as Query Monitor, but there might be workarounds.

What do you think?

#9 @johnbillion
3 years ago

  • Keywords needs-patch added; has-patch removed
  • Milestone 6.5 → 6.6
  • Type task (blessed) → enhancement

Alrighty let's bump this. I should be able to work around this via the magic of the reflection API for now.

This ticket was mentioned in ​Slack in #core by nhrrob. ​View the logs.


2 years ago

This ticket was mentioned in ​Slack in #core by oglekler. ​View the logs.


2 years ago

#12 @oglekler
2 years ago

  • Milestone 6.6 → 6.7

No movement in 3 months, I am moving this ticket to the 6.7 milestone

This ticket was mentioned in ​Slack in #core by chaion07. ​View the logs.


2 years ago

#14 @chaion07
2 years ago

  • Milestone 6.7 → Future Release

Thanks @johnbillion for reporting this. We discussed this Ticket during a recent bug-scrub session. Based on the feedback received we are updating the milestone to Future Release and if anyone is willing to contribute then they can simply update the milestone back to 6.7 🙏

Props to @mukesh27

Cheers!

#15 @gziolo
2 years ago

This came up as in issue again when working on enhancements in the Gutenberg plugin:

​https://github.com/WordPress/gutenberg/pull/65380/files#r1761621029

Re-sharing my feedback:

It's about time to take action in WordPress core and expose the missing functionality for extenders:

  • get_import_map should become public, it contains imports field only, so it can get extended but it's enough for now as it's exactly the same shape that gets printed in HTML.
  • add public method is_marked_for_enqueue to verify if a script tag will get printed in HTML
  • add public method is_marked_for_import to verify if a script module will be listed in the import map

Bonus points for adding a filter that gets called before returning the value from get_import_map call so plugins could apply their changes.

I guess it also makes sense to promote get_marked_for_enqueue to public to have the same level of access for both concepts.

Last edited 2 years ago by gziolo (previous) (diff)

#16 @jonsurrell
2 years ago

The use case I've wanted most often is a method to know what Script Modules are "available" - either enqueued themselves or part of the import map because they're in the dependency graph.

In ​https://github.com/WordPress/gutenberg/pull/65380, the use case was "is this specific Script Module available?"

Previously, ​I needed a way to iterate through all available Script Modules. That implementation moved to Core in [58579] and was able to access private methods to achieve its goal.

Last edited 2 years ago by jonsurrell (previous) (diff)

#17 @justlevine
23 months ago

Another use case:

I'm working to add Block Theme support for WPGraphQL, for which we need to be able to return the array of enqueued script modules so a headless frontend can generate and output the corresponding <script> tags (alongside the traditional $wp_scripts).

Similar use case to John's Query Monitor, except this is for frontend rendering, not an admin tool, so we ideally want to avoid the reflection penalty.

#18 @gziolo
23 months ago

  • Milestone Future Release → 6.8

This ticket was mentioned in ​PR #8069 on ​WordPress/wordpress-develop by ​@sukhendu2002.


21 months ago
#19

  • Keywords has-patch has-unit-tests added; needs-patch removed

This ticket was mentioned in ​Slack in #core by audrasjb. ​View the logs.


20 months ago

#21 @johnbillion
20 months ago

  • Milestone 6.8 → Future Release

#22 @westonruter
12 months ago

  • Owner set to westonruter
  • Status new → assigned

#23 @westonruter
12 months ago

Also discussed in #61734 starting at this comment.

Note that WP_Script_Moules::get_marked_for_enqueue() now has a public method available as of 6.9 in r60951 for #63676: WP_Script_Modiles::get_queue().

Last edited 12 months ago by westonruter (previous) (diff)

#24 @westonruter
11 months ago

In 61073:

Script Loader: Restore original return value for WP_Script_Modules::get_dependencies().

Even though this method is private, there are some usages of it in the ecosystem via the Reflection API. So this reverts the script module IDs string[] return value in favor of the original array<string, array> return value. This re-emphasizes the need for more public accessor methods being tracked in #60597.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/10403

Follow-up to [60999].

Props pbiron, westonruter, johnbillion.
See #63486, #60597.

#25 @westonruter
8 months ago

  • Milestone Future Release → 7.1

#26 @westonruter
5 months ago

In 62278:

I18N: Add translation support for script modules.

Add automatic translation loading for script modules (ES modules), so strings using __() and friends from @wordpress/i18n can be translated at runtime. This brings classic script i18n parity to script modules registered via wp_register_script_module(), which previously had no way to load translation data, leaving strings untranslated on screens like Connectors and Fonts that are built as script modules.

At the admin_print_footer_scripts and wp_footer actions, every enqueued script module and its dependencies are walked, the translation chunk is loaded for each, and an inline <script> calls wp.i18n.setLocaleData() so translations are available before deferred modules execute. Note there is currently a runtime dependency on the wp-i18n classic script, which is printed just-in-time if not already enqueued. This coupling is to be removed in a future release.

Public API:

  • WP_Script_Modules::set_translations() stores the text domain (and optional path) per registered module to override the text domain and path. A global wp_set_script_module_translations() function is added as a wrapper around wp_script_modules()->set_translations().
  • WP_Script_Modules::get_registered() obtains a registered module's data. See #60597.
  • WP_Script_Modules::print_script_module_translations() emits inline wp.i18n.setLocaleData() calls after classic scripts load but before modules execute.
  • load_script_module_textdomain() loads the translation data for a given script module ID and text domain.
  • The existing load_script_textdomain_relative_path filter gains a third $is_module parameter so callers can distinguish classic-script and script-module lookups when resolving translation paths.

PHPStan types are also added in WP_Script_Modules. See #64238.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/11543

Props manzoorwanijk, westonruter, jsnajdr, jonsurrell, mukesh27, peterwilsoncc, 369work, desrosj, sabernhardt, nilambar, jorgefilipecosta, malayladu.
See #64238, #60597.
Fixes #65015.

#27 @peterwilsoncc
5 months ago

In 62286:

I18N: Add translation support for script modules.

Add automatic translation loading for script modules (ES modules), so strings using __() and friends from @wordpress/i18n can be translated at runtime. This brings classic script i18n parity to script modules registered via wp_register_script_module(), which previously had no way to load translation data, leaving strings untranslated on screens like Connectors and Fonts that are built as script modules.

At the admin_print_footer_scripts and wp_footer actions, every enqueued script module and its dependencies are walked, the translation chunk is loaded for each, and an inline <script> calls wp.i18n.setLocaleData() so translations are available before deferred modules execute. Note there is currently a runtime dependency on the wp-i18n classic script, which is printed just-in-time if not already enqueued. This coupling is to be removed in a future release.

Public API:

  • WP_Script_Modules::set_translations() stores the text domain (and optional path) per registered module to override the text domain and path. A global wp_set_script_module_translations() function is added as a wrapper around wp_script_modules()->set_translations().
  • WP_Script_Modules::get_registered() obtains a registered module's data. See #60597.
  • WP_Script_Modules::print_script_module_translations() emits inline wp.i18n.setLocaleData() calls after classic scripts load but before modules execute.
  • load_script_module_textdomain() loads the translation data for a given script module ID and text domain.
  • The existing load_script_textdomain_relative_path filter gains a third $is_module parameter so callers can distinguish classic-script and script-module lookups when resolving translation paths.

PHPStan types are also added in WP_Script_Modules. See #64238.

Developed in ​https://github.com/WordPress/wordpress-develop/pull/11543

Reviewed by audrasjb.
Merges r62278 to the 7.0 branch.

Props manzoorwanijk, westonruter, jsnajdr, jonsurrell, mukesh27, peterwilsoncc, 369work, desrosj, sabernhardt, nilambar, jorgefilipecosta, malayladu.
See #64238, #60597.
Fixes #65015.

#28 @wildworks
3 months ago

I would like to check the status of this ticket. Is there anything else we need to address? If not, I would like to close this ticket.

#29 @westonruter
3 months ago

  • Milestone 7.1 → 7.2

Additional methods are needed to expose the internals. So I'm punting to the next release.

#30 @jonsurrell
7 weeks ago

​This comment and surrounding discussion from @jsnajdr describes more extension points that would be useful for Script Modules.

Note: See TracTickets for help on using tickets.