WordPress.org

Make WordPress Core

Opened 5 years ago

Last modified 8 days ago

#14853 accepted enhancement

wp_add_inline_script()

Reported by: mattwiebe Owned by: swissspidy
Milestone: 4.5 Priority: normal
Severity: normal Version: 3.1
Component: Script Loader Keywords: has-patch has-unit-tests
Focuses: Cc:

Description

There should be actions that fire after a script is printed. This would enabled this best-practices usage scenario (from http://github.com/paulirish/html5-boilerplate/blob/master/index.html ):

<!-- Grab Google CDN's jQuery. fall back to local if necessary -->
  <script src="http://ajax.googleapis.com/ajax/libs/jquery/1.4.2/jquery.min.js"></script>
  <script>!window.jQuery && document.write('<script src="js/jquery-1.4.2.min.js"><\/script>')</script>

The before action isn't covered by that use-case, but if we're going to have an after action, we might as well have a before one.

I've attached a patch that would allow for this. Dead simple.

Attachments (7)

wp-scripts-filters.diff (569 bytes) - added by mattwiebe 5 years ago.
wp-scripts-filter-2.diff (551 bytes) - added by mattwiebe 5 years ago.
14853.3.diff (3.8 KB) - added by atimmer 2 years ago.
class.wp-scripts.php.patch (1.0 KB) - added by abiralneupane 2 months ago.
wp-includes.patch (2.5 KB) - added by abiralneupane 2 months ago.
14853.diff (8.7 KB) - added by swissspidy 8 weeks ago.
14853.2.diff (10.5 KB) - added by swissspidy 3 weeks ago.

Download all attachments as: .zip

Change History (44)

#1 @mattwiebe
5 years ago

  • Cc mattwiebe added

Another example use-case that could make use of this from Typekit:

<script src="http://use.typekit.com/typekit-id.js"></script>
<!-- make use of wp_print_scripts_after-typekit -->
<script>try{Typekit.load();}catch(e){}</script>

And I'm sure tons more. It'd be nice for plugins to make use of WP methods rather than just spitting out their code in the wp_head or wp_footer actions.

#2 @mattwiebe
5 years ago

  • Keywords has-patch added

I thought about this some more, and a filter would be more appropriate than before/after actions. In addition to the benefits outlined above, this would allow a plugin to strip the type='text/javascript' when using HTML5 syntax.

#3 @mattwiebe
5 years ago

  • Summary changed from WP_Scripts::do_item before/after actions to WP_Scripts::do_item filter

This would also be useful for caching/minifying plugins. I've looked at WP Minify and the way it goes about its business is a bit of a nightmare. This would simplify things greatly.

#4 @westi
5 years ago

  • Keywords 3.2-early added
  • Owner set to westi
  • Status changed from new to accepted

I'll look at this when I go through and improve the Script Loader L10n support

#5 follow-up: @jczorkmid
5 years ago

  • Cc jczorkmid added

This would also be useful for doing the jQuery.noConflict() call when using an external jQuery.

#6 in reply to: ↑ 5 ; follow-up: @westi
5 years ago

Replying to jczorkmid:

This would also be useful for doing the jQuery.noConflict() call when using an external jQuery.

I'm hoping to move that call out of our bundled jQuery and into a second script on of these days :-)

#7 @nacin
5 years ago

  • Milestone changed from Awaiting Review to Future Release

#8 @scribu
5 years ago

Related: #16494

#9 @scribu
5 years ago

  • Keywords 3.2-early removed

A filter on the entire output would certainly allow the most flexibility, including the ability to change the attributes of <script> tags.

Note however that 'wp_print_scripts' is already used as an action. 'wp_print_script' would be more appropriate.

Another idea would be to introduce an add_inline_script() function, similar to the newly introduced add_inline_style() in [18464]. Example usage:

add_inline_script( 'jquery', '!window.jQuery && document.write(\'<script src="js/jquery-1.4.2.min.js"></script>\')' );

It could also accept a third parameter, $position, which could be 'before' or 'after'.

@atimmer
2 years ago

#10 @atimmer
2 years ago

  • Cc atimmermans@… added

14853.3.diff is a patch to add wp_add_inline_script the way scribu proposed. I really like this idea because it is consistent with wp_add_inline_style and it is very clear for developers.

I haven't added the possibility for the third parameter, because I couldn't think of any use case for 'before', if you can I will add the possibility.

Version 0, edited 2 years ago by atimmer (next)

#11 @nacin
2 years ago

  • Component changed from General to Script Loader

#12 @TobiasBg
14 months ago

#30617 was marked as a duplicate.

#13 follow-up: @aristath
8 months ago

Is there any reason why we haven't implemented this one yet?
If we're just waiting for a patch I could just start working on it...

#14 in reply to: ↑ 13 @samuelsidler
7 months ago

Replying to aristath:

Is there any reason why we haven't implemented this one yet?
If we're just waiting for a patch I could just start working on it...

I don't see any reason, but one may come up with a new patch. :) Working up a patch would be a good way to see what other issues exist.

#15 @johnbillion
7 months ago

  • Keywords needs-patch added; has-patch removed

#16 follow-up: @mcaskill
3 months ago

Maybe I'm just unfamiliar with SVN and WordPress' workflow but isn't this the patch?

#17 @swissspidy
2 months ago

#34784 was marked as a duplicate.

#18 in reply to: ↑ 16 @swissspidy
2 months ago

Replying to mcaskill:

Maybe I'm just unfamiliar with SVN and WordPress' workflow but isn't this the patch?

It is, but it's also over 5 years old and definitely needs a refresh.

#19 @abiralneupane
2 months ago

  • Focuses javascript added
  • Keywords has-patch added; needs-patch removed

Hello,

I have submitted patches (two files: class.wp-scripts.php.patch and wp-includes.patch).

Hope to see these function in upcoming WordPress version.

Thanks

#20 @swissspidy
2 months ago

  • Focuses javascript removed
  • Keywords needs-unit-tests added

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


8 weeks ago

#22 @abiralneupane
8 weeks ago

Hello,

Can someone do a unit testing on the patches? :)

@swissspidy
8 weeks ago

#23 follow-up: @swissspidy
8 weeks ago

  • Keywords has-unit-tests dev-feedback added; needs-unit-tests removed

14853.diff is a renewed patch that mirrors the functionality of WP_Styles for adding a wp_add_inline_script() function.

I've added a $position parameter to it as mentioned earlier in the ticket. This would be helpful when you need to set some variables before the script is loaded. What do other devs think about that?

Note that we can always remove that param in a further patch if it seems not worth doing.

Adds unit tests.

#24 in reply to: ↑ 23 @abiralneupane
8 weeks ago

That would be great feature to add

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


8 weeks ago

#26 @swissspidy
8 weeks ago

  • Owner changed from westi to swissspidy

#27 follow-up: @digamberpradhan
8 weeks ago

looks like this function will come handy, Is there something I can get involved with?

#28 in reply to: ↑ 27 @swissspidy
8 weeks ago

Replying to digamberpradhan:

looks like this function will come handy, Is there something I can get involved with?

At the moment I'd say: testing, testing, testing. Apply the patch on a local install (see the handbook for details how to do this) and try to develop some things using this function.
Does it work as expected or does it break? Is it intuitive, fast? Is the inline documentation clear?

#29 @swissspidy
6 weeks ago

  • Milestone changed from Future Release to 4.5
  • Summary changed from WP_Scripts::do_item filter to wp_add_inline_script()

#30 @swissspidy
4 weeks ago

  • Keywords dev-feedback removed

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


3 weeks ago

#32 in reply to: ↑ 6 @swissspidy
3 weeks ago

This would also be useful for doing the jQuery.noConflict() call when using an external jQuery.

I'm hoping to move that call out of our bundled jQuery and into a second script on of these days :-)

With the current patch, this would be as easy as calling $scripts->add_inline_script( 'jquery-core', 'jQuery.noConflict();' ); in script-loader.php.

#33 follow-up: @ocean90
3 weeks ago

  • Keywords needs-refresh needs-unit-tests added; has-unit-tests removed

14853.diff:

  • print_inline_script() returns always a boolean, but if $this->do_concat is true the expected return value is the script because of $this->print_code .= $this->print_inline_script( $handle, 'before', false );. print_inline_style() does it right in this case.
  • When using wp_add_inline_script( 'common', 'console.log("before");', 'before' ); the script is printed after the localized data, but in concat mode it's printed before. It should be always after the localized data IMO.
  • We don't have ID attributes for all the other script handling methods, is there a use case why add_inline_script() should add one? (Note: In concat mode the ID gets ignored.)
  • Beside jQuery.noConflict(); are there any other places where core could benefit of the new methods? Maybe wp_color_scheme_settings()? (For back-compat reasons I don't think we can remove the noConflict() line for a while.)

#34 in reply to: ↑ 33 @swissspidy
3 weeks ago

  • print_inline_script() returns always a boolean, but if $this->do_concat is true the expected return value is the script because of $this->print_code .= $this->print_inline_script( $handle, 'before', false );. print_inline_style() does it right in this case.
  • When using wp_add_inline_script( 'common', 'console.log("before");', 'before' ); the script is printed after the localized data, but in concat mode it's printed before. It should be always after the localized data IMO.

Yeah, that makes sense. Will update the patch.

  • We don't have ID attributes for all the other script handling methods, is there a use case why add_inline_script() should add one? (Note: In concat mode the ID gets ignored.)

I copied it from add_inline_style, but I agree that it's not really useful except that you know the handle it was added for. Doesn't hurt when it's absent.

  • Beside jQuery.noConflict(); are there any other places where core could benefit of the new methods? Maybe wp_color_scheme_settings()? (For back-compat reasons I don't think we can remove the noConflict() line for a while.)

I haven't found any other place where JavaScript is printed inline besides these two. It's mainly for developers I think.

@swissspidy
3 weeks ago

#35 @swissspidy
3 weeks ago

  • Keywords has-unit-tests added; needs-refresh needs-unit-tests removed

14853.2.diff includes two new tests for concatenated scripts, localized script data and even conditionals. It makes sure that conditionals (e.g. <!--[if gte IE 9]>) are always wrapped around inline scripts.

Contrary to to localized script data, inline scripts are added to $wp_scripts->print_html when the output should be concatenated. That's why test_wp_add_inline_script_concat() might look strange at first, but apparently that's the way it works.

@ocean90 Wanna have another look? :-)

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


8 days ago

Note: See TracTickets for help on using tickets.