-
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
Editor: Update the WordPress Packages to Gutenberg 17.6 RC3 #5984
Conversation
f2709f2
to
7ee5a67
Compare
7ee5a67
to
0765a2d
Compare
@@ -53,32 +53,11 @@ module.exports = function ( | |||
}, | |||
environment: { module: true }, | |||
}, | |||
module: { |
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.
Now that the modules are built automatically, there's no need to use babel anymore.
); | ||
} | ||
|
||
add_action( 'init', 'wp_interactivity_register_script_modules' ); |
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.
Should we introduce a new action similar to wp_default_scripts
?
Inside src/wp-includes/default-filters.php
, we would put:
add_action( 'wp_default_script_modules', 'wp_default_script_modules_packages' );
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.
That makes sense, I believe this is meant as a start though. @luisherranz is planning a follow-up for this PR to bring the full interactivity API.
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.
Yes. Once this is committed, my plan is to delete it in #5953, which already includes these filters.
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.
In #5953, it is no longer part of the init
hook, but instead the script modules for Interactivity API get registered when someone calls wp_interactivity()
function.
Test using WordPress PlaygroundThe changes in this pull request can previewed and tested using a WordPress Playground instance. WordPress Playground is an experimental project that creates a full WordPress instance entirely within the browser. Some things to be aware of
For more details about these limitations and more, check out the Limitations page in the WordPress Playground documentation. |
@@ -1358,7 +1358,7 @@ function block_core_navigation_get_most_recently_published_navigation() { | |||
* @param WP_Post $post `wp_navigation` post object corresponding to the block. | |||
* @return string Serialized inner blocks in mock Navigation block wrapper, with hooked blocks inserted, if any. | |||
*/ | |||
function block_core_navigation_insert_hooked_blocks( $inner_blocks, $post = null ) { | |||
function block_core_navigation_insert_hooked_blocks( $inner_blocks, $post ) { |
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.
Would making $post
now to be required could cause Fatal error: Uncaught ArgumentCountError: Too few arguments to function
if someone is using it without it? Was this function introduced before 6.5?
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.
Looks like this has been discussed. See WordPress/gutenberg#58379 (comment) cc @ockham
if ( function_exists( 'get_hooked_blocks' ) ) { | ||
// Injection of hooked blocks into the Navigation block relies on some functions present in WP >= 6.5 | ||
// that are not present in Gutenberg's WP 6.5 compatibility layer. | ||
if ( function_exists( 'get_hooked_block_markup' ) ) { |
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.
@ockham, should it be moved to WordPress Core instead? It looks like it will never get executed in the Gutenberg plugin with the guard call included.
There are some remaining comments I left, but they aren't blockers for this PR. |
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.
Everything looks good on the Interactivity API bundling. Thanks, Riad! 🙂
); | ||
} | ||
|
||
add_action( 'init', 'wp_interactivity_register_script_modules' ); |
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.
Yes. Once this is committed, my plan is to delete it in #5953, which already includes these filters.
Trac ticket: https://core.trac.wordpress.org/ticket/60315
This PR updates the WordPress packages to the latest version (which is current Gutenberg 17.6 RC3)
It brings with a set of iterations and follow-ups to the initial PR #30428
It also fixes a regression that happened for interactive blocks.