-
Notifications
You must be signed in to change notification settings - Fork 2.7k
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
Allow classic scripts to depend on modules #8024
base: trunk
Are you sure you want to change the base?
Changes from all commits
39a11a8
65fd1df
843e1e0
2e180cf
7fc42d2
5144a90
d83ea4b
9125683
5ddc2dd
9e976bb
591e3b5
d1d8c43
10c3a02
a444ffa
b0f579b
8366f45
45cacbb
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 | ||||||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
@@ -208,10 +208,15 @@ public function add_hooks() { | |||||||||||||||||||||
*/ | ||||||||||||||||||||||
public function print_enqueued_script_modules() { | ||||||||||||||||||||||
foreach ( $this->get_marked_for_enqueue() as $id => $script_module ) { | ||||||||||||||||||||||
$src = $this->get_src( $id ); | ||||||||||||||||||||||
if ( null === $src ) { | ||||||||||||||||||||||
continue; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
wp_print_script_tag( | ||||||||||||||||||||||
array( | ||||||||||||||||||||||
'type' => 'module', | ||||||||||||||||||||||
'src' => $this->get_src( $id ), | ||||||||||||||||||||||
'src' => $src, | ||||||||||||||||||||||
'id' => $id . '-js-module', | ||||||||||||||||||||||
) | ||||||||||||||||||||||
); | ||||||||||||||||||||||
|
@@ -228,11 +233,16 @@ public function print_enqueued_script_modules() { | |||||||||||||||||||||
*/ | ||||||||||||||||||||||
public function print_script_module_preloads() { | ||||||||||||||||||||||
foreach ( $this->get_dependencies( array_keys( $this->get_marked_for_enqueue() ), array( 'static' ) ) as $id => $script_module ) { | ||||||||||||||||||||||
$src = $this->get_src( $id ); | ||||||||||||||||||||||
if ( null === $src ) { | ||||||||||||||||||||||
continue; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
// Don't preload if it's marked for enqueue. | ||||||||||||||||||||||
if ( true !== $script_module['enqueue'] ) { | ||||||||||||||||||||||
echo sprintf( | ||||||||||||||||||||||
'<link rel="modulepreload" href="%s" id="%s">', | ||||||||||||||||||||||
esc_url( $this->get_src( $id ) ), | ||||||||||||||||||||||
esc_url( $src ), | ||||||||||||||||||||||
esc_attr( $id . '-js-modulepreload' ) | ||||||||||||||||||||||
); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
@@ -262,14 +272,53 @@ public function print_import_map() { | |||||||||||||||||||||
* | ||||||||||||||||||||||
* @since 6.5.0 | ||||||||||||||||||||||
* | ||||||||||||||||||||||
* @return array Array with an `imports` key mapping to an array of script module identifiers and their respective | ||||||||||||||||||||||
* URLs, including the version query. | ||||||||||||||||||||||
* @global WP_Dependencies $wp_scripts | ||||||||||||||||||||||
* | ||||||||||||||||||||||
* @return array Array with an `imports` key mapping to an array of script | ||||||||||||||||||||||
* module identifiers and their respective URLs, including | ||||||||||||||||||||||
* the version query. | ||||||||||||||||||||||
*/ | ||||||||||||||||||||||
private function get_import_map(): array { | ||||||||||||||||||||||
global $wp_scripts; | ||||||||||||||||||||||
|
||||||||||||||||||||||
$imports = array(); | ||||||||||||||||||||||
foreach ( $this->get_dependencies( array_keys( $this->get_marked_for_enqueue() ) ) as $id => $script_module ) { | ||||||||||||||||||||||
$imports[ $id ] = $this->get_src( $id ); | ||||||||||||||||||||||
|
||||||||||||||||||||||
$classic_script_dependencies = array(); | ||||||||||||||||||||||
if ( $wp_scripts instanceof WP_Scripts ) { | ||||||||||||||||||||||
foreach ( $wp_scripts->registered as $dependency ) { | ||||||||||||||||||||||
$handle = $dependency->handle; | ||||||||||||||||||||||
|
||||||||||||||||||||||
if ( | ||||||||||||||||||||||
! $wp_scripts->query( $handle, 'done' ) && | ||||||||||||||||||||||
! $wp_scripts->query( $handle, 'to_do' ) && | ||||||||||||||||||||||
! $wp_scripts->query( $handle, 'enqueued' ) | ||||||||||||||||||||||
) { | ||||||||||||||||||||||
continue; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
Comment on lines
+291
to
+297
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. Is it sufficient to just check
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. Apparently yes. It seems that 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. Oh, maybe not, because this is not accounting for the dependencies of an enqueued script. So I think you do need to check |
||||||||||||||||||||||
|
||||||||||||||||||||||
$module_deps = $wp_scripts->get_data( $handle, 'module_deps' ); | ||||||||||||||||||||||
if ( ! $module_deps ) { | ||||||||||||||||||||||
continue; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
foreach ( $module_deps as $id ) { | ||||||||||||||||||||||
$src = $this->get_src( $id ); | ||||||||||||||||||||||
if ( null === $src ) { | ||||||||||||||||||||||
continue; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
$imports[ $id ] = $src; | ||||||||||||||||||||||
$classic_script_dependencies[] = $id; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
} | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
foreach ( $this->get_dependencies( array_merge( $classic_script_dependencies, array_keys( $this->get_marked_for_enqueue() ) ) ) as $id => $script_module ) { | ||||||||||||||||||||||
$src = $this->get_src( $id ); | ||||||||||||||||||||||
if ( null === $src ) { | ||||||||||||||||||||||
continue; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
$imports[ $id ] = $src; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
return array( 'imports' => $imports ); | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
|
@@ -335,11 +384,11 @@ function ( $dependency_script_modules, $id ) use ( $import_types ) { | |||||||||||||||||||||
* @since 6.5.0 | ||||||||||||||||||||||
* | ||||||||||||||||||||||
* @param string $id The script module identifier. | ||||||||||||||||||||||
* @return string The script module src with a version if relevant. | ||||||||||||||||||||||
* @return string|null The script module src with a version if relevant. | ||||||||||||||||||||||
*/ | ||||||||||||||||||||||
private function get_src( string $id ): string { | ||||||||||||||||||||||
private function get_src( string $id ): ?string { | ||||||||||||||||||||||
if ( ! isset( $this->registered[ $id ] ) ) { | ||||||||||||||||||||||
return ''; | ||||||||||||||||||||||
return null; | ||||||||||||||||||||||
} | ||||||||||||||||||||||
|
||||||||||||||||||||||
$script_module = $this->registered[ $id ]; | ||||||||||||||||||||||
|
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.
This is similar to what
wp_dependencies_unique_hosts
does:wordpress-develop/src/wp-includes/general-template.php
Lines 3724 to 3726 in ef76060
I'm not confident this is the most optimal way to get all classic scripts in the enqueued dependency graph so would appreciate scrutiny or domain knowledge on that.
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.
This probably isn't much more efficient, but you could instead loop over
array_unique( array_merge( $wp_scripts->done, $wp_scripts->to_do, $wp_scripts->queue ) )
. That will give you all the relevant handles and you can then eliminate the belowif
statement.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.
The most important thing, however, is to be sure that
$wp_scripts->all_deps()
has run thisget_import_map()
method is called. This gets called when printing scripts, like in the head and footer, and when printing out scripts manually. But I suppose the importmap is printed at the very end of the document anyway since there currently can only be one?