Edit Site: Add theme exporter. #22922
Conversation
epiqueras
added
[Feature] Full Site Editing
[Type] Experimental
|
Size Change: +4.91 kB (0%) Total Size: 1.13 MB
ℹ️ View Unchanged
|
| } ) | ||
| .then( ( res ) => res.blob() ) | ||
| .then( ( blob ) => | ||
| downloadjs( |
There was a problem hiding this comment.
Is there not a built-in way to download a file from WordPress? I don't like that we have to add a whole new dependency for it 😕
There was a problem hiding this comment.
Not that I know of. It's a tiny browser-compat shim.
There was a problem hiding this comment.
Can't we just "link" to the download URL instead of using apiFetch?
There was a problem hiding this comment.
We would need to persist the file in a public directory. This seemed cleaner.
There was a problem hiding this comment.
I'm pretty sure that's not needed since the request already sends a zip file but maybe there's something I'm missing.
There was a problem hiding this comment.
Oh, you mean a link to the URL using an href. I thought you meant having the API return the link to the file.
That won't work either, because we need the nonce middleware and all that for authentication. Unless you think there is another way to authenticate there?
There was a problem hiding this comment.
I believe if you use a link, you might not need the nonces and things like that as it's a "GET" request and it uses the session right?
There was a problem hiding this comment.
I tried it, but it's still needed.
The API fetch is also a "GET" request.
There was a problem hiding this comment.
It's definitely weird to bring in downloadjs just for this, even if it's not a huge lib.
The limitation with directly accessing the endpoint lies not in the nonce requirement, since that can be satisfied with the _wpnonce GET parameter, but in the authentication itself.
My reflex here was that we should try to make this work with plain URLs, but after digging a bit I don't think it's very practical.
However, depending on downloadjs is a little unfortunate, and looking at its source it seems like it does a lot of legwork to support older browsers that we don't need to care about anymore, not to mention that it supports different download methods (e.g. URL-based). Could we replace it with something dead-simple? The following works for me in my browser console:
function downloadBlob( data, filename, mimeType ) {
const blob = new Blob( [ data ], { type: mimeType } );
const url = URL.createObjectURL( blob );
const anchor = document.createElement( 'a' );
anchor.download = filename;
anchor.href = url;
document.body.appendChild( anchor );
anchor.click();
document.body.removeChild( anchor );
}
downloadBlob( '<html>...</html>', 'home.html', 'text/html' );If this is worthwhile I can open a PR.
| $filename = tempnam( get_temp_dir(), 'edit-site-export' ); | ||
| $zip = new ZipArchive(); | ||
| $zip->open( $filename, ZipArchive::OVERWRITE ); | ||
| $zip->addEmptyDir( 'theme' ); |
There was a problem hiding this comment.
Why not put block-templates and block-template-parts at the top-level? (Unless I'm wrong about typical theme .zip files having these directories on the top-level)
There was a problem hiding this comment.
Because unzipping a zip file yields its contents, not its contents wrapped in an extra directory. Try downloading the file to see what I mean.
There was a problem hiding this comment.
Let's maybe make a constant for the theme directory name? We might later want to add an option for the user to give a name to the theme 🤔
There was a problem hiding this comment.
We can change it then. I don't see much value in complicating this very simple code with lots of string joins when it's not needed yet.
|
Thanks! |
| $zip->close(); | ||
| header( 'Content-Type: application/zip' ); | ||
| header( 'Content-Disposition: attachment; filename=edit-site-export.zip' ); | ||
| header( 'Content-Length: ' . filesize( $filename ) ); | ||
| flush(); | ||
| echo readfile( $filename ); |
There was a problem hiding this comment.
Shouldn't we clean up, delete the file? Especially since it's using temporary filenames, so we're not just overwriting the same file.
There was a problem hiding this comment.
Is there an issue with keeping it around? Deleting it would technically be more work.
There was a problem hiding this comment.
If we do make changes here we should also switch to wp_tempnam which I missed the first time round.
There was a problem hiding this comment.
Curious if that's just for consistency/style or is there some benefit to it?
There was a problem hiding this comment.
I think its mostly consistency and style, but it does appear to have some edge case handling.
There was a problem hiding this comment.
Is there an issue with keeping it around?
I don't think we can make assumptions on how often the location returned by get_temp_dir() is flushed. We must assume that ZIP files would proliferate over time.
Deleting it would technically be more work.
How so?
There was a problem hiding this comment.
How so?
You always have to create it/overwrite it. Deleting it is another set of operations we could avoid, although I doubt it makes a measurable difference.
I think we can be safe here by using the same file name every time. That way it just overwrites the same file if it hasn't been cleaned up already.
There was a problem hiding this comment.
For the record, @epiqueras and I talked about this over Slack, and my recommendation was to ensure good random filenames (WordPress's wp_tempnam might be better at avoiding naming conflicts) and ensure we delete the file in the moment.
This stands in contrast with adopting a fixed filename so as to overwrite the file over time. This might be enough in simple sites, but is bound to cause race-condition-type problems in any larger installation, and especially in multisite or highly concurrent scenarios.
| } ) | ||
| .then( ( res ) => res.blob() ) | ||
| .then( ( blob ) => | ||
| downloadjs( |
There was a problem hiding this comment.
It's definitely weird to bring in downloadjs just for this, even if it's not a huge lib.
The limitation with directly accessing the endpoint lies not in the nonce requirement, since that can be satisfied with the _wpnonce GET parameter, but in the authentication itself.
My reflex here was that we should try to make this work with plain URLs, but after digging a bit I don't think it's very practical.
However, depending on downloadjs is a little unfortunate, and looking at its source it seems like it does a lot of legwork to support older browsers that we don't need to care about anymore, not to mention that it supports different download methods (e.g. URL-based). Could we replace it with something dead-simple? The following works for me in my browser console:
function downloadBlob( data, filename, mimeType ) {
const blob = new Blob( [ data ], { type: mimeType } );
const url = URL.createObjectURL( blob );
const anchor = document.createElement( 'a' );
anchor.download = filename;
anchor.href = url;
document.body.appendChild( anchor );
anchor.click();
document.body.removeChild( anchor );
}
downloadBlob( '<html>...</html>', 'home.html', 'text/html' );If this is worthwhile I can open a PR.
I saw lots of variations of that in StackOverflow, but most comments recommended using I guess we can make our own package that targets the browsers WordPress cares about and test it thoroughly. |
If this sort of StackOverflow feedback is recent, then I guess we shouldn't be reinventing the wheel. If not, well… I don't care enough about the issue, I just would like to think that we don't need special libraries to download blobs in 2020. :) |
I know, classic Web. |
|
Looking at something totally unrelated, I stumbled upon this: gutenberg/packages/list-reusable-blocks/src/utils/file.js Lines 8 to 25 in 49003cb |
|
Mmm, @youknowriad should we use that instead? |
Closes #19260
Description
This PR adds a theme exporter button to the site editor header's "More" dropdown.
It is like #21958 but implements the zip creation in the server to avoid loading a large zip management library in the client.
How to test this?
Try exporting templates/parts with different modifications and verify that the exported files match what you have on the editor.
Screenshots
Types of Changes
New Feature: The site editor now has a theme exporter button for exporting customized templates and template parts in a theme zip file.
Checklist: