-
Notifications
You must be signed in to change notification settings - Fork 0
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.
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
DRAFT: PR for enhancing WP_Scripts with a loading strategy #54
Changes from 245 commits
5795b4c
0ebfaa5
f6b0483
7b36d21
f71319c
624e797
b7334b7
2cb01be
5305f1c
073684a
5e343b8
b56c0e1
9caf8bb
b70ee6c
dbf6da9
2deadcf
fcaa62e
150ba8b
d578e50
5262bf7
2970f1a
28af60d
f746dfd
00a50d1
d4bbaff
01e1d4f
a367951
51e3e41
62cbf60
0f80794
f20cfea
e406a13
b961108
70fb6c6
2b3d11c
16c4634
a10cb5c
dfbecd8
4fa3d7b
043c244
53a54e6
d90ed89
514876f
2c31104
4e1aaaf
df906be
6be6f16
7564a68
484789f
3ad953f
dcd2a32
8d33fac
e4b461d
8421a1c
371e42f
c517bc1
04aabf5
e366d5d
0fafd88
386022e
fe7acc0
394405f
cc5dace
b558f34
f04a246
b5f4df4
2eb8aed
e0e560e
c856372
ff25567
b2d6ec1
d9a0f39
d110556
b6b7022
9e8c378
ffc40c1
8af918c
d952da5
b71b557
479d656
968d5b7
ad9d9c3
d522c46
ce5fc87
0a0b253
a24665a
f29c144
0988655
d66907e
b7bff57
013cb1e
db02925
42bc576
07179c8
169fc28
ad9cafb
5cda63d
a587e5e
b861844
70122ff
c81fdf2
2dce5f4
2571691
b7ac63f
326555d
8d5729c
6187dd1
1321d6f
66ab60c
bbdad37
24452a6
08f200f
966be0c
f9355cd
ae38432
54c74be
31e4507
2031aff
6beabce
a8dba9f
6d75add
ffe725c
c656495
23630b9
2087305
dcbf0af
01c890e
9aaca52
8e08454
b3f7225
4df5288
264853c
556ae91
d4be88d
180a6ae
fdf0bd3
4b16162
0c7d18e
4d0140d
a5b83a4
0269128
db29ae2
28ba27c
4cc2502
0007785
59ee419
6125283
71811ab
f61d867
90e6105
88fe91d
82d5ea8
569fd3e
9eb6336
76b3705
1b7ac23
5114138
204ff24
2e8cbfa
70aed2d
146499d
0be95f2
24003f0
a3baa8d
4945a14
f062f0b
239ee0b
626e2bf
c3f5898
fefe6ee
1052b59
1d7b9bd
b10982d
30747d1
0b393b7
4df050c
e4c5128
ae5797b
90764c8
08ed2d3
97846ba
03cb904
a569440
5be4ecf
5bc5864
37eeff9
9f91ccb
09c4abb
4e59471
a75c67d
bfc27f9
06514c4
59d2503
fb93f94
9804459
83b2b27
286d589
15dfe0f
909714e
4534440
f5d4b55
4be6d42
3385b26
3b46fe9
b1d57ea
5508cf0
996e018
654d1e7
a80156f
b92f5be
8549c52
281aadc
69f20dd
bd4c18c
156555d
e0c60d4
07ae509
35bc9c7
2b6155a
3ad9b74
8116c22
6bccf2e
213d83d
dc9f902
fd16848
1a77dd4
d0e740e
6801291
dbca402
17516c7
9813ba8
0c90511
25db6e8
b61fe3d
343e5ed
79ef56d
f75a8d2
2770ba9
9d0e68c
3044aa3
3ca847e
edd3d52
59324a8
6ab9511
5a94bfa
0e120b7
da5668f
9c3ded2
3d9d0d8
430e5b2
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -295,14 +295,43 @@ public function do_item( $handle, $group = false ) { | |||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
$before_handle = $this->print_inline_script( $handle, 'before', false ); | ||||||||||||||||||
$after_handle = $this->print_inline_script( $handle, 'after', false ); | ||||||||||||||||||
|
||||||||||||||||||
if ( $before_handle ) { | ||||||||||||||||||
$before_handle = sprintf( "<script%s id='%s-js-before'>\n%s\n</script>\n", $this->type_attr, esc_attr( $handle ), $before_handle ); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
if ( $after_handle ) { | ||||||||||||||||||
$after_handle = sprintf( "<script%s id='%s-js-after'>\n%s\n</script>\n", $this->type_attr, esc_attr( $handle ), $after_handle ); | ||||||||||||||||||
$strategy = $this->get_eligible_loading_strategy( $handle ); | ||||||||||||||||||
|
||||||||||||||||||
$after_handle = ''; | ||||||||||||||||||
|
||||||||||||||||||
// Eligible loading strategies will only be 'async', 'defer', or ''. | ||||||||||||||||||
if ( '' === $strategy ) { | ||||||||||||||||||
kt-12 marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||
$after_handle = $this->print_inline_script( $handle, 'after', false ); | ||||||||||||||||||
|
||||||||||||||||||
if ( $after_handle ) { | ||||||||||||||||||
$after_handle = sprintf( "<script%s id='%s-js-after'>\n%s\n</script>\n", $this->type_attr, esc_attr( $handle ), $after_handle ); | ||||||||||||||||||
joemcgill marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||
} | ||||||||||||||||||
} else { | ||||||||||||||||||
$after_standalone_handle = $this->print_inline_script( $handle, 'after-standalone', false ); | ||||||||||||||||||
|
||||||||||||||||||
if ( $after_standalone_handle ) { | ||||||||||||||||||
$after_handle .= sprintf( "<script%s id='%s-js-after'>\n%s\n</script>\n", $this->type_attr, esc_attr( $handle ), $after_standalone_handle ); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
$after_non_standalone_handle = $this->print_inline_script( $handle, 'after-non-standalone', false ); | ||||||||||||||||||
|
||||||||||||||||||
if ( $after_non_standalone_handle ) { | ||||||||||||||||||
$initial_type_attr = $this->type_attr; | ||||||||||||||||||
$this->type_attr = " type='text/template'"; | ||||||||||||||||||
$after_handle .= sprintf( | ||||||||||||||||||
'<script%1$s id=\'%2$s-js-after\' data-wp-executes-after=\'%2$s\'>%4$s%3$s%4$s</script>%4$s', | ||||||||||||||||||
$this->type_attr, | ||||||||||||||||||
esc_attr( $handle ), | ||||||||||||||||||
$after_non_standalone_handle, | ||||||||||||||||||
PHP_EOL | ||||||||||||||||||
); | ||||||||||||||||||
$this->type_attr = $initial_type_attr; | ||||||||||||||||||
joemcgill marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
if ( $before_handle || $after_handle ) { | ||||||||||||||||||
|
@@ -333,7 +362,13 @@ 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. | ||||||||||||||||||
|
@@ -390,8 +425,20 @@ public function do_item( $handle, $group = false ) { | |||||||||||||||||
return true; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
if ( '' !== $strategy ) { | ||||||||||||||||||
$strategy = ' ' . $strategy; | ||||||||||||||||||
if ( ! empty( $after_non_standalone_handle ) ) { | ||||||||||||||||||
$strategy .= sprintf( " onload='wpLoadAfterScripts(\"%s\")'", esc_attr( $handle ) ); | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This should be more robust to guard against malicious script handles:
Suggested change
|
||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
$tag = $translations . $cond_before . $before_handle; | ||||||||||||||||||
$tag .= sprintf( "<script%s src='%s' id='%s-js'></script>\n", $this->type_attr, $src, esc_attr( $handle ) ); | ||||||||||||||||||
$tag .= sprintf( | ||||||||||||||||||
"<script%s src='%s' id='%s-js'%s></script>\n", | ||||||||||||||||||
$this->type_attr, | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. ditto, can we really trust There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @adamsilverstein yes, this is not an addition by us, this is a private var in the Please see here. |
||||||||||||||||||
esc_url( $src ), | ||||||||||||||||||
esc_attr( $handle ), | ||||||||||||||||||
$strategy | ||||||||||||||||||
); | ||||||||||||||||||
$tag .= $after_handle . $cond_after; | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
|
@@ -419,14 +466,15 @@ public function do_item( $handle, $group = false ) { | |||||||||||||||||
* | ||||||||||||||||||
* @since 4.5.0 | ||||||||||||||||||
* | ||||||||||||||||||
* @param string $handle Name of the script to add the inline script to. | ||||||||||||||||||
* Must be lowercase. | ||||||||||||||||||
* @param string $data String containing the JavaScript to be added. | ||||||||||||||||||
* @param string $position Optional. Whether to add the inline script | ||||||||||||||||||
* before the handle or after. Default 'after'. | ||||||||||||||||||
* @param string $handle Name of the script to add the inline script to. | ||||||||||||||||||
* Must be lowercase. | ||||||||||||||||||
* @param string $data String containing the JavaScript to be added. | ||||||||||||||||||
* @param string $position Optional. Whether to add the inline script | ||||||||||||||||||
* before the handle or after. Default 'after'. | ||||||||||||||||||
* @param bool $standalone Optional. Inline script opted to be standalone or not. Default false. | ||||||||||||||||||
* @return bool True on success, false on failure. | ||||||||||||||||||
*/ | ||||||||||||||||||
public function add_inline_script( $handle, $data, $position = 'after' ) { | ||||||||||||||||||
public function add_inline_script( $handle, $data, $position = 'after', $standalone = false ) { | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. same question here - should we consider updating all usages in core? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @adamsilverstein I think this would be okay to do, my only comment would be that should any of the main scripts that the inline scripts attached to ever be updated to have a deferred loading strategy (even by association with another script), we'd need to be very certain that these |
||||||||||||||||||
if ( ! $data ) { | ||||||||||||||||||
return false; | ||||||||||||||||||
} | ||||||||||||||||||
|
@@ -438,6 +486,12 @@ public function add_inline_script( $handle, $data, $position = 'after' ) { | |||||||||||||||||
$script = (array) $this->get_data( $handle, $position ); | ||||||||||||||||||
$script[] = $data; | ||||||||||||||||||
|
||||||||||||||||||
// Maintain a list of standalone and non-standalone before/after scripts. | ||||||||||||||||||
$standalone_key = $standalone ? $position . '-standalone' : $position . '-non-standalone'; | ||||||||||||||||||
$standalone_script = (array) $this->get_data( $handle, $standalone_key ); | ||||||||||||||||||
$standalone_script[] = $data; | ||||||||||||||||||
$this->add_data( $handle, $standalone_key, $standalone_script ); | ||||||||||||||||||
|
||||||||||||||||||
return $this->add_data( $handle, $position, $script ); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
|
@@ -464,7 +518,21 @@ public function print_inline_script( $handle, $position = 'after', $display = tr | |||||||||||||||||
$output = trim( implode( "\n", $output ), "\n" ); | ||||||||||||||||||
|
||||||||||||||||||
if ( $display ) { | ||||||||||||||||||
printf( "<script%s id='%s-js-%s'>\n%s\n</script>\n", $this->type_attr, esc_attr( $handle ), esc_attr( $position ), $output ); | ||||||||||||||||||
if ( 'after-non-standalone' === $position ) { | ||||||||||||||||||
10upsimon marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||
$initial_type_attr = $this->type_attr; | ||||||||||||||||||
$this->type_attr = " type='text/template'"; | ||||||||||||||||||
printf( | ||||||||||||||||||
'<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 ), | ||||||||||||||||||
$output, | ||||||||||||||||||
PHP_EOL | ||||||||||||||||||
); | ||||||||||||||||||
$this->type_attr = $initial_type_attr; | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. See previous comment about swapping There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. |
||||||||||||||||||
} else { | ||||||||||||||||||
printf( "<script%s id='%s-js-%s'>\n%s\n</script>\n", $this->type_attr, esc_attr( $handle ), esc_attr( $position ), $output ); | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Numbered placeholders and readability:
Suggested change
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Reply same as above (first comment) |
||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
return $output; | ||||||||||||||||||
|
@@ -714,6 +782,192 @@ public function in_default_dir( $src ) { | |||||||||||||||||
return false; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* This overrides the add_data method from WP_Dependencies, to support normalizing of $args. | ||||||||||||||||||
* | ||||||||||||||||||
* @param string $handle Name of the item. Should be unique. | ||||||||||||||||||
* @param string $key The data key. | ||||||||||||||||||
* @param mixed $value The data value. | ||||||||||||||||||
* @return bool True on success, false on failure. | ||||||||||||||||||
*/ | ||||||||||||||||||
public function add_data( $handle, $key, $value ) { | ||||||||||||||||||
if ( 'script_args' === $key ) { | ||||||||||||||||||
$args = $this->get_normalized_script_args( $handle, $value ); | ||||||||||||||||||
if ( $args['in_footer'] ) { | ||||||||||||||||||
parent::add_data( $handle, 'group', 1 ); | ||||||||||||||||||
} | ||||||||||||||||||
return parent::add_data( $handle, $key, $args ); | ||||||||||||||||||
} | ||||||||||||||||||
return parent::add_data( $handle, $key, $value ); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Checks all handles for any delayed inline scripts. | ||||||||||||||||||
* | ||||||||||||||||||
* @return bool True if the inline script present, otherwise false. | ||||||||||||||||||
*/ | ||||||||||||||||||
public function has_delayed_inline_script() { | ||||||||||||||||||
foreach ( $this->registered as $handle => $script ) { | ||||||||||||||||||
// Non standalone scripts in the after position, of type async or defer, are usually delayed. | ||||||||||||||||||
if ( in_array( $this->get_intended_strategy( $handle ), array( 'defer', 'async' ), true ) && | ||||||||||||||||||
$this->has_non_standalone_inline_script( $handle, 'after' ) | ||||||||||||||||||
) { | ||||||||||||||||||
return true; | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
return false; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Normalize the data inside the $args parameter and support backward compatibility. | ||||||||||||||||||
* | ||||||||||||||||||
* @param string $handle Name of the script. | ||||||||||||||||||
* @param array $args { | ||||||||||||||||||
* Optional. Additional script arguments. Default empty array. | ||||||||||||||||||
* | ||||||||||||||||||
* @type boolean $in_footer Optional. Default false. | ||||||||||||||||||
* @type string $strategy Optional. Values blocking|defer|async. Default 'blocking'. | ||||||||||||||||||
* } | ||||||||||||||||||
* @return array Normalized $args array. | ||||||||||||||||||
*/ | ||||||||||||||||||
private function get_normalized_script_args( $handle, $args = array() ) { | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Excellent! noting this will need a There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @adamsilverstein I think we need to seek clarity around this, as when searching the contents of I also can not find anything about this in the official coding standards, see link here. I'm happy to add it, as it changes little aside from having to update one or two references to the function, but I do feel that this is an area of preference as opposed to standard. If I am wrong, please let me know :) There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Since this is a private class method, the prefix is unnecessary. In |
||||||||||||||||||
$default_args = array( | ||||||||||||||||||
'in_footer' => false, | ||||||||||||||||||
'strategy' => 'blocking', | ||||||||||||||||||
); | ||||||||||||||||||
|
||||||||||||||||||
// Handle backward compatibility for $in_footer. | ||||||||||||||||||
if ( true === $args ) { | ||||||||||||||||||
$args = array( 'in_footer' => true ); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
return wp_parse_args( $args, $default_args ); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Get all dependents of a script. | ||||||||||||||||||
* | ||||||||||||||||||
* @param string $handle The script handle. | ||||||||||||||||||
* @return array Array of script handles. | ||||||||||||||||||
*/ | ||||||||||||||||||
private function get_dependents( $handle ) { | ||||||||||||||||||
$dependents = array(); | ||||||||||||||||||
|
||||||||||||||||||
// Iterate over all registered scripts, finding dependents of the script passed to this method. | ||||||||||||||||||
foreach ( $this->registered as $registered_handle => $args ) { | ||||||||||||||||||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. this loop execution time will grow as the number of registered handles grows. Probably fine, but I do have a concern that on large sites with many scripts it could become a bit heavy. a potential optimization here would be building a map of handle->array(dependents) the first time it is called, then use the map for subsequent calls. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @adamsilverstein I've amended this in a recent commit to make sue of a private var to maintain the mapping, and use this for a given handle should it exist. |
||||||||||||||||||
if ( in_array( $handle, $args->deps, true ) ) { | ||||||||||||||||||
$dependents[] = $registered_handle; | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
return $dependents; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Get the strategy assigned during script registration. | ||||||||||||||||||
* | ||||||||||||||||||
* @param string $handle The script handle. | ||||||||||||||||||
* @return string|bool Strategy set during script registration. False if none was set. | ||||||||||||||||||
*/ | ||||||||||||||||||
private function get_intended_strategy( $handle ) { | ||||||||||||||||||
$script_args = $this->get_data( $handle, 'script_args' ); | ||||||||||||||||||
|
||||||||||||||||||
return isset( $script_args['strategy'] ) ? $script_args['strategy'] : false; | ||||||||||||||||||
joemcgill marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Check if a script has a non standalone inline script associated with it. | ||||||||||||||||||
* | ||||||||||||||||||
* @param string $handle The script handle. | ||||||||||||||||||
* @param string $position Position of the inline script. | ||||||||||||||||||
* | ||||||||||||||||||
* @return bool True if script present. False if empty. | ||||||||||||||||||
*/ | ||||||||||||||||||
private function has_non_standalone_inline_script( $handle, $position ) { | ||||||||||||||||||
$non_standalone_script_key = $position . '-non-standalone'; | ||||||||||||||||||
$non_standalone_script = $this->get_data( $handle, $non_standalone_script_key ); | ||||||||||||||||||
|
||||||||||||||||||
return ! empty( $non_standalone_script ); | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Check if all of a scripts dependents are deferrable, which is required to maintain execution order. | ||||||||||||||||||
* | ||||||||||||||||||
* @param string $handle The script handle. | ||||||||||||||||||
* @param array $checked Optional. An array of already checked script handles, used to avoid recursive loops. | ||||||||||||||||||
* @return bool True if all dependents are deferrable, false otherwise. | ||||||||||||||||||
*/ | ||||||||||||||||||
private function has_only_deferrable_dependents( $handle, $checked = array() ) { | ||||||||||||||||||
// If this node was already checked, this script can be deferred and the branch ends. | ||||||||||||||||||
if ( in_array( $handle, $checked, true ) ) { | ||||||||||||||||||
return true; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
$checked[] = $handle; | ||||||||||||||||||
$dependents = $this->get_dependents( $handle ); | ||||||||||||||||||
|
||||||||||||||||||
// If there are no dependents remaining to consider, the script can be deferred. | ||||||||||||||||||
if ( empty( $dependents ) ) { | ||||||||||||||||||
return true; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
// Consider each dependent and check if it is deferrable. | ||||||||||||||||||
foreach ( $dependents as $dependent ) { | ||||||||||||||||||
// If the dependent script is not using the defer or async strategy, no script in the chain is deferrable. | ||||||||||||||||||
if ( ! in_array( $this->get_intended_strategy( $dependent ), array( 'defer', 'async' ), true ) ) { | ||||||||||||||||||
joemcgill marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||||||||||||||
return false; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
// If the dependent script has a non-standalone inline script in the 'before' position associated with it, do not defer. | ||||||||||||||||||
if ( $this->has_non_standalone_inline_script( $dependent, 'before' ) ) { | ||||||||||||||||||
return false; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
// Recursively check all dependents. | ||||||||||||||||||
if ( ! $this->has_only_deferrable_dependents( $dependent, $checked ) ) { | ||||||||||||||||||
return false; | ||||||||||||||||||
} | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
return true; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Get the best eligible loading strategy for a script. | ||||||||||||||||||
* | ||||||||||||||||||
* @param string $handle The registered handle of the script. | ||||||||||||||||||
* @return string $strategy return the final strategy. | ||||||||||||||||||
*/ | ||||||||||||||||||
private function get_eligible_loading_strategy( $handle ) { | ||||||||||||||||||
if ( ! isset( $this->registered[ $handle ] ) ) { | ||||||||||||||||||
return ''; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
$intended_strategy = $this->get_intended_strategy( $handle ); | ||||||||||||||||||
|
||||||||||||||||||
/* | ||||||||||||||||||
* Handle known blocking strategy scenarios. | ||||||||||||||||||
* | ||||||||||||||||||
* blocking if script args not set. | ||||||||||||||||||
* blocking if explicitly set. | ||||||||||||||||||
*/ | ||||||||||||||||||
if ( ! $intended_strategy || 'blocking' === $intended_strategy ) { | ||||||||||||||||||
return ''; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
// Handling async strategy scenarios. | ||||||||||||||||||
if ( 'async' === $intended_strategy && empty( $this->registered[ $handle ]->deps ) && empty( $this->get_dependents( $handle ) ) ) { | ||||||||||||||||||
return 'async'; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
// Handling defer strategy scenarios. | ||||||||||||||||||
if ( $this->has_only_deferrable_dependents( $handle ) ) { | ||||||||||||||||||
return 'defer'; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
return ''; | ||||||||||||||||||
} | ||||||||||||||||||
|
||||||||||||||||||
/** | ||||||||||||||||||
* Resets class properties. | ||||||||||||||||||
* | ||||||||||||||||||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What about
blocking
?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@westonruter this is not where we define a strategy, but where we get the value back from
get_eligible_loading_strategy()
. If the intended strategy (i.e the one supplied in$args
) was'blocking'
that still gets returned as''
and we treat that as blocking.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even if the intended strategy was set as
''
in$args
, that too is treated as the default strategy, which is blocking.