Add manifest data #1
Conversation
| 'scripts_html' => $scripts, | ||
| 'scripts' => $script_urls, | ||
| 'styles' => $style_urls, | ||
| 'hash' => hash( 'sha256', join($script_urls) . join($style_urls) ), |
There was a problem hiding this comment.
WordPress allows inline scripts that do not have a src attribute. Should the hash instead rely upon the id attribute to ensure we account for all dependencies?I believe all dependencies have an id attributes—at least WP 5.5 and later.
There was a problem hiding this comment.
If we just hash the id attribute, the content could change within a script and we wouldn't detect it, right?
Whereas hopefully the URLs all contain their own hash of the contents of the file?
Inline scripts wouldn't invalidate the hash because we drop the scripts_html verbatim into the HTML page, so we should always have the latest version (we'd always need to validate the hash against what we already have before loading the editor, or if we can't reach the Internet, we'll proceed offline)
There was a problem hiding this comment.
If we just hash the
idattribute, the content could change within a script and we wouldn't detect it, right?Whereas hopefully the URLs all contain their own hash of the contents of the file?
Yeah, that's a good point. Using an id wouldn't be enough.
Inline scripts wouldn't invalidate the hash because we drop the
scripts_htmlverbatim into the HTML page, so we should always have the latest version (we'd always need to validate the hash against what we already have before loading the editor, or if we can't reach the Internet, we'll proceed offline)
My worry related to a scenario where only an inline script changed. Presumably, in that context, the hash would not change, and we'd load a cached HTML file that includes outdated inline scripts. Is that incorrect?
| 'methods' => WP_REST_Server::READABLE, | ||
| 'callback' => array( $this, 'get_items' ), | ||
| 'permission_callback' => array( $this, 'get_items_permissions_check' ), | ||
| 'permission_callback' => '__return_true', |
There was a problem hiding this comment.
Friendly reminder that we should revert this before merging.
No description provided.