WordPress.org

Make WordPress Core

Opened 4 years ago

Last modified 4 weeks ago

#22249 assigned feature request

Add ability to set or remove attributes on enqueued scripts and styles.

Reported by: ryanve Owned by:
Milestone: Future Release Priority: normal
Severity: normal Version:
Component: Script Loader Keywords: needs-refresh early needs-patch
Focuses: Cc:

Description

I think it should be easier to customize the loading of scripts and styles (easier to customize the markup generated by the script/style system). Proposed solutions:

Solution 1: Allow wp_enqueue_script, wp_enqueue_style, wp_register_script, wp_register_style to accept an array of attributes as the $src parameter. For example:

wp_enqueue_script( 'my-plugin', array(
    'src' => 'http://example.com/js/app.js'
    'defer' => ''
    'data-my-plugin' => 'custom data attr value'
), array('jquery'), null, true );

Solution 2: Add a filter before the markup is generated that allows devs to filter the attributes while they are in array format. For example:

add_filter('script_loader_attrs', function ($attrs, $handle) {
    unset ( $attrs['type'] );
    'my-plugin' === $handle and $attrs['data-my-plugin'] = 'plugin data';
    $attrs['src'] = remove_query_arg( $attrs['src'] );
    return $attrs;
}, 12, 2);

In class.wp-scripts.php it might look something like:

$attrs = (array) apply_filters('script_loader_attrs', $attrs, $handle);

and/or:

$attrs = (array) apply_filters("{$handle}_script_loader_attrs", $attrs );

I imagine that solution 2 would be easier to implement than 1, and 2 allows for themes/plugins to modify scripts/styles w/o re-registering resources.

The key feature of both solutions is the ability to modify the attrs while in array format. There are other ways that one could achieve the same results, but the array is by far the cleanest. Dirty alternatives include:

  • Use preg_replace() on the markup after it is generated (see #22245)
  • Use output buffers and PHP's DOMElement interface
  • Filter away the "print_scripts_array" and regenerate the markupmanually.)

Attachments (9)

22249.diff (4.1 KB) - added by jtsternberg 2 years ago.
New method, get_script_attributes, to concatenate an array of script attributes passed through a filter.
22249-2.diff (9.4 KB) - added by jtsternberg 18 months ago.
Modifications based on feedback from boonebgorges and jeremyfelt
22249-3.diff (21.2 KB) - added by camdensegal 18 months ago.
Added enqueue/register styles attribute argument.
22249-4.diff (21.2 KB) - added by alex-ye 18 months ago.
22249.2.diff (6.8 KB) - added by ericlewis 5 months ago.
22249.3.diff (10.7 KB) - added by ericlewis 3 months ago.
22249.4.diff (13.6 KB) - added by ericlewis 3 months ago.
22249.5.diff (13.7 KB) - added by ericlewis 3 months ago.
22249.6.diff (16.3 KB) - added by ericlewis 5 weeks ago.

Download all attachments as: .zip

Change History (47)

#1 @alex-ye
4 years ago

  • Cc nashwan.doaqan@… added

Yes It might be a good idea , I was needing something like this when I was using LessCSS sometimes you need to change "rel" value to "stylesheet/less" .

#2 @jtsternberg
4 years ago

  • Cc justin@… added

This is also needed for some 3rd party scripts. Outbrain looks like:

<script type="text/javascript" async="async" src="http://widgets.outbrain.com/outbrain.js"></script>

#3 @bpetty
3 years ago

Related: #19742

#4 @ryanve
3 years ago

#23236 would make this easier.

#5 @raphaelvalerio
3 years ago

This feature would also be useful for removing type='text/javascript', since it's redundant in HTML5. As far as I know, currently the only way of using a <script> element without the type attribute is to hardcode it into your theme.

Styles do have a filter hook you can use to alter the markup:

add_filters( 'style_loader_tag' )

(see wp-includes/class.wp-styles.php, line 83)

I haven't tried hooking into this filter, but I think the whole <link> element is a string, not an array, which could be improved upon. But at least it's hookable.

Scripts, on the other hand, have no filter to hook. Not only that, but the <script> element is printed to the site with a simple echo statement right in the function, so there's no variable to modify.

This happens in wp-includes/class.wp-scripts.php, in the do_item() function on line 125. I toyed around with adding in a filter. Although this doesn't correspond exactly to what you're asking, here's one solution for the script tag side of things:

Replace lines 122-125 of wp-includes/class.wp-scripts.php with:

$attrs = array( 'type' => 'text/javascript' );

$attrs = apply_filters( 'script_loader_attrs', $attrs );

$attrs_string = '';

if(!empty( $attrs ) ) {
	foreach ( $attrs as $key => $value ) {
		$attrs_string .= "$key='" . esc_attr( $value ) . "' ";
	}
	$attrs = ltrim( $attrs_string );
}

if ( $this->do_concat )
	$this->print_html .= "<script " . $attrs . " src='$src'></script>\n";
else
	echo "<script " . $attrs . "src='$src'></script>\n";

You can then use add_filter in your themes/plugins like so:

add_filter( 'script_loader_attrs', 'my_function' );

function my_function( $attrs ) {
   $attrs = array('async' => 'async', 'charset' => 'utf8') // whatever attributes you want

   // alternatively, eliminate type='text/javascript' by emptying $attrs:
   // $attrs = '';

   return $attrs;
}

The code above will always produce XHTML type attributes, e.g. async='async'. I'd need to add in a little code in the loop if the author wanted the more succinct async HTML5 version of the boolean.

For backward compatibility, the code will default back to including type='text/javascript' if no filter hooks in.

If people are interested in trying this solution, I could upload a diff file.

#6 @raphaelvalerio
3 years ago

  • Cc raphaelvalerio added

#7 @Volker_E.
3 years ago

  • Cc Volker_E. added

#8 @lkraav
3 years ago

  • Cc leho@… added

#9 @nacin
2 years ago

  • Component changed from General to Script Loader

#10 in reply to: ↑ description @jphase
2 years ago

Replying to ryanve:

I think it should be easier to customize the loading of scripts and styles (easier to customize the markup generated by the script/style system). Proposed solutions:

Solution 1: Allow wp_enqueue_script, wp_enqueue_style, wp_register_script, wp_register_style to accept an array of attributes as the $src parameter. For example:

wp_enqueue_script( 'my-plugin', array(
    'src' => 'http://example.com/js/app.js'
    'defer' => ''
    'data-my-plugin' => 'custom data attr value'
), array('jquery'), null, true );

I think Solution 1 makes complete sense from a developer's standpoint. This solution could keep any previous filters intact and provide a simple type check on the second param to change the way this is rendered. I'd be happy to help submit a diff for this or #23236 if any help is needed.

Solution 2: Add a filter before the markup is generated that allows devs to filter the attributes while they are in array format. For example:

add_filter('script_loader_attrs', function ($attrs, $handle) {
    unset ( $attrs['type'] );
    'my-plugin' === $handle and $attrs['data-my-plugin'] = 'plugin data';
    $attrs['src'] = remove_query_arg( $attrs['src'] );
    return $attrs;
}, 12, 2);

In class.wp-scripts.php it might look something like:

$attrs = (array) apply_filters('script_loader_attrs', $attrs, $handle);

and/or:

$attrs = (array) apply_filters("{$handle}_script_loader_attrs", $attrs );

I imagine that solution 2 would be easier to implement than 1, and 2 allows for themes/plugins to modify scripts/styles w/o re-registering resources.

The key feature of both solutions is the ability to modify the attrs while in array format. There are other ways that one could achieve the same results, but the array is by far the cleanest. Dirty alternatives include:

  • Use preg_replace() on the markup after it is generated (see #22245)
  • Use output buffers and PHP's DOMElement interface
  • Filter away the "print_scripts_array" and regenerate the markupmanually.)

@jtsternberg
2 years ago

New method, get_script_attributes, to concatenate an array of script attributes passed through a filter.

#11 @jtsternberg
2 years ago

  • Keywords has-patch reporter-feedback 2nd-opinion needs-testing added

With 22249.diff, you would be able to modify/remove the default type="text/javascript" as well as add your own attributes (like async="async").

#12 @danielbachhuber
20 months ago

I like the first option, although we could just as easily do both. The use case is something I run into regularly.

#13 follow-ups: @boonebgorges
20 months ago

22249.diff is a pretty cool idea. A couple thoughts I had while looking the patch over:

  • There was a suggestion earlier about making wp_register_script() accept an array for $src. 22249.diff doesn't include this, but hides everything in a filter. By not making this accessible as a function argument, it buries the functionality a bit. It also suggests that changes to these attributes are generally something you'd want to do on a specific site or with a specific theme - acting on *someone else's* scripts - rather than in a plugin that registers its own scripts. I can imagine situations where the plugin/theme registering the script would want to pass some custom attributes, and it seems pretty hackish to require them to add a filter for that purpose. So I'd suggest adding array support. (I'm not 100% sure that it makes sense to do this by allowing $src to be an array - in which case the variable name and docs would certainly have to be changed - or if another param should be added to the function for the attributes.)
  • Related: 'src' is not being passed to the new filter, but is hardcoded into the <script> tag. I can imagine some benefits of putting 'src' into the $default_attributes array and allowing it to be filtered as well. You'd probably need some sanity checks after the filter to make sure that 'src' hasn't been removed from the array.
  • Could stylesheets use the same treatment?

#14 in reply to: ↑ 13 @jeremyfelt
18 months ago

  • Milestone changed from Awaiting Review to Future Release

Big +1. All major browsers now support async, which would be very helpful. defer is likely less important because of in_footer, though still helpful. I'm sure there are other attributes I'm not aware of that could be included and result in more plugins using wp_enqueue_scripts() rather than something else.

I like the idea of $src being an array. It could also make sense to turn the $in_footer parameter into an array - maybe array( 'in_footer' => false, 'attributes' => array( 'type' => 'text/javascript' ) ). This may better imply the order of output. Including text/javascript as default may also be overkill - not sure.

Related #12009

@jtsternberg
18 months ago

Modifications based on feedback from boonebgorges and jeremyfelt

#15 in reply to: ↑ 13 @jtsternberg
18 months ago

[22249-2.diff](https://core.trac.wordpress.org/attachment/ticket/22249/22249-2.diff) Contains updates to:

  • Change $in_footer param for wp_register_script and wp_enqueue_script to an $args array where 'attributes' could be passed with an array of attribute key/value pairs. Also accepts 'in_footer' as a parameter and has a back-compat check for a passed non-array value.
  • Includes the src attribute/value, the type attribute/value, and any attributes passed in via the new $args param on wp_register_script and wp_enqueue_script in the array of attributes passed to the new script_loader_attributes filter.

If this passes muster, and all agree, I can apply the same treatment to the css side of things.

Last edited 18 months ago by jtsternberg (previous) (diff)

@camdensegal
18 months ago

Added enqueue/register styles attribute argument.

#16 @camdensegal
18 months ago

I've added the same functionality for wp_enqueue_style and wp_register_style. Though instead of having the attributes in a sub-array I have it as the main array because the argument being replaced ($media) is also an attribute.

@alex-ye
18 months ago

#17 @alex-ye
18 months ago

An update for @camdensegal patch..

  • Style media attribute is always a string.
  • Use the double-quotation strings only when necessary.

Still needs testing...

Last edited 18 months ago by alex-ye (previous) (diff)

#18 @Viper007Bond
17 months ago

[30403] allows you to do this but it'd still be nice to be able to control this more easily than via a filter.

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


11 months ago

#20 @bhubbard
9 months ago

Any update on this, I would really like to use this filter for Cloudflare:
https://support.cloudflare.com/hc/en-us/articles/200169436-How-can-I-have-Rocket-Loader-ignore-my-script-s-in-Automatic-Mode-

I also want to use it to add the integrity and crossorigin when I register Bootstrap scripts from BootstrapCDN:

<link href="https://maxcdn.bootstrapcdn.com/bootstrap/3.3.5/css/bootstrap.min.css" rel="stylesheet" integrity="sha256-MfvZlkHCEqatNoGiOXveE8FIwMzZg4W85qfrfIFBfYc= sha512-dTfge/zgoMYpP7QbHy4gWMEGsbsdZeCXz7irItjcC3sPUFtf0kuFbDz/ixG7ArTxmDjLXDmezHubeNikyKGVyQ==" crossorigin="anonymous">
Last edited 9 months ago by bhubbard (previous) (diff)

#21 @boonebgorges
9 months ago

  • Keywords needs-docs needs-unit-tests added; dev-feedback reporter-feedback 2nd-opinion needs-testing removed

Yeah, let's move forward on this.

Responding to alexye's most recent patch:

@ericlewis
5 months ago

#22 @ericlewis
5 months ago

  • Keywords needs-docs removed
  • Milestone changed from Future Release to 4.5

attachment:2249.2.diff

Still need unit tests here if anyone wants to give that a go.

@boonebgorges @jeremyfelt @danielbachhuber if we're going to modify the function signature, can we go all in?

<?php
wp_enqueue_script( array( 
  'handle' => 'd3',
  'src' => 'https://cdnjs.cloudflare.com/ajax/libs/d3/3.5.12/d3.js',
  'attributes' => array( 
    'async' => 'async'
  )
) );
Last edited 5 months ago by ericlewis (previous) (diff)

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


4 months ago

#24 @chriscct7
4 months ago

  • Owner set to chriscct7
  • Status changed from new to assigned

I'll try getting some unit tests done for this

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


3 months ago

@ericlewis
3 months ago

#26 @ericlewis
3 months ago

Had a good chat with @boonebgorges about the demerits of refactoring our script loader to use an array hash.

In attachment:22249.3.diff

  • Instead of changing $in_footer to an array hash, add a sixth argument for element attributes. Placing a script in the footer seems like a separate responsibility to think about when writing code.
  • Create separate methods on WP_Scripts for get_script_attributes() and get_concatenated_script_attributes().
  • Add tests.

Do we still want to offer a filter for script attributes now that its available on the register/enqueue functions?

#27 follow-up: @boonebgorges
3 months ago

Cool, this is shaping up nicely.

The method name get_concatenated_script_attributes() threw me for a loop, because 'concatenated' has a different and more frequently used meaning in the context of scripts/styles. I'd suggest something like get_script_attribute_html().

Did we actively decide not to give the same treatment to wp_enqueue_style()?

Why are we passing around $handle and $src in get_concatenated_script_attributes(), etc, but getting $args and $attributes from object properties?

Can we not do the weird ternary formatting in get_script_attributes()? If it's so confusing, just make it an if/else :)

Do we still want to offer a filter for script attributes now that its available on the register/enqueue functions?

You mean 'script_loader_attributes'? I guess I'd keep it - seems like there might be legit cases where programatically modification is appropriate - but I'd move it to get_script_attributes().

@ericlewis
3 months ago

#28 in reply to: ↑ 27 @ericlewis
3 months ago

Replying to boonebgorges:

The method name get_concatenated_script_attributes() threw me for a loop, because 'concatenated' has a different and more frequently used meaning in the context of scripts/styles. I'd suggest something like get_script_attribute_html().

Totally.

Did we actively decide not to give the same treatment to wp_enqueue_style()?

Let's land on an implementation for scripts first.

Why are we passing around $handle and $src in get_concatenated_script_attributes(), etc, but getting $args and $attributes from object properties?

The script object's property for $src is not an expanded URL (no base URL or version query string). That happens in WP_Scripts->do_item(). I've introduced WP_Scripts->get_script_src() in attachment:22249.4.diff which extracts this src expansion logic. Using this in get_script_attributes() removes the need to toss around $src. Also has tests.

Can we not do the weird ternary formatting in get_script_attributes()? If it's so confusing, just make it an if/else :)

100% yes.

Also in attachment:22249.4.diff, I've renamed the filter for a script's attributes from script_loader_attributes to just script_attributes, and removed the $src parameter. Handle should be useful enough.

#29 follow-up: @westonruter
3 months ago

@ericlewis can get_script_src() use esc_url_raw() instead of esc_url() when it returns? It would be more useful to add HTML entities via esc_url() at the point where it gets written into HTML, so the method can be used for other purposes (e.g. listing out scripts enqueued in the response for a given request to render in Selective Refresh partials).

@ericlewis
3 months ago

#30 in reply to: ↑ 29 @ericlewis
3 months ago

Replying to westonruter:

@ericlewis can get_script_src() use esc_url_raw() instead of esc_url() when it returns?

Sounds good. Added in attachment:22249.5.diff

#31 @ocean90
3 months ago

#24954 was marked as a duplicate.

#32 @ocean90
3 months ago

  • Keywords needs-refresh added

22249.5.diff:

  • Needs a refresh after recent changes
  • "The sixth argument was added. " should be "Introduced the $attributes parameter."
  • In get_script_src() spaces should be added when using variables as array keys, $this->registered[$handle]; => $this->registered[ $handle ];
  • IMO get_script_attributes() shouldn't return the $src value. I'd call it get_(script)_additonal_attributes(). That would solve all the special cases which makes the code a bit too complex right now.
  • This shouldn't go in without support for styles. Note that WP_Styles has already _css_href( $src, $ver, $handle ).

#33 @ericlewis
3 months ago

  • Owner chriscct7 deleted

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


3 months ago

#35 @jorbin
3 months ago

  • Milestone changed from 4.5 to Future Release

#36 @ericlewis
3 months ago

  • Keywords early needs-patch added; has-patch needs-unit-tests removed

@ocean90 said

IMO get_script_attributes() shouldn't return the $src value. I'd call it get_(script)_additonal_attributes().

that sounds good to me.

Let's revisit this in 4.6 early.

In the meantime, if anyone would like to write a new patch with @ocean90's considerations in mind, be my guest. Our implementation for scripts has come together, so the next patch can include an implementation for styles.

@ericlewis
5 weeks ago

#37 @ericlewis
5 weeks ago

I've refreshed the patch in attachment:22249.6.diff, comments inline.

@ocean90 said:

IMO get_script_attributes() shouldn't return the $src value. I'd call it get_(script)_additonal_attributes(). That would solve all the special cases which makes the code a bit too complex right now.

This implementation would make preserving the existing attribute order of type and then src cumbersome, so the current implementation seems like an okay amount of complex.

This shouldn't go in without support for styles. Note that WP_Styles has already _css_href( $src, $ver, $handle ).

attachment:22249.6.diff has the beginning of an implementation, but it's complicated.

We're planning on adding an $args parameter to wp_register_script(), which holds the extra attributes for the script. wp_enqueue_style() already accepts the $media value which it shoves in to the $args parameter in the underlying WP_Scripts->add() call.

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


4 weeks ago

Note: See TracTickets for help on using tickets.