WordPress.org

Make WordPress Core

Opened 2 years ago

Closed 8 months ago

Last modified 4 months ago

#19825 closed enhancement (wontfix)

Bugs and modifications to localize script

Reported by: ssmathias Owned by:
Milestone: Priority: normal
Severity: normal Version:
Component: General Keywords: close
Focuses: Cc:

Description

After working in trunk, I noted that I get a fatal error when trying to localize some objects with wp_localize_script informing me that I cannot use an object of that type as an array. For tests, I've done this with $wp, $wp_query, $wpdb, and $wp_scripts on a clean install with no plugins.

I also have been working on sites that would like to compress all of their JS calls into a single request where possible, but in the past I was not able to keep localizations that certain scripts required, and had to leave them out.

With the changes I've made in this patch file, the issues regarding pushing arbitrary objects for localization are addressed. It also stores those objects, rather than a string of JS with $wp_scripts. This allows me to pull the localizations used for some files, and attach them instead to another file using wp_localize_script.

As a side benefit, by not processing the object into JS until output, we are able to preserve the data cleanly, and reduce process on any script that may be dequeued later that has such dependencies.

I would like to submit this patch as a core patch for a future release. Its design is fully compatible with existing functionality in 3.3, with the exception of any code modifying the underlying data key in an enqueued script directly.

Attachments (1)

wp33patch.txt (2.3 KB) - added by ssmathias 2 years ago.
Core patch modifying localize scripts functionality

Download all attachments as: .zip

Change History (8)

ssmathias2 years ago

Core patch modifying localize scripts functionality

comment:1 follow-up: scribu2 years ago

This has been discussed extensively in #11520

comment:2 in reply to: ↑ 1 ; follow-up: ssmathias2 years ago

Replying to scribu:

This has been discussed extensively in #11520

That is true. I didn't see a reasoning for the string implementation versus storing the data, however, and believe this approach would ultimately serve better.

[The code in 11520] is also the code, as currently implemented in trunk, that is generating a fatal error when I attempt to pass an object into wp_localize_script.

Edit for clarity.

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

comment:3 in reply to: ↑ 2 ; follow-up: azaozz2 years ago

Replying to ssmathias:

[The code in 11520] is also the code, as currently implemented in trunk, that is generating a fatal error when I attempt to pass an object into wp_localize_script.

$wp_scripts->localize() accepts an associative array as argument, why would you pass an object? Further it has "dual functionality": that array can be multi-dimentional and html_entity_decode() is run only on the first level, leaving the second + levels untouched.

Don't see any advantages in storing the associative array(s) and json_encoding them at output (it actually used to do that at one point). This can only bring some collision problems when localize() is called more than once for the same script.

Also direct hacking into the $wp_scripts object is not supported as the internals can change in newer versions.

comment:4 in reply to: ↑ 3 ssmathias2 years ago

Replying to azaozz:

$wp_scripts->localize() accepts an associative array as argument, why would you pass an object? Further it has "dual functionality": that array can be multi-dimentional and html_entity_decode() is run only on the first level, leaving the second + levels untouched.

I don't have a good reason why I would pass an object at this time, but I don't see a reason not to support it, and therefore believe it should be supported. As far as the "dual functionality", that portion is still being handled the same way, with it only encoding the first level and leaving the deeper levels untouched.

Don't see any advantages in storing the associative array(s) and json_encoding them at output (it actually used to do that at one point). This can only bring some collision problems when localize() is called more than once for the same script.

There are a couple of reasons I would leave the data alone. The first is a philosophy my organization and I use, which is that data should be preserved and only changed for output when it is being output, unless there is good cause to change it in advance.

The second reason is because it does take some processing time to convert the data into the JavaScript string, and ultimately that script may be removed. If that is the case, then we've wasted some processing time that could be avoided otherwise. I will grant it's a small amount in all but the most extreme cases, but it does exist.

I don't see how we would have any more collision problems than already exist with the current functionality, in which you can also localize a script more than once. In one case, you'd have an array key overwritten with a new value, in another you'd have a JS variable overwritten with a new value. The same collision and resolution would occur, as I see it.

Also direct hacking into the $wp_scripts object is not supported as the internals can change in newer versions.

Exactly why I feel that such an interface as this would be necessary. Right now, for me to achieve the goals for one of our clients (which is to reduce ALL JS requests to a single request), I would have to dig down into the "data" key in the WP_Dependency object, copy that string, and manually replace it into the underlying WP_Dependency object on my concatenated script file. This actually forces me to tie more closely to the current implementation than I would like.

By being able to get the localized object off of one script through a supported method, then localize my concatenated script with wp_localize_script, I can avoid needing to know the underlying implementation of that localization functionality.

comment:5 bigdawggi2 years ago

  • Cc matt@… added

comment:6 c3mdigital8 months ago

  • Keywords close added
  • Resolution set to wontfix
  • Status changed from new to closed

No discussion in almost 2 years. Closing as wontfix for allowing objects to be passed to wp_localize_script(). If anyone has any further insight or would like to express a valid reason why we should allow objects to be passed please reopen.

comment:7 markoheijnen4 months ago

  • Milestone Awaiting Review deleted

Clearing out the milestones for closed tickets on Awaiting Review

Note: See TracTickets for help on using tickets.