Make WordPress Core

Opened 2 years ago

Last modified 4 weeks ago

#58873 reopened feature request

Add function to pass variables to scripts

Reported by: apedog's profile apedog Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version: 5.7
Component: Script Loader Keywords: needs-patch
Focuses: javascript, performance Cc:

Description

Since WordPress 5.7 using wp_localize_scripts to pass variables to registered scripts throws a _doing_it_wrong notice.

The suggested "correct" way of passing variables to scripts, using wp_add_inline_script, feels like a step backwards.

  • Instead of just passing a variable name and value through a PHP function, that abstracts all of this away, we now have to write fully qualified JavaScript code in PHP strings.
  • We have to actually parse/decode the values being passed on a case-by-case basis. wp_localize_script did this reliably.
  • Using wp_add_inline_script is more verbose and prone to errors. Static analysis tools will not work on PHP strings.
  • It makes for uglier code that is less human-readable.

On a personal note,
Converting to wp_add_inline_script feels a lot more like I'm "doing it wrong" than using wp_localize_script. It's simple, short and it works. (I do feel chagrined to see Query Monitor show up brown on every page load. But not enough to go and make my code uglier.)
I also doubt other "in the wild" developers will forego the use of wp_localize_script because of the _doing_it_wrong notice. The wp_add_inline_script alternative is simply not as robust.

Suggestion:
Add a function wp_pass_variables_to_script (or probably a shorter name) to re-introduce the robust functionality provided by wp_localize_script.

Change History (12)

#1 in reply to: ↑ description @jonsurrell
3 months ago

  • Milestone Awaiting Review deleted
  • Resolution set to worksforme
  • Status changed from new to closed

Since WordPress 5.7 using wp_localize_scripts to pass variables to registered scripts throws a _doing_it_wrong notice.

I don't see a general _doing_it_wrong notice. I do see a notice if the value is not an array.

I don't believe there's any general warning about using wp_localize_script(). If you provide a reproducible example of how the warning is generated, I'm happy to take a look.

I agree with much of your feedback about how data is passed to scripts. However, with these with these systems established, in place, and generally working well, it's hard to advocate to add another different system without really compelling justification.

One note, Script Modules, use a different system that's simpler. You can read about it here.

#2 @westonruter
3 months ago

@jonsurrell There is a downside to using wp_localize_script() in that it casts all top-level array items to strings, because this function is intended for i18n data. See #25280. Really, the better way to pass arbitrary data from PHP to a classic script is to use wp_add_inline_script(), although the DX is slightly less ergonomic.

<?php
wp_localize_script( $handle, 'foo', $data );

vs

<?php
wp_add_inline_script(
    $handle, 
    sprintf( 'var foo = %s;', wp_json_encode( $data, JSON_HEX_TAG | JSON_UNESCAPED_SLASHES ) ),
    'before'
);
Last edited 3 months ago by westonruter (previous) (diff)

#3 @jonsurrell
3 months ago

  • Keywords 2nd-opinion added
  • Resolution worksforme deleted
  • Status changed from closed to reopened

Thanks @westonruter, I didn't know that!

For what it's worth, I much prefer the "pull" style of passing data that's used by script modules with the script_module_data_{$module_id} filter. Some advantages:

  • The browser does not evaluate the script contents _at all_ because it's not a JavaScript script tag.
  • The data is parsed and used _on demand_ when necessary by the script that requires it.
  • No use of a shared global JavaScript namespace.

There's potential for improvement for classic scripts, but I'd be very wary of adding a third pattern for passing data to classic scripts. If anything, we could consider porting the script modules form to scripts to avoid a new pattern, but share an existing pattern.

I'll reopen for a second opinion.

Version 0, edited 3 months ago by jonsurrell (next)

#4 @westonruter
3 months ago

@jonsurrell What if we take your approach for script modules and make it available to classic scripts as well?

Before printing any inline before/localize/translations scripts in WP_Scripts, we apply script_data_{$handle} filters to obtain any data intended to be exported from PHP to JS, and then construct a JSON script to print first? This will then be available for classic scripts to parse out of the textContent on demand.

#5 @jonsurrell
2 months ago

I think that's a good idea since there are benefits over the existing options wp_add_inline_script() or wp_localize_script(). I'm concerned about introducing a third pattern that may not actually get much usage. Do you have ideas about how we could measure interest and impact?

It may be interesting to introduce the filter in Gutenberg and migrate some important scripts to use it. That could provide information about whether there's any impact and it's worth continuing.

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


2 months ago

#7 @westonruter
2 months ago

  • Keywords needs-patch added; 2nd-opinion removed
  • Milestone set to Future Release

@jonsurrell I've used the emoji loader as a case study for this in #63842. By eliminating the blocking inline script, I see a >5% improvement on LCP when emulating a low-tier mobile device.

#8 @jonsurrell
2 months ago

  • Focuses javascript performance added

Thanks for that exploration, it's very promising!

This is on the dev chat agenda to discuss today. If there are no objections raised, this should be ready to move ahead with development.

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


2 months ago

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


2 months ago

#11 @jonsurrell
6 weeks ago

This is ready for deveopment. The Script Modules implementation is here and can serve as inspiration:

https://core.trac.wordpress.org/browser/tags/6.8.2/src/wp-includes/class-wp-script-modules.php#L443

The implementation should be similar to the Script Modules implementation. I'd say the script tag containing the data should be printed before extra scripts (those added by wp_localize_script().

#12 @westonruter
4 weeks ago

In 60899:

Emoji: Convert the emoji loader from an inline blocking script to a (deferred) script module.

This modernizes the emoji loader script by converting it from a blocking inline script with an IIFE to a script module. Using a script module improves the performance of the FCP and LCP metrics since it does not block the HTML parser. Since script modules are deferred and run immediately before DOMContentLoaded, the logic to wait until that event is also now removed. Additionally, since the script is loaded as a module, it has been modernized to use const, let, and arrow functions. The sourceURL comment is also added. See #63887.

The emoji settings are now passed via a separate script tag with a type of application/json, following the new "pull" paradigm introduced for exporting data from PHP to script modules. See #58873. The JSON data is also now encoded in a more resilient way according to #63851. When the wp-emoji-loader.js script module executes, it continues to populate the window._wpemojiSettings global for backwards-compatibility for any extensions that may be using it.

A new uglify:emoji-loader grunt task is added which ensures wp-includes/js/wp-emoji-loader.js is processed as a module and that top-level symbols are minified.

Follow-up to [60681].

Props westonruter, jonsurrell, adamsilverstein, peterwilsoncc.
See #63851, #63887.
Fixes #63842.

Note: See TracTickets for help on using tickets.