Make WordPress Core

Opened 5 years ago

Last modified 3 months ago

#52320 new defect (bug)

Empty entries in WP_Scripts 'extra' field

Reported by: vanyukov's profile vanyukov Owned by:
Milestone: Awaiting Review Priority: normal
Severity: trivial Version: 4.5
Component: Script Loader Keywords: needs-patch
Focuses: performance, coding-standards Cc:

Description

When WordPress registers all the scripts in wp_default_scripts() the add_inline_script() will often add false to the extra field of the script. That false value does not do anything and can be easily removed by comparing against empty data from get_data() in add_inline_script(). Doesn't look like this has any effect on anything, as false values will later on be ignored. But this will save a few extra bytes of memory.

Steps to verify:

  • Get the $wp_scripts global
  • Check some registered scripts, for example $wp_scripts->registered->editor->extra['after']. It will have two array values - one is false and one is the actual data window.wp.oldEditor = window.wp.editor;

Or via debugging by putting a breakpoint in wp-includes/class.wp-scripts.php in add_inline_script() for $script = (array) $this->get_data( $handle, $position );

Change History (2)

#1 @hellofromTonya
5 years ago

  • Version changed from trunk to 4.5

#2 @jonsurrell
3 months ago

  • Focuses performance coding-standards added
  • Keywords needs-patch added

This is a good spot, I've verified the behavior in trunk (6.8.2 is the latest release). It seems that when wp_add_inline_script() (or WP_Scripts::add_inline_script()) is called, this line causes the array to start as array( false ). WP_Scripts::get_data() returns `false` if the script has no associated data and (array) false coerces to this array with a false element:

<?php
var_dump( (array) false );

prints

array(1) {
  [0]=>
  bool(false)
}

It certainly seems like that first false element was not intended and isn't desirable. It should likely start as an empty array.

This has been the case since ::add_inline_script() was added in [36633].


One thing to consider, I believe the following could be used to set the data to false:

<?php
$wp_scripts->add_data(
  $handle,
  'after', /* or 'before' */
  false
);

One could argue that coercing to array( false ) is the correct thing to do in that case. However, it seems unlikely that anyone is relying on that behavior. A check like this seems reasonable:

<?php
$data   = $this->get_data( $handle, $position );
$script = false === $data ? array() : (array) $data;
Note: See TracTickets for help on using tickets.