#54529 closed defect (bug) (fixed)
Allow for `wp_register_script()` to be called after `wp_enqueue_script()`.
Reported by: | dd32 | Owned by: | audrasjb |
---|---|---|---|
Milestone: | 5.9 | Priority: | normal |
Severity: | normal | Version: | 5.9 |
Component: | Script Loader | Keywords: | has-patch has-unit-tests commit has-dev-note |
Focuses: | Cc: |
Description (last modified by )
This is a partially thought out PR, in response to fixing issues with a plugin and FSE.
When a plugin registers styles/scripts on wp_enqueue_scripts
(as we encourage plugins to do), and conditionally enqueues their script/style on the_content
filter, things "just work".
Unfortunately with FSE, things do not just work.
In FSE themes, the_content
is run prior to the header being processed, which results in the above scenario failing.
The attached PR makes a wp_enqueue_script( 'example' ); wp_register_script( 'example', 'https://.....' );
work, where as currently the enqueue silently fails (no doing it wrong, etc) and the following register has no impact.
Example plugin:
// Classic Theme: This will be called first // FSE: This will be called second. add_action( 'wp_enqueue_scripts', function() { wp_register_script( 'example', 'https://example.com/example.js' ); } ); // Classic Theme: This function will be called second, and enqueue the above script // FSE: This function will be called first, and fail to enqueue add_filter( 'the_content', function( $content ) { wp_enqueue_script( 'example' ); return $content; } );
(note: not tested, just written in trac)
I'm not convinced this is a perfect option to take, and FSE might be able to adjust how it's template loading is done, but it's done in the current way specifically so that blocks can enqueue scripts/styles into the header.
This also doesn't deal with certain functions, like wp_add_inline_style()
that attach data to an existing registered item, only dealing with the enqueue happening before a register (or 'add' in WP_Dependancy terms).
Change History (13)
This ticket was mentioned in PR #1968 on WordPress/wordpress-develop by dd32.
3 years ago
#1
- Keywords has-patch has-unit-tests added
#3
@
3 years ago
- Milestone changed from Awaiting Review to 5.9
- Version set to trunk
I'm adding this to the 5.9 milestone so it can be considered before FSE becomes stable. It seems like a good change to me and the ripe time to do it is before requiring plugin authors to update their code.
This ticket was mentioned in Slack in #core by audrasjb. View the logs.
3 years ago
#6
follow-up:
↓ 7
@
3 years ago
- Keywords commit added
The patch/PR looks good here, scripts can be enqueued and dequeued (by "handle") before they are registered.
This also doesn't deal with certain functions, like wp_add_inline_style() that attach data to an existing registered item.
Thinking that's okay. Enqueueing is often used as one-step registering + enqueueing. Then adding before/after should work. When doing two steps: registering and then enqueueing the before/after should always be addedafter the registering and seems it's going to work with this patch.
#7
in reply to:
↑ 6
;
follow-up:
↓ 8
@
3 years ago
Replying to azaozz:
This also doesn't deal with certain functions, like wp_add_inline_style() that attach data to an existing registered item.
Thinking that's okay.
As do I, it's not perfect, but it's not horrible.
This is an example of what wouldn't work though:
add_action( 'wp_enqueue_scripts', function() { wp_register_script( 'example', 'https://example.com/example.js' ); } ); add_filter( 'the_content', function( $content, $post ) { wp_enqueue_script( 'example' ); wp_localize_script( 'example', 'exampleData', [ 'postID' => $post->ID ] ); return $content; }, 10, 2 );
but like @azaozz said, that doesn't seem like a real-world example, usually you'd have something more akin to this, which would still work, or you'd not use register
and just a straight enqueue + url + localise
at the same time.
add_action( 'wp_enqueue_scripts', function() { wp_register_script( 'example', 'https://example.com/example.js' ); wp_localize_script( 'example', 'exampleData', [ 'url' => 'https://example.com/' ] ); } );
One thing I'm unsure of - The PR causes the enqueue to happen before the script register "in footer" call is made, but I'm not sure if that's actually a problem, as I don't know how the 'group' part is handled.. I think it's okay.
$registered = $wp_scripts->add( $handle, $src, $deps, $ver ); // it'll be enqueued at this point if ( $in_footer ) { $wp_scripts->add_data( $handle, 'group', 1 ); // This might run after the enqueue. }
(note: Can someone else who is more familiar with scripts/styles please test and commit if you like it, I'm way too rusty on these things)
#8
in reply to:
↑ 7
@
3 years ago
Replying to dd32:
When a plugin registers styles/scripts on wp_enqueue_scripts (as we encourage plugins to do),
This is actually wrong. Scripts should be registered on init
, and enqueued on wp_enqueue_scripts
.
As do I, it's not perfect, but it's not horrible.
Right. This is a "double" edge case :) When doing script registering and enqueueing separately scripts should be registered on init
, and enqueued on wp_enqueue_scripts
.
The case we're trying to accommodate is registering on wp_enqueue_scripts
(that's late, this action runs after init
), and then enqueueing on the wp_content
filter by using it as an action (that can be late too, this filter usually runs in the middle of the HTML <body>
but may run much earlier).
So generally there are two "doing it wrong" here, the first worse than the second. Wondering if there should be notices for them.
Also note that scripts can be registered and enqueued at the same time by using the wp_enqueue_script()
function, see https://developer.wordpress.org/reference/functions/wp_enqueue_script/. Then it is the proper/expected way to use the wp_enqueue_scripts
action. This will also help to mitigate the problem we're trying to solve here.
This is an example of what wouldn't work though:
Yeah, probably better to add that doing-it-wrong when registering a script from the wp_enqueue_scripts
action.
One thing I'm unsure of - The PR causes the enqueue to happen before the script register "in footer" call is made, but I'm not sure if that's actually a problem
Seems this should work fine. This patch "holds" the actual enqueueing until the script is registered. Then the registering and enqueueing happens in the proper order.
The possible problems (as mentioned above) are from the fact that wp_enqueue_script()
was called with a script handle only but the script is not yet registered until the registering happens. That means there cannot be any extras like localization, before/after, etc. However these extras are usually added after registering a script, not after enqueueing it. Frankly I'm not sure this rare edge case can be fixed easily.
#9
@
3 years ago
- Owner set to audrasjb
- Status changed from new to assigned
Self assigning for test then commit as it looks like it's ready to go before Beta 2 today.
#10
@
3 years ago
- Keywords needs-dev-note added
Tested using the above example:
// Classic Theme: This will be called first // FSE: This will be called second. add_action( 'wp_enqueue_scripts', function() { wp_register_script( 'example', 'https://example.com/example.js' ); } ); // Classic Theme: This function will be called second, and enqueue the above script // FSE: This function will be called first, and fail to enqueue add_filter( 'the_content', function( $content ) { wp_enqueue_script( 'example' ); return $content; } );
## Test results
Before patch:
- On a block theme, the
example
script is not printed on front-end. - On a Classic Theme, the
example
is printed on front-end.
After patch:
- On a block theme, the
example
script is printed on front-end. - On a Classic Theme, the
example
is still printed on front-end.
Tests look good.
3 years ago
#12
Committed in https://core.trac.wordpress.org/changeset/52338
Trac ticket: https://core.trac.wordpress.org/ticket/54529