Conversation
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. |
36f227c to
84566ea
Compare
…lbacks. `WP_Script_Modules::add_hooks()` registered every print callback as `array( $this, 'method' )`, binding the callbacks to the instance that was the global when `wp-settings.php` ran `add_hooks()` on `after_setup_theme`. Replacing the `$wp_script_modules` global after that point changed where `wp_enqueue_script_module()` wrote, but not what was printed. Classic scripts and styles do not have this problem because their hooks are procedural functions that read the global when they run. Add procedural print functions that delegate to `wp_script_modules()` and register those instead. See #66100, #64484.
84566ea to
b461889
Compare
`block_editor_rest_api_preload()` runs REST requests in-process, including the post with `context=edit`. Rendering the post content enqueues each block's view assets. The function already restores `$wp_scripts` and `$wp_styles` after preloading, so classic view scripts and view styles are discarded. It did not restore `$wp_script_modules`, so a block's `viewScriptModule` stayed enqueued and was printed in the block editor. Back up and restore `$wp_script_modules` the same way. Depends on the script module print hooks reading the global instance (#66100). See #64484, #55151.
1c1e0de to
7931499
Compare
|
There's a potential follow-up to do the same with Interactivity API state, which could also be populated undesirably on preload. |
This reverts commit 89a0dbf.
…hook callbacks." This reverts commit c42ebcd.
`block_editor_rest_api_preload()` cloned each global into a backup variable, ran the preload against the original object, then assigned the clone to the global. The global ended up pointing at a different object than before the call, so anything bound to the original instance kept reading an object nothing enqueues into any more. Swap copies in for the duration of the preload and restore the originals afterwards, which is what `_wp_get_iframed_editor_assets()` already does. Hooks and other references bound to the original instances stay valid, so the script module print callbacks registered by `WP_Script_Modules::add_hooks()` read the restored instance. This removes the dependency on the script module print hooks resolving the global instance (#66100). Follow-up to [52733]. See #64484, #55151.
|
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 Core Committers: Use this line as a base for the props when committing in SVN: To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook. |
| $wp_scripts = $backup_wp_scripts; | ||
| $wp_styles = $backup_wp_styles; | ||
| // Restore the original $post, $wp_scripts, $wp_styles, and $wp_script_modules instances. | ||
| $post = $original_post; |
There was a problem hiding this comment.
If $original_post is not empty, should this call setup_postdata( $post ) too, potentially?
| $post = $original_post; | |
| $post = $original_post; | |
| if ( $post ) { | |
| setup_postdata( $post ); | |
| } |
There are endpoints which call setup_postdata(), including these methods:
WP_REST_Posts_Controller::prepare_item_for_response()WP_REST_Block_Renderer_Controller::get_item()WP_REST_Revisions_Controller::prepare_item_for_response()
Granted, this is not strictly in the scope of what is being fixed.
There was a problem hiding this comment.
You mean that more globals were susceptible to modification and calling setup_postdata() here is part of a more complete fix?
I'm happy to make that change, it's not behavior I've observed but it seems reasonable.
westonruter
left a comment
westonruter
left a comment
There was a problem hiding this comment.
A couple suggestions/questions, but these can be addressed later and aren't blocking.
Co-authored-by: Weston Ruter <westonruter@gmail.com>
2 participants
block_editor_rest_api_preload()renders the post for thecontext=editpreload, andWP_Block::render()enqueues view assets. The function restores$wp_scriptsand$wp_stylesafterwards (r52733) but not$wp_script_modules, so a block'sviewScriptModuleis printed in the editor.This swaps copies of
$post,$wp_scripts,$wp_styles, and$wp_script_modulesin for the duration of the preload and restores the original instances, rather than restoring copies as r52733 does. Hooks bound to the original instances stay valid, which is what the script module print callbacks need._wp_get_iframed_editor_assets()already does it this way.Test: a block with a view script, style, and module. After preloading, the script and style are not queued,
admin_print_footer_scriptsdoes not print the module, and a module enqueued before preloading is still printed.Trac ticket: https://core.trac.wordpress.org/ticket/64484
Follow-up to r52733 (#2312).
Use of AI Tools
AI assistance: Yes
Tool(s): Claude Code
Model(s): Claude Fable 5.1, Claude Opus
Used for: diagnosis, code, tests, description.
This Pull Request is for code review only. Please keep all other discussion in the Trac ticket. Do not merge this Pull Request. See GitHub Pull Requests for Code Review in the Core Handbook for more details.
🤖 Generated with Claude Code