Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add / 48 Abort Script Strategy Logic When Concat Scripts Present #51

Merged
merged 18 commits into from
Apr 3, 2023
Merged

Add / 48 Abort Script Strategy Logic When Concat Scripts Present #51

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
24452a6
Abort/reset script concatenation if the handle in question is of a de…
10upsimon Mar 27, 2023
08f200f
Fixed comment typo
10upsimon Mar 27, 2023
f9355cd
Merge branch 'enhancement/implement_load_handlers' into add/issue-48-…
kt-12 Mar 28, 2023
ae38432
strategy already available
kt-12 Mar 28, 2023
54c74be
Add more specific defer and async strategy checks for concatenation e…
10upsimon Mar 28, 2023
31e4507
test defer script with concat
kt-12 Mar 29, 2023
2031aff
concat with async
kt-12 Mar 29, 2023
6beabce
blocking before and after defer
kt-12 Mar 29, 2023
a8dba9f
Merge origin into local
10upsimon Mar 29, 2023
6d75add
Merge branch 'add/issue-48-abort-script-strategy-logic-when-concat-sc…
10upsimon Mar 29, 2023
ffe725c
Remove two trailing spaces in printed output that were failing test i…
10upsimon Mar 30, 2023
c656495
Replaced hard coded references to default script directory string to …
10upsimon Mar 30, 2023
23630b9
Fix spacing for PHPCS
joemcgill Mar 30, 2023
2087305
Update script handle name to match async strategy in tests
joemcgill Mar 30, 2023
fdf0bd3
Merge branch 'enhancement/implement_load_handlers' into add/issue-48-…
kt-12 Mar 31, 2023
4b16162
remove extra variable
kt-12 Mar 31, 2023
0c7d18e
remove hardcoded type
kt-12 Mar 31, 2023
f61d867
Merge branch 'enhancement/implement_load_handlers' into add/issue-48-…
kt-12 Apr 3, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 5 additions & 4 deletions src/wp-includes/class-wp-scripts.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -329,8 +329,6 @@ public function do_item( $handle, $group = false ) {
PHP_EOL
);
$this->type_attr = $initial_type_attr;

$this->has_load_later_inline = true;
}
}

Expand Down Expand Up @@ -362,7 +360,10 @@ public function do_item( $handle, $group = false ) {
*/
$srce = apply_filters( 'script_loader_src', $src, $handle );

if ( $this->in_default_dir( $srce ) && ( $before_handle || $after_handle || $translations_stop_concat ) ) {
// Used as a conditional to prevent script concatenation.
$is_deferred_or_async_handle = in_array( $strategy, array( 'defer', 'async' ), true );

if ( $this->in_default_dir( $srce ) && ( $before_handle || $after_handle || $translations_stop_concat || $is_deferred_or_async_handle ) ) {
$this->do_concat = false;

// Have to print the so-far concatenated scripts right away to maintain the right order.
Expand Down Expand Up @@ -516,7 +517,7 @@ public function print_inline_script( $handle, $position = 'after', $display = tr
$initial_type_attr = $this->type_attr;
$this->type_attr = " type='text/template'";
printf(
'<script%1$s id=\'%2$s-js-after\' type=\'text/template\' data-wp-executes-after=\'%2$s\'>%5$s%4$s%5$s</script>%5$s',
'<script%1$s id=\'%2$s-js-after\' data-wp-executes-after=\'%2$s\'>%5$s%4$s%5$s</script>%5$s',
$this->type_attr,
esc_attr( $handle ),
esc_attr( $position ),
Expand Down
4 changes: 2 additions & 2 deletions src/wp-includes/script-loader.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1856,8 +1856,8 @@ function wp_print_template_loader_script() {
$output = <<<JS
function wpLoadAfterScripts( handle ) {
let scripts = document.querySelectorAll(`[type="text/template"][data-wp-executes-after="\${handle}"]`);
scripts.forEach( (script) => {
script.setAttribute("type","text/javascript");
scripts.forEach( (script) => {
script.setAttribute("type","text/javascript");
eval(script.innerHTML);
})
}
Expand Down
166 changes: 134 additions & 32 deletions tests/phpunit/tests/dependencies/scripts.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,9 @@ class Tests_Dependencies_Scripts extends WP_UnitTestCase {

protected $wp_scripts_print_translations_output;

// Stores a string reference to a default scripts directory name, utilised by certain tests.
protected $default_scripts_dir = '/directory/';

public function set_up() {
parent::set_up();
$this->old_wp_scripts = isset( $GLOBALS['wp_scripts'] ) ? $GLOBALS['wp_scripts'] : null;
Expand Down Expand Up @@ -79,8 +82,8 @@ public function test_non_standalone_and_standalone_after_script_combined() {
<script type='text/javascript' id='wp-executes-after-js'>
function wpLoadAfterScripts( handle ) {
let scripts = document.querySelectorAll(`[type="text/template"][data-wp-executes-after="\${handle}"]`);
scripts.forEach( (script) => {
script.setAttribute("type","text/javascript");
scripts.forEach( (script) => {
script.setAttribute("type","text/javascript");
eval(script.innerHTML);
})
}
Expand Down Expand Up @@ -155,8 +158,8 @@ public function test_non_standalone_after_inline_script_with_defer_main_script()
<script type='text/javascript' id='wp-executes-after-js'>
function wpLoadAfterScripts( handle ) {
let scripts = document.querySelectorAll(`[type="text/template"][data-wp-executes-after="\${handle}"]`);
scripts.forEach( (script) => {
script.setAttribute("type","text/javascript");
scripts.forEach( (script) => {
script.setAttribute("type","text/javascript");
eval(script.innerHTML);
})
}
Expand Down Expand Up @@ -188,8 +191,8 @@ public function test_non_standalone_after_inline_script_with_async_main_script()
<script type='text/javascript' id='wp-executes-after-js'>
function wpLoadAfterScripts( handle ) {
let scripts = document.querySelectorAll(`[type="text/template"][data-wp-executes-after="\${handle}"]`);
scripts.forEach( (script) => {
script.setAttribute("type","text/javascript");
scripts.forEach( (script) => {
script.setAttribute("type","text/javascript");
eval(script.innerHTML);
})
}
Expand Down Expand Up @@ -625,6 +628,105 @@ public function test_get_normalized_script_args() {
$this->assertSame( $expected_args, $wp_scripts->get_data( 'footer-old', 'script_args' ) );
}

/**
* Test script concatenation with deferred main script.
*
* @ticket 12009
*/
public function test_concatenate_with_defer_strategy() {
global $wp_scripts, $concatenate_scripts;

$old_value = $concatenate_scripts;
$concatenate_scripts = true;

$wp_scripts->do_concat = true;
$wp_scripts->default_dirs = array( $this->default_scripts_dir );

wp_register_script( 'one-concat-dep', $this->default_scripts_dir . 'script.js' );
wp_register_script( 'two-concat-dep', $this->default_scripts_dir . 'script.js' );
wp_register_script( 'three-concat-dep', $this->default_scripts_dir . 'script.js' );
wp_enqueue_script( 'main-defer-script', '/main-script.js', array( 'one-concat-dep', 'two-concat-dep', 'three-concat-dep' ), null, array( 'strategy' => 'defer' ) );

wp_print_scripts();
$print_scripts = get_echo( '_print_scripts' );

// reset global before asserting.
$concatenate_scripts = $old_value;

$ver = get_bloginfo( 'version' );
$expected = "<script type='text/javascript' src='/wp-admin/load-scripts.php?c=0&amp;load%5Bchunk_0%5D=one-concat-dep,two-concat-dep,three-concat-dep&amp;ver={$ver}'></script>\n";
$expected .= "<script type='text/javascript' src='/main-script.js' id='main-defer-script-js' defer></script>\n";

$this->assertSame( $expected, $print_scripts );
}

/**
* Test script concatenation with `async` main script.
*
* @ticket 12009
*/
public function test_concatenate_with_async_strategy() {
global $wp_scripts, $concatenate_scripts;

$old_value = $concatenate_scripts;
$concatenate_scripts = true;

$wp_scripts->do_concat = true;
$wp_scripts->default_dirs = array( $this->default_scripts_dir );

wp_enqueue_script( 'one-concat-dep-1', $this->default_scripts_dir . 'script.js' );
wp_enqueue_script( 'two-concat-dep-1', $this->default_scripts_dir . 'script.js' );
wp_enqueue_script( 'three-concat-dep-1', $this->default_scripts_dir . 'script.js' );
wp_enqueue_script( 'main-async-script-1', '/main-script.js', array(), null, array( 'strategy' => 'async' ) );

wp_print_scripts();
$print_scripts = get_echo( '_print_scripts' );

// reset global before asserting.
$concatenate_scripts = $old_value;

$ver = get_bloginfo( 'version' );
$expected = "<script type='text/javascript' src='/wp-admin/load-scripts.php?c=0&amp;load%5Bchunk_0%5D=one-concat-dep-1,two-concat-dep-1,three-concat-dep-1&amp;ver={$ver}'></script>\n";
$expected .= "<script type='text/javascript' src='/main-script.js' id='main-async-script-1-js' async></script>\n";

$this->assertSame( $expected, $print_scripts );
}

/**
* Test script concatenation with blocking scripts before and after a `defer` script.
*
* @ticket 12009
*/
public function test_concatenate_with_blocking_script_before_and_after_script_with_defer_strategy() {
global $wp_scripts, $concatenate_scripts;

$old_value = $concatenate_scripts;
$concatenate_scripts = true;

$wp_scripts->do_concat = true;
$wp_scripts->default_dirs = array( $this->default_scripts_dir );

wp_enqueue_script( 'one-concat-dep-2', $this->default_scripts_dir . 'script.js' );
wp_enqueue_script( 'two-concat-dep-2', $this->default_scripts_dir . 'script.js' );
wp_enqueue_script( 'three-concat-dep-2', $this->default_scripts_dir . 'script.js' );
wp_enqueue_script( 'deferred-script-2', '/main-script.js', array(), null, array( 'strategy' => 'defer' ) );
wp_enqueue_script( 'four-concat-dep-2', $this->default_scripts_dir . 'script.js' );
wp_enqueue_script( 'five-concat-dep-2', $this->default_scripts_dir . 'script.js' );
wp_enqueue_script( 'six-concat-dep-2', $this->default_scripts_dir . 'script.js' );

wp_print_scripts();
$print_scripts = get_echo( '_print_scripts' );

// reset global before asserting.
$concatenate_scripts = $old_value;

$ver = get_bloginfo( 'version' );
$expected = "<script type='text/javascript' src='/wp-admin/load-scripts.php?c=0&amp;load%5Bchunk_0%5D=one-concat-dep-2,two-concat-dep-2,three-concat-dep-2,four-concat-dep-2,five-concat-dep-2,six-concat-dep-2&amp;ver={$ver}'></script>\n";
$expected .= "<script type='text/javascript' src='/main-script.js' id='deferred-script-2-js' defer></script>\n";

$this->assertSame( $expected, $print_scripts );
}

/**
* @ticket 42804
*/
Expand Down Expand Up @@ -694,11 +796,11 @@ public function test_script_concatenation() {
global $wp_scripts;

$wp_scripts->do_concat = true;
$wp_scripts->default_dirs = array( '/directory/' );
$wp_scripts->default_dirs = array( $this->default_scripts_dir );

wp_enqueue_script( 'one', '/directory/script.js' );
wp_enqueue_script( 'two', '/directory/script.js' );
wp_enqueue_script( 'three', '/directory/script.js' );
wp_enqueue_script( 'one', $this->default_scripts_dir . 'script.js' );
wp_enqueue_script( 'two', $this->default_scripts_dir . 'script.js' );
wp_enqueue_script( 'three', $this->default_scripts_dir . 'script.js' );

wp_print_scripts();
$print_scripts = get_echo( '_print_scripts' );
Expand Down Expand Up @@ -1091,21 +1193,21 @@ public function test_wp_add_inline_script_before_with_concat() {
global $wp_scripts;

$wp_scripts->do_concat = true;
$wp_scripts->default_dirs = array( '/directory/' );
$wp_scripts->default_dirs = array( $this->default_scripts_dir );

wp_enqueue_script( 'one', '/directory/one.js' );
wp_enqueue_script( 'two', '/directory/two.js' );
wp_enqueue_script( 'three', '/directory/three.js' );
wp_enqueue_script( 'one', $this->default_scripts_dir . 'one.js' );
wp_enqueue_script( 'two', $this->default_scripts_dir . 'two.js' );
wp_enqueue_script( 'three', $this->default_scripts_dir . 'three.js' );

wp_add_inline_script( 'one', 'console.log("before one");', 'before' );
wp_add_inline_script( 'two', 'console.log("before two");', 'before' );

$ver = get_bloginfo( 'version' );
$expected = "<script type='text/javascript' id='one-js-before'>\nconsole.log(\"before one\");\n</script>\n";
$expected .= "<script type='text/javascript' src='/directory/one.js?ver={$ver}' id='one-js'></script>\n";
$expected .= "<script type='text/javascript' src='{$this->default_scripts_dir}one.js?ver={$ver}' id='one-js'></script>\n";
$expected .= "<script type='text/javascript' id='two-js-before'>\nconsole.log(\"before two\");\n</script>\n";
$expected .= "<script type='text/javascript' src='/directory/two.js?ver={$ver}' id='two-js'></script>\n";
$expected .= "<script type='text/javascript' src='/directory/three.js?ver={$ver}' id='three-js'></script>\n";
$expected .= "<script type='text/javascript' src='{$this->default_scripts_dir}two.js?ver={$ver}' id='two-js'></script>\n";
$expected .= "<script type='text/javascript' src='{$this->default_scripts_dir}three.js?ver={$ver}' id='three-js'></script>\n";

$this->assertSame( $expected, get_echo( 'wp_print_scripts' ) );
}
Expand All @@ -1117,19 +1219,19 @@ public function test_wp_add_inline_script_before_with_concat2() {
global $wp_scripts;

$wp_scripts->do_concat = true;
$wp_scripts->default_dirs = array( '/directory/' );
$wp_scripts->default_dirs = array( $this->default_scripts_dir );

wp_enqueue_script( 'one', '/directory/one.js' );
wp_enqueue_script( 'two', '/directory/two.js' );
wp_enqueue_script( 'three', '/directory/three.js' );
wp_enqueue_script( 'one', $this->default_scripts_dir . 'one.js' );
wp_enqueue_script( 'two', $this->default_scripts_dir . 'two.js' );
wp_enqueue_script( 'three', $this->default_scripts_dir . 'three.js' );

wp_add_inline_script( 'one', 'console.log("before one");', 'before' );

$ver = get_bloginfo( 'version' );
$expected = "<script type='text/javascript' id='one-js-before'>\nconsole.log(\"before one\");\n</script>\n";
$expected .= "<script type='text/javascript' src='/directory/one.js?ver={$ver}' id='one-js'></script>\n";
$expected .= "<script type='text/javascript' src='/directory/two.js?ver={$ver}' id='two-js'></script>\n";
$expected .= "<script type='text/javascript' src='/directory/three.js?ver={$ver}' id='three-js'></script>\n";
$expected .= "<script type='text/javascript' src='{$this->default_scripts_dir}one.js?ver={$ver}' id='one-js'></script>\n";
$expected .= "<script type='text/javascript' src='{$this->default_scripts_dir}two.js?ver={$ver}' id='two-js'></script>\n";
$expected .= "<script type='text/javascript' src='{$this->default_scripts_dir}three.js?ver={$ver}' id='three-js'></script>\n";

$this->assertSame( $expected, get_echo( 'wp_print_scripts' ) );
}
Expand All @@ -1141,23 +1243,23 @@ public function test_wp_add_inline_script_after_with_concat() {
global $wp_scripts;

$wp_scripts->do_concat = true;
$wp_scripts->default_dirs = array( '/directory/' );
$wp_scripts->default_dirs = array( $this->default_scripts_dir );

wp_enqueue_script( 'one', '/directory/one.js' );
wp_enqueue_script( 'two', '/directory/two.js' );
wp_enqueue_script( 'three', '/directory/three.js' );
wp_enqueue_script( 'four', '/directory/four.js' );
wp_enqueue_script( 'one', $this->default_scripts_dir . 'one.js' );
wp_enqueue_script( 'two', $this->default_scripts_dir . 'two.js' );
wp_enqueue_script( 'three', $this->default_scripts_dir . 'three.js' );
wp_enqueue_script( 'four', $this->default_scripts_dir . 'four.js' );

wp_add_inline_script( 'two', 'console.log("after two");' );
wp_add_inline_script( 'three', 'console.log("after three");' );

$ver = get_bloginfo( 'version' );
$expected = "<script type='text/javascript' src='/wp-admin/load-scripts.php?c=0&amp;load%5Bchunk_0%5D=one&amp;ver={$ver}'></script>\n";
$expected .= "<script type='text/javascript' src='/directory/two.js?ver={$ver}' id='two-js'></script>\n";
$expected .= "<script type='text/javascript' src='{$this->default_scripts_dir}two.js?ver={$ver}' id='two-js'></script>\n";
$expected .= "<script type='text/javascript' id='two-js-after'>\nconsole.log(\"after two\");\n</script>\n";
$expected .= "<script type='text/javascript' src='/directory/three.js?ver={$ver}' id='three-js'></script>\n";
$expected .= "<script type='text/javascript' src='{$this->default_scripts_dir}three.js?ver={$ver}' id='three-js'></script>\n";
$expected .= "<script type='text/javascript' id='three-js-after'>\nconsole.log(\"after three\");\n</script>\n";
$expected .= "<script type='text/javascript' src='/directory/four.js?ver={$ver}' id='four-js'></script>\n";
$expected .= "<script type='text/javascript' src='{$this->default_scripts_dir}four.js?ver={$ver}' id='four-js'></script>\n";

$this->assertSame( $expected, get_echo( 'wp_print_scripts' ) );
}
Expand Down