Download all WordPress assets on boot #1532
Conversation
bgrgicak
added
[Type] Enhancement
bgrgicak
changed the title
There was a problem hiding this comment.
This is cool to see.
I thought it would be an quick approval, but it ended up touching on some deeper questions I had with #1531 also. So I'll just leave questions and comments for now.
There was a problem hiding this comment.
I think this is looking pretty good, but there are some questions about how this applies to browser and device storage that would be good to address before merging.
I left a couple other questions too, but the storage related questions are most important IMO.
| const zipPath = '/tmp/wordpress-static-assets.zip'; | ||
| await php.writeFile( | ||
| zipPath, | ||
| new Uint8Array(await response.arrayBuffer()) | ||
| ); | ||
| await php.run({ | ||
| code: `<?php | ||
| $document_root = ${phpVar(php.requestHandler.documentRoot)}; | ||
| $zip_path = ${phpVar(zipPath)}; | ||
| $zip = new ZipArchive; | ||
| $res = $zip->open($zip_path); | ||
| if ($res !== TRUE) { | ||
| return; | ||
| } | ||
| for ($i = 0; $i < $zip->numFiles; $i++) { | ||
| $filename = $zip->getNameIndex($i); | ||
| $extractPath = str_replace('wordpress-static', $document_root, $filename); | ||
| // Create directories if they don't exist | ||
| if (substr($filename, -1) === '/') { | ||
| if (is_dir($extractPath)) { | ||
| continue; | ||
| } | ||
| mkdir($extractPath, 0777, true); | ||
| } else { | ||
| // Extract files | ||
| copy("zip://$zip_path#".$filename, $extractPath); | ||
| } | ||
| } | ||
| $zip->close(); | ||
| `, | ||
| }); | ||
| if (await php.fileExists(zipPath)) { | ||
| await php.unlink(zipPath); | ||
| } |
There was a problem hiding this comment.
I think this can be replaced by the unzipFile helper:
| const zipPath = '/tmp/wordpress-static-assets.zip'; | |
| await php.writeFile( | |
| zipPath, | |
| new Uint8Array(await response.arrayBuffer()) | |
| ); | |
| await php.run({ | |
| code: `<?php | |
| $document_root = ${phpVar(php.requestHandler.documentRoot)}; | |
| $zip_path = ${phpVar(zipPath)}; | |
| $zip = new ZipArchive; | |
| $res = $zip->open($zip_path); | |
| if ($res !== TRUE) { | |
| return; | |
| } | |
| for ($i = 0; $i < $zip->numFiles; $i++) { | |
| $filename = $zip->getNameIndex($i); | |
| $extractPath = str_replace('wordpress-static', $document_root, $filename); | |
| // Create directories if they don't exist | |
| if (substr($filename, -1) === '/') { | |
| if (is_dir($extractPath)) { | |
| continue; | |
| } | |
| mkdir($extractPath, 0777, true); | |
| } else { | |
| // Extract files | |
| copy("zip://$zip_path#".$filename, $extractPath); | |
| } | |
| } | |
| $zip->close(); | |
| `, | |
| }); | |
| if (await php.fileExists(zipPath)) { | |
| await php.unlink(zipPath); | |
| } | |
| await unzipFile( | |
| php.requestHandler.documentRoot, | |
| new File([ new Uint8Array(await response.arrayBuffer()) ], "assets.zip") | |
| ); |
There was a problem hiding this comment.
I tried, but it didn't work. I also couldn't get it to work with just unzip in PHP or JS.
The only way unzipping works for me is file by file, but I might be doing something wrong.
There was a problem hiding this comment.
This seems to be the problem:
$extractPath = str_replace('wordpress-static', $document_root, $filename);
If wp-content, wp-include etc was already at the zip root, you could unzip it into /wordpress and the helper would do the right thing.
|
Yay! Almost there, I left a few more comments |
|
@adamziel I'm merging this PR as-is, but let me know if we should make any additional changes. |
| * Don't download static assets if they're already downloaded. | ||
| * WordPress may be loaded either from a production release or a minified bundle. | ||
| * Minified bundles are shipped without most CSS files, JS files, and other static assets. | ||
| * Instead, they contain a list of remote assets in wordpress-remote-asset-paths. | ||
| * We use this list to determine if we should fetch them on demand or if they are already downloaded. | ||
| * If the list is empty, we assume the assets are already downloaded. | ||
| * See https://github.com/WordPress/wordpress-playground/pull/1531 to understand how we use the remote asset list to backfill assets on demand. |
There was a problem hiding this comment.
@bgrgicak I'd still rewrite this entire comment for clarity and probably make it at least 2-3 times as long as it is now. A few thing that stood out for me were:
- Where are we in the boot cycle when this function is called?
- When is WordPress loaded from a production release and when is it loaded from a minified bundle? What difference does it make for us in this function?
- What are remote assets?
- What is
wordpress-remote-asset-paths? I understand it is a file because I have the context fresh in my head, but I don't quite get it from the comment. What would be an example of the content of that file? Or, if not an inline example here, where can I go to see it? - Why do we assume the assets are already downloaded if the list is empty?
- Backfill remote asset listing when needed #1531 does not backfill assets on demand.
This could be a docblock for the entire backfillStaticFilesRemovedFromMinifiedBuild function.
There was a problem hiding this comment.
@dmsnell authored one of the best examples I know of what useful, high-quality inline documentation looks like:
Motivation for the change, related issues
To achieve offline support we need to download all WordPress files into the Playground filesystem. Today Playground downloads WordPress partially on boot and downloads assets as they are needed using fetch requests.
This PR downloads all assets into the Playground filesystem without blocking the boot process.
Implementation details
Zip files are generated using the WordPress build script and are accessible in the
/wp-VERSION/folder like any other assets.The download is triggered after WordPress is installed. The fetch is async and won't block the boot process.
In case some of the assets are needed immediately on boot they will still be downloaded using fetch to ensure they are available to WordPress.
Once the assets are downloaded they are stored in the Playground filesystem and will be served from there.
Testing Instructions (or ideally a Blueprint)
npx nx bundle-wordpress:nightly playground-wordpress-buildspackages/playground/wordpress-builds/public/wp-nightly/wordpress-static.zipfile was createdwordpress-static.zipfile was downloaded/wp-admin/