-
Notifications
You must be signed in to change notification settings - Fork 4.3k
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
feat: Experimental editor assets REST API endpoint #64517
base: trunk
Are you sure you want to change the base?
Conversation
Increase consistency, focus on explaining the reason for the referenced code.
Denote the endpoint's unstable nature.
Disable unused variable warning as the `$request` parameter must be referenced to satisfy inherited class requirements. ``` Fatal error: Declaration of WP_REST_Block_Editor_Assets_Controller::get_items_permissions_check() must be compatible with WP_REST_Controller::get_items_permissions_check($request) in /var/www/html/wp-content/plugins/gutenberg/lib/experimental/class-wp-rest-block-editor-assets-controller.php on line 144 ```
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. |
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.
Instead of this new PR, please can you review my PR - #21244.
Thank you for sharing this helpful context, @spacedmonkey. 🙇🏻 I reviewed the earlier PR — it is very cool and appears quite robust! I am concerned that its original goals may differ too much from the WordPress mobile team’s current goals. That divergence may complicate using the prior implementation for our particular needs. More specifically, the following are a few challenges I encountered in my research. I’m unsure if/how your prior implementation might address them. I welcome your thoughts on these. First, third-party plugins often fail to globally register conditionally utilized assets, instead the assets are only conditionally enqueued. This results in assets un-queryable at the global level. As quoted below, the WPGraphQL project referenced this same challenge in its own asset query feature as a “known issue.” In my proposed changes, I mitigate this issue by simulating an editor environment through applying relevant hooks, a very unique action for the particular use case of retrieving all editor assets.
Second, the lower level nature of the prior implementation provides a robust foundation, but comes at the expense of requiring the client to reimplement dependency features WordPress already performs — e.g., sorting dependencies for correct loading order, deduplicating dependencies shared across assets, and building script/link tags. Rather than building this functionality in the mobile app, my proposed changes rely upon WordPress for these necessary actions. Lastly, the prior implementation’s current form appears to only support querying a single asset by handle, rather than querying all assets for the editor; I presume we could adapt it to support querying all assets, though. My proposed changes focus on the specific need of loading all editor assets, rather than a single asset. Again, thank you for sharing the past context. I’m interested to hear your thoughts on the challenges I share and the WordPress mobile team’s specific goal; it could be that both implementations provide unique value/capability for achieving the mobile team's goal. And, of course, please let me know if I misinterpreted anything in my review of your prior PR. I look forward to exploring this together! |
To give you a little more context, I am a maintainer of the REST API in core and a core committer. I am the committer most likely to get this into core. I agree that there are many issues with scripts being dynamically registered. It is likely that we will only be able to support scripts and styles that are registered by the register_block_type_from_metadata function. We may also need to manually trigger the register_core_block_style_handles function and possibly the enqueue_block_assets action as well. I understand that my endpoint may not exactly match what you need, but it is a start. I strongly believe we can get it to a point where it could serve your purposes. However, I must emphasize that any endpoint that ends up in core needs to be generic and function as an interface that will remain in core indefinitely. Long after the mobile application codebase has changed, this endpoint will need to work the same way, as we promise backward compatibility. My endpoint is written to serve anyone requesting scripts and styles, not just a specific use case. For example, this endpoint could be used in a headless application. For these reasons, I do not support the current format of this PR, as I don’t feel the endpoint is generic enough for other uses. In its current state, I would block it from being committed to core. However, I believe that my PR is a good base to build on. Could you take my PR and build on top of it, @dcalhoun? I am willing to pair on this, but I have limited time as I am about to become a father. |
@spacedmonkey thank you for providing additional details for your perspective on these proposed changes. I understand and recognize the value in a more generic approach that might better serve additional use cases. Currently, I am exploring an alternative approach inspired by the web editor's I'll close this PR for the time being, as it seems a different approach — either the aforementioned Congratulations on fatherhood, that is wonderful! |
This cleared out previously registered dependencies by core and plugins.
It appears third-party plugins often rely upon this hook for enqueuing assets.
These are necessary for the post editor to function.
What?
Enable requesting all assets — styles and scripts — loaded for the block editor via the REST API.
Why?
The WordPress mobile team is exploring a WebView-based editor where a given site's editor assets power the editing experience. If deemed successful, the experimental approach might bring a few benefits to the iOS and Android mobile apps editing experience:
How?
Add an experimental block editor assets REST API endpoint. The endpoint returns a collection of relevant assets utilizing an approach similar to WordPress core's iframe editor assets utility.
Testing Instructions
Without Permission
curl -L <site_url>/?rest_route=/__experimental/wp-block-editor/v1/editor-assets
With Permission
admin
user).echo -n 'admin:<application_password>' | base64 | pbcopy
curl --header "Authorization: Basic <base64_string>" -L <site_url>/?rest_route=/__experimental/wp-block-editor/v1/editor-assets
Testing Instructions for Keyboard
N/A, no user-facing changes.
Screenshots or screencast
N/A, no user-facing changes.