Blueprints: Support zipped assets without top-level folder #491
Conversation
adamziel
added
[Type] Bug
| @@ -51,8 +51,7 @@ foreach ( ( glob( $plugin_path . '/*.php' ) ?: array() ) as $file ) { | |||
| echo 'NO_ENTRY_FILE'; | |||
| `, | |||
| }); | |||
| if (result.errors) throw new Error(result.errors); | |||
| if (result.text === 'NO_ENTRY_FILE') { | |||
| if (result.text.endsWith('NO_ENTRY_FILE')) { | |||
There was a problem hiding this comment.
Some plugins trigger warnings during the installation – let's check just the last part of the output text
| @@ -51,8 +51,7 @@ foreach ( ( glob( $plugin_path . '/*.php' ) ?: array() ) as $file ) { | |||
| echo 'NO_ENTRY_FILE'; | |||
| `, | |||
| }); | |||
| if (result.errors) throw new Error(result.errors); | |||
There was a problem hiding this comment.
Some plugins trigger warnings during the installation and they show as result.errors so we can't bale out here
| @@ -164,7 +164,7 @@ export class VFSResource extends Resource { | |||
|
|
|||
| /** @inheritDoc */ | |||
| get name() { | |||
| return this.resource.path; | |||
| return this.resource.path.split('/').pop() || ''; | |||
There was a problem hiding this comment.
The resource returned a full path as a file name and that threw off installAsset
Installing a plugin where the plugin files are directly at the top level does not work. This PR assumes that, unless the zip file contains only a single directory, the plugin files start at the top level. Related: #427 Unit tests included. Run `npm run dev` and confirm that adding ?gutenberg-pr=47739 to the URL installs the PR.
adamziel
force-pushed
the
install-assets-without-top-level-folder
branch
from
4ebf208
to
e30ec9b
Compare
|
I'm going to merge to get the |
|
I see, thank you for the quick update, to make the |
|
WordPress can be confusing! Do you think this fix works, though? I know your PR specifically looked for a directory inside a zip. I also thought it made sense but I can't recall my own thinking. Since this PR removes that lookup, I wanted to check in with you to make sure my change didn't break any larger assumptions. |
My previous assumption was that plugin zip packages needed to contain a single top-level folder. I learned it from working on a plugin update server and corresponding updater module in a number of plugins, which integrated with the WordPress plugin update process. There's a particular way the zip package is generated from a plugin, like: zip -r plugin-name.zip plugin-nameThe other ("wrong") way is: cd plugin-name; zip -r plugin-name.zip .I also seem to remember when I submitted a zip package for approval into the official plugin directory, it needed to have a single top-level folder inside. But I can't find that requirement written anywhere. It seems best for Looks like this succinct line covers it: const zipHasRootFolder =
files.length === 1 && (await playground.isDir(files[0])); |
Installing a plugin where the plugin files are directly at the top level does not work. This PR assumes that, unless the zip file contains only a single directory, the plugin files start at the top level.
Related: #427
Unit tests included.
Run
npm run devand confirm that adding ?gutenberg-pr=47739 to the URL installs the PR.cc @eliot-akira – I completely missed that case