-
Notifications
You must be signed in to change notification settings - Fork 4.6k
Build: Auto-register script modules #72541
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
Conversation
|
The following accounts have interacted with this PR and/or linked issues. I will continue to update these lists as activity occurs. You can also manually ask me to refresh this list by adding the If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message. To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
lib/client-assets.php
Outdated
| 'fetchpriority' => 'low', | ||
| ); | ||
|
|
||
| gutenberg_register_interactive_script_module_id( $script_module_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.
I think this is the only thing that I'm unsure about, what is this for, which modules should be marked as "interactive", all of them? Can you help @DAreRodz
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.
Please, merge/rebase trunk in this branch, as that code has been changed here.
Basically, we're using the wp_script_attributes filter to add a new directive (data-wp-router-options='{"loadOnClientNavigation":true}') to all the script modules that belong to interactive blocks. This helps detect them when the Interactivity API router navigates and needs to load the ones that didn't exist on the previous page.
More info on the WP Core patch:
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.
which modules should be marked as "interactive", all of them?
Here we are marking all the script modules belonging to the interactive Core blocks, since they are all compatible with client-side navigation.
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 code in trunk was merging all the modules without exceptions as compatible. I can update the code to explicitly mark the block ones. (after rebase)
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.
We did it like this in the PR:
if ( str_starts_with( $script_module_id, '@wordpress/block-library' ) ...
wp_interactivity()->add_client_navigation_support_to_script_module( $script_module_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.
I kept it as is for now but outside the scripts registration. So it should work the same but I don't really know what to test :)
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.
I don't think we have tests for the Core blocks, just for the third-party blocks. I'll check it out, don't worry.
04c5927 to
0ff77e8
Compare
|
Size Change: 0 B Total Size: 2.19 MB ℹ️ View Unchanged
|
0ff77e8 to
6414238
Compare
|
Flaky tests detected in e8aab75. 🔍 Workflow run URL: https://github.com/WordPress/gutenberg/actions/runs/18725435060
|
|
I'd appreciate a final review here. Things work well for me and I would love to expand this to scripts, styles in a follow-up. |
|
Oh forgot the props, sorry Luis 😬 |
|
Looks like this PR improved the server loading metrics, My guess is it's because of the removal of the directory travel to find modules and the move to explicit discovery (build based) |
Related #72032
What
This continues the work on the build tool. This is the first PR where we're introducing automatic php code generation. This PR auto-registers script modules based on the packages built in the current plugin.
Why?
wpPlugin.prefixconfig (support other plugins later)How?
build/modules/index.php- module registry data arraybuild/index.php- registration logic with hookslib/client-assets.phpTesting Instructions
You can check that interactive blocks continue to work on the frontend.