Make WordPress Core

Opened 13 years ago

Closed 10 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
Priority: normal Milestone:
Component: Script Loader Version: 3.6
Severity: normal Keywords: needs-patch
Cc: Focuses:

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 13 years ago.

Download all attachments as: .zip

Change History (13)

#1 @nacin
13 years ago

  • Component GeneralScript Loader

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


12 years ago

#3 @wonderboymusic
12 years ago

  • Milestone Awaiting Review4.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
12 years ago

  • Owner set to wonderboymusic
  • Resolutionfixed
  • Status newclosed

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
12 years ago

  • Resolution fixed
  • Status closedreopened

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.

Version 0, edited 12 years ago by azaozz (next)

#6 @wonderboymusic
12 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 12 years ago by wonderboymusic (previous) (diff)

#7 @azaozz
12 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 12 years ago by azaozz (previous) (diff)

#8 @azaozz
12 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 12 years ago by azaozz (previous) (diff)

#9 @SergeyBiryukov
12 years ago

  • Keywords revert added

#10 @wonderboymusic
12 years ago

In 31189:

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

See #25277.

#11 @wonderboymusic
12 years ago

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

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

#12 @swissspidy
10 years ago

  • Milestone Future Release
  • Resolutionwontfix
  • Status reopenedclosed

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.