WP Scripts: Add a --root-folder argument to the plugin-zip command
#61375
Conversation
nicolasgalvez
requested review from
gziolo,
ntwb,
nerrad,
ajitbohra and
ryanwelcher
as code owners
|
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. |
github-actions
bot
added
the
First-time Contributor
|
👋 Thanks for your first Pull Request and for helping build the future of Gutenberg and WordPress, @nicolasgalvez! In case you missed it, we'd love to have you join us in our Slack community. If you want to learn more about WordPress development in general, check out the Core Handbook full of helpful information. |
gziolo
added
[Tool] WP Scripts
aaronjorbin
added
the
props-bot
github-actions
bot
removed
the
props-bot
|
Tested and working locally (WSL2 + PHP 8.2.x + WP 6.6 + Node 20 ) One suggestion: In its current form, this is an additive enhancement, that requires However, since the docs currently say
|
|
I like the suggestions shared by @justlevine. That would be the ideal scenario as the default would better align with the best practices. At the same time there would be an easy migration path to include in the changelog entry in the case someone has some workflow that depends on the fact that there is no folder included. They would only need the following change: - wp-scripts plugin-zip
+ wp-scripts plugin-zip --root-folder=""@nicolasgalvez, are you available to apply the necessary changes? |
|
@gziolo @justlevine I'm sorry for the delayed response. Thanks for the help and the great suggestion! I agree it would be great if it was the default behavior and I'll make the change today. |
|
New manual tests Default No Root Folder Custom root folder |
…zip file by default
1. The plugin-zip command will now create a root folder in the zip file matching the name of the plugin. This aligns with the documentation stating that "By default, it uses Plugin Handbook best practices to discover files."
2. `--zip-root-folder` argument from my previous commit is now named `--root-folder`
3. The --root-folder argument will allow specifying a custom folder, or no folder for backwards compatibility.
Example usage:
{{{
npm run plugin-zip --root-folder='foo'
}}}
Follow-up to [61375], [60481].
Props justlevine, gziolo.
nicolasgalvez
requested review from
TimothyBJacobs,
spacedmonkey,
geriux,
dmsnell,
tellthemachines,
ObliviousHarmony,
t-hamano,
desrosj,
ellatrix,
adamsilverstein,
swissspidy and
kevin940726
as code owners
nicolasgalvez
requested review from
ZebulanStanphill,
michalczaplinski,
mmtr,
juanmaguitar and
fabiankaegy
as code owners
nicolasgalvez
force-pushed
the
add/plugin-zip-directory
branch
from
a3ae55a to
0c59cbe
Compare
|
I have confirmed that it works correctly on a Windows host OS (not WSL). When I first saw the I think it would be good to use the following three patterns, but what do you think?
|
@t-hamano what are your thoughts about just requiring a Personally, I find semantic-only aliases to offer a worse DX than guardrails (here, needing to learn 2 flags and how they interrelate), and adds tech debt (e.g. need to deconflict both flags being passed in the same command), but I'm not sure if there's a WP-specific design convention here. |
|
@t-hamano I agree, it seems weird but I couldn't put my finger on why when working on it. Thanks for pointing that out and thank you thank you for testing on Windows! I think I agree with the points @justlevine makes and I would like to implement the solution outlined below if that's cool with everyone:
Thanks for the help! |
Yes, this looks like a good approach to me 👍 |
|
I added a commit f720100 with the CHANGELOG entry explaining the changes applied and a note on how to replicate the old behavior. Is there anything left to land this enhancement? |
gziolo
changed the title
|
@t-hamano @justlevine Well, I discovered the zipRootFolderArg automatically converts falsy values to null, so there isn't a way to differentiate between and empty string '' and null. Given this, I'll go with @t-hamano's suggestion after all. @gziolo Thank you very much for adding the change message, I will update the message to match the behavior @t-hamano suggested: wp-scripts plugin-zip: Create the root folder Testing on macos: Default No Root Folder Custom root folder I'll keep an eye out for any other comments on here, thanks again everyone! |
Require an argument for root-folder. Add option no-root-folder to zip files without a root folder for backwards compatibility. Update documentation, and changelog. (WordPress#60481)
| const trimmedZipRootFolderArg = | ||
| typeof zipRootFolderArg === 'string' ? zipRootFolderArg.trim() : null; | ||
| if ( trimmedZipRootFolderArg === null ) { | ||
| stdout.write( |
There was a problem hiding this comment.
Nit: this could use some formatting to better indicate that the param was misconfigured and the zip wasn't created.
There was a problem hiding this comment.
I updated the message to Unable to create zip package: please provide a --root-folder name or use `--no-root-folder.
gziolo
changed the title
--root-folder argument to the plugin-zip command
Apply suggestions from code review. props: @gziolo Co-authored-by: Greg Ziółkowski <[email protected]>
Co-authored-by: Aki Hamano <[email protected]>
|
@nicolasgalvez, awesome work! I accepted the suggestion from @t-hamano regarding the documentation. I also set the PR to auto-merge 🚀 |
WordPress#61375) * Scripts: Add a --zip-root-folder argument to the plugin-zip command (WordPress#60481) * Scripts: Change the plugin-zip command to add a folder in the plugin zip file by default 1. The plugin-zip command will now create a root folder in the zip file matching the name of the plugin. This aligns with the documentation stating that "By default, it uses Plugin Handbook best practices to discover files." 2. `--zip-root-folder` argument from my previous commit is now named `--root-folder` 3. The --root-folder argument will allow specifying a custom folder, or no folder for backwards compatibility. Example usage: {{{ npm run plugin-zip --root-folder='foo' }}} Follow-up to [61375], [60481]. Props justlevine, gziolo. * Add CHANGELOG entry * Update CHANGELOG.md * Scripts: Modify plugin-zip to add no-root-folder and root-folder options Require an argument for root-folder. Add option no-root-folder to zip files without a root folder for backwards compatibility. Update documentation, and changelog. (WordPress#60481) * Scripts: adjust message for plugin-zip command Apply suggestions from code review. props: @gziolo Co-authored-by: Greg Ziółkowski <[email protected]> * Update packages/scripts/README.md Co-authored-by: Aki Hamano <[email protected]> --------- Co-authored-by: Greg Ziółkowski <[email protected]> Co-authored-by: Aki Hamano <[email protected]> Co-authored-by: nicolasgalvez <[email protected]> Co-authored-by: gziolo <[email protected]> Co-authored-by: justlevine <[email protected]> Co-authored-by: t-hamano <[email protected]>
What?
Fixes #60481.
Allows adding a root folder to the zip file generated by plugin-zip
Why?
This allows creating a zip file that can be used to update a plugin via the WP Admin update page, since the plugin updater expects the zip file to contain a folder with the name of the plugin in the root of the zip file.
Please refer to the issue I opened here: #60481
How?
Add an argument to the plugin-zip command that will allow a developer to specify the root folder inside the zip file. Modifies zip creation to use this argument if specified. Defaults to the plugin name if no string was passed along with the argument.
Testing Instructions
Here's what I did to test:
npm linkcd ..npx @wordpress/create-block testcd testnpm link gutenbergnpm run build../gutenberg/node_modules/.bin/wp-scripts plugin-zipArchive: test.zip
Test --zip-root-folder argument
Expected: should use the name of the plugin as the root folder.
../gutenberg/node_modules/.bin/wp-scripts plugin-zip --zip-root-folderunzip -l test.zipshould show you atestfolder at the root of the zip:Test with --zip-root-folder argument with a value
Expected: should use a custom name for the root folder. Zip file name will remain unchanged.
../gutenberg/node_modules/.bin/wp-scripts plugin-zip --zip-root-folder=foounzip -l test.zipshould show you afoofolder at the root of the zip:Test files array in package.json still works
../gutenberg/node_modules/.bin/wp-scripts plugin-zip --zip-root-folder=foounzip -l test.zipshould show you afoofolder at the root of the zip:, and the additional file(s) you add should be in the archive.Thanks for reading! I can't wait to get some feedback. Please let me know if there is anything I can improve on.