-
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
Implement load handlers #50
Changes from 16 commits
04aabf5
e366d5d
0fafd88
386022e
fe7acc0
394405f
cc5dace
b558f34
f04a246
b5f4df4
d9a0f39
d110556
b6b7022
9e8c378
ffc40c1
8af918c
d952da5
b71b557
479d656
968d5b7
ad9d9c3
326555d
6187dd1
1321d6f
66ab60c
bbdad37
966be0c
dcbf0af
8e08454
b3f7225
4df5288
264853c
556ae91
d4be88d
180a6ae
4d0140d
a5b83a4
0269128
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 |
---|---|---|
|
@@ -319,7 +319,14 @@ public function do_item( $handle, $group = false ) { | |
$after_non_standalone_handle = $this->print_inline_script( $handle, 'after-non-standalone', false ); | ||
|
||
if ( $after_non_standalone_handle ) { | ||
$after_handle .= sprintf( "<script%s id='%s-js-after' type='text/template'>\n%s\n</script>\n", $this->type_attr, esc_attr( $handle ), $after_non_standalone_handle ); | ||
$after_handle .= sprintf( | ||
'<script%1$s id=\'%2$s-js-after\' type=\'text/template\' 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->has_load_later_inline = true; | ||
} | ||
} | ||
|
||
|
@@ -410,6 +417,9 @@ public function do_item( $handle, $group = false ) { | |
|
||
if ( '' !== $strategy ) { | ||
$strategy = ' ' . $strategy; | ||
if ( ! empty( $after_non_standalone_handle ) ) { | ||
$strategy .= sprintf( " onload='wpLoadAfterScripts(\"%s\")'", esc_attr( $handle ) ); | ||
} | ||
} | ||
$tag = $translations . $cond_before . $before_handle; | ||
$tag .= sprintf( | ||
|
@@ -499,7 +509,14 @@ public function print_inline_script( $handle, $position = 'after', $display = tr | |
|
||
if ( $display ) { | ||
if ( 'after-non-standalone' === $position ) { | ||
printf( "<script%s id='%s-js-after' type='text/template'>\n%s\n</script>\n", $this->type_attr, esc_attr( $handle ), esc_attr( $position ), $output ); | ||
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', | ||
$this->type_attr, | ||
esc_attr( $handle ), | ||
esc_attr( $position ), | ||
$output, | ||
PHP_EOL | ||
); | ||
} else { | ||
printf( "<script%s id='%s-js-%s'>\n%s\n</script>\n", $this->type_attr, esc_attr( $handle ), esc_attr( $position ), $output ); | ||
} | ||
|
@@ -771,6 +788,23 @@ public function add_data( $handle, $key, $value ) { | |
return parent::add_data( $handle, $key, $value ); | ||
} | ||
|
||
/** | ||
* Check all handles for any delayed inline scripts. | ||
kt-12 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
* | ||
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.
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. @mukeshpanchal27 We haven't fixed on the final |
||
* @return bool True if script present. False if empty. | ||
kt-12 marked this conversation as resolved.
Show resolved
Hide resolved
|
||
*/ | ||
public function maybe_has_delayed_inline_script() { | ||
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. @kt-12 this is a nitpick comment I guess, but I am wondering if If you agree, let's rename this to If I've misunderstood, please comment as such thank you :) 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. You are correct. I used maybe as there are certain cases where the final strategy of a script might change before we print them effecting it's inline scripts decision. For example a defer may change to blocking, but we never know it in print_scripts action. But i do agree |
||
foreach ( $this->registered as $handle => $script ) { | ||
if ( in_array( $this->get_intended_strategy( $handle ), array( 'defer', 'async' ), true ) ) { | ||
// non standalone after scripts of async or defer are usually delayed. | ||
if ( $this->has_non_standalone_inline_script( $handle, 'after' ) ) { | ||
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. These could be combined into a single |
||
return true; | ||
} | ||
} | ||
} | ||
return false; | ||
} | ||
|
||
/** | ||
* Normalize the data inside the $args parameter and support backward compatibility. | ||
* | ||
|
Original file line number | Diff line number | Diff line change | ||
---|---|---|---|---|
|
@@ -1843,6 +1843,26 @@ function wp_just_in_time_script_localization() { | |||
); | ||||
} | ||||
|
||||
|
||||
/** | ||||
* Prints a loader script if there is text/template registered script. | ||||
*/ | ||||
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. Per PHP Documentation Standards, Use proper format for function 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. Updated. Usage of |
||||
function wp_print_template_loader_script() { | ||||
$wp_scripts = wp_scripts(); | ||||
if ( $wp_scripts->maybe_has_delayed_inline_script() ) { | ||||
$output = <<<JS | ||||
let wpLoadAfterScripts = ( 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 could probably be a regular function rather than an arrow function.
|
||||
let scripts = document.querySelectorAll(`[type="text/template"][data-wp-executes-after="\${handle}"]`); | ||||
scripts.forEach( (script) => { | ||||
script.setAttribute("type","text/javascript"); | ||||
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.
Suggested change
WP use 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. @mukeshpanchal27 this is the JS code for modifying type after a script is loaded on the DOM. So we can't use |
||||
eval(script.innerHTML); | ||||
}) | ||||
} | ||||
JS; | ||||
printf( "<script id='wp-executes-after-js'>\n%s\n</script>\n", $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.
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. Fixed, but |
||||
} | ||||
} | ||||
|
||||
/** | ||||
* Localizes the jQuery UI datepicker. | ||||
* | ||||
|
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.
Similar to other scripts, use
\n
instead ofPHP_EOL
and use%s
as it is sufficient for these changes as we don't need to add translators or change the order of the arguments.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.
@mukeshpanchal27 I was trying the same initially but \n don't work when the parent string is in single quotes. It is instead printed as string
\n
. Numbered placeholder was added as there are multiple places i wanted the handle string and for numbered placeholder I need the enclosing string to be in single quotes.