WordPress.org

Make WordPress Core

Opened 7 years ago

Closed 4 years ago

#25277 closed feature request (wontfix)

WP_scripts does not allow to add data after the enque has been added to HTML

Reported by: hakre Owned by: wonderboymusic
Milestone: Priority: normal
Severity: normal Version: 3.6
Component: Script Loader Keywords: needs-patch
Focuses: Cc:

Description

For example if a template part makes use of a jQuery plugin that needs jQuery but additionally some javascript code to initialize the plugin which is not inside a js file, the data "data" can not be used to set that initialization code for the enqueued script becuse it would be put before the resource is loaded:

Example Code:

$wp_scripts->add("jquery.webticker", get_stylesheet_directory_uri() . "/js/jquery.webticker.js", array("jquery"));
$wp_scripts->add_data("jquery.webticker", "data", "jQuery('#webticker').webTicker();");
$wp_scripts->enqueue("jquery.webticker");

Example Output:

<script type='text/javascript'>
/* <![CDATA[ */
jQuery('#webticker').webTicker();
/* ]]> */
</script>
<script type='text/javascript' src='http://example.com/wp-content/themes/example/js/jquery.webticker.js?ver=3.6'></script>

This feature request suggests to allow an additional key to be used instead of "data" (here exemplary "data-after") to place script contents *after* the enqueued script:

Example Code (after patch):

$wp_scripts->add("jquery.webticker", get_stylesheet_directory_uri() . "/js/jquery.webticker.js", array("jquery"));
$wp_scripts->add_data("jquery.webticker", "data-after", "jQuery('#webticker').webTicker();");
                                          ############
$wp_scripts->enqueue("jquery.webticker");

Example Output (after patch):

<script type='text/javascript' src='http://example.com/wp-content/themes/example/js/jquery.webticker.js?ver=3.6'></script>
<script type='text/javascript'>
/* <![CDATA[ */
jQuery('#webticker').webTicker();
/* ]]> */
</script>

Attachments (1)

wp-25277-data-after.patch (7.0 KB) - added by hakre 7 years ago.

Download all attachments as: .zip

Change History (13)

#1 @nacin
7 years ago

  • Component changed from General to Script Loader

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


6 years ago

#3 @wonderboymusic
6 years ago

  • Milestone changed from Awaiting Review to 4.2

This is a good idea:

$scripts->add_data( 'utils', 'data', 'var cool = ' . json_encode( array( 'stuff' => 1 ) ) . ";" );
$scripts->add_data( 'utils', 'data-after', 'new Boom();' );

#4 @wonderboymusic
6 years ago

  • Owner set to wonderboymusic
  • Resolution set to fixed
  • Status changed from new to closed

In 31032:

Add the ability to print data *after* a script, whether it is concatenated or not:

  • Add a third argument to WP_Scripts->print_extra_script(), $key, which will be passed to ->get_data() (no longer passes hardcoded 'data')
  • When $key is set to 'data-after', the inline script will be printed after the <script> tag. If the scripts are being concatenated, all scripts' 'data-after' data will be printed after the concatenated <script> has been rendered.

Props hakre, wonderboymusic.
Fixes #25277.

#5 @azaozz
6 years ago

  • Resolution fixed deleted
  • Status changed from closed to reopened

We are getting into JS land here. Don't think this is a good idea, mostly because it doesn't guarantee the "data-after" will be immediately after the actual script. It will be after all scripts when concatenating. That makes it pretty much unusable.

For the very rare cases where this might be needed, it can be done from PHP by using the same action with higher priority. No need to add specific support/hand-holding in WP_Scripts.

(Oh, and print_after_html name is misleading, there cannot be any HTML in there, it is the actual script.)

Last edited 6 years ago by azaozz (previous) (diff)

#6 @wonderboymusic
6 years ago

yeah.... so the idea was just to allow something to output after, since you can currently only register something to output before ....

Where this could be useful, if the result ends up being:

<script>
var settingsBlob = {
    apiKey: 'q1w2e3r4t5'
};
</script>
<script src="lib/api.js"></script>
<script>
new API( settings );
</script>

This works great when the file in the middle doesn't produce side-effects (most of the WP files produce side-effects ... )

Passing in JS as a string to $wp_scripts->add_data() sucks, might be better if it also took a callable, which wp_localize_script() now does.

When concatenation is used, the choices are:

  • Output the data right after the script within the concat blob
  • Output all data at once after the concatenated script (which I have done) - once again, could get weird if files produce side effects.

Another potential issue - if someone enqueues the script but doesn't want the data.

I'll leave the decision here to you @azaozz. I am not married to this, a revert would not hurt my feelings.

Last edited 6 years ago by wonderboymusic (previous) (diff)

#7 @azaozz
6 years ago

  • Keywords 2nd-opinion added

I am not married to this, a revert would not hurt my feelings.

I'm leaning towards reverting as this may be more confusing than it's worth.

The "before" is handy for providing data needed for the script so we can initialize it as soon as loaded. Good thing is that data can be outputted at any place before the <script> tag.

This part:

<script>
new API( settings );
</script>

is pretty much always handled from the JS that would use that API.

The only two cases where "after" may be useful would be:

  • For the jQuery.noConflict().
  • "Duck punching" that API. Very undesirable.

In both cases it is crucial for the "after" to be immediately after the <script> tag. However when concatenating the default scripts, we cannot add it inside the blob. The reason is WP doesn't run when outputting concatenated scripts and styles, or it would have to run three times on every page load (load-scripts.php and load-styles.php use script-loader as a whitelist only). So if a plugin adds/removes/changes the "after", we cannot catch that.

In that terms I don't think we need to have this functionality for scripts. On the other hand this is useful for styles (was added in 3.3), and the placement there is not that critical.

Last edited 6 years ago by azaozz (previous) (diff)

#8 @azaozz
6 years ago

Also, looking at the "Example Output (after patch)" from the ticket description:

<script type='text/javascript' src='http://example.com/.../jquery.webticker.js'></script>
<script type='text/javascript'>
/* <![CDATA[ */
jQuery('#webticker').webTicker();
/* ]]> */
</script>

The "after" script is best run on jQuery( document ).ready() or last in the footer, so jQuery('#webticker') can be selected properly. It is possible that the script and the "after" are outputted in the HTML head.

Using an action like admin_print_footer_scripts or wp_print_footer_scripts to output it would be better/more appropriate.

Last edited 6 years ago by azaozz (previous) (diff)

#9 @SergeyBiryukov
6 years ago

  • Keywords revert added

#10 @wonderboymusic
6 years ago

In 31189:

Revert [31032], this did not get much love.

See #25277.

#11 @wonderboymusic
6 years ago

  • Keywords needs-patch added; has-patch 2nd-opinion revert removed
  • Milestone changed from 4.2 to Future Release

Perhaps a different implementation? Not sure what is useful here.

#12 @swissspidy
4 years ago

  • Milestone Future Release deleted
  • Resolution set to wontfix
  • Status changed from reopened to closed

Closing this after no traction for over 2 years. Also, we added wp_add_inline_script() a while ago to make things like jQuery.noConflict() easier.

Note: See TracTickets for help on using tickets.