Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add zip-plugin-builds command #1228

Merged
merged 8 commits into from
May 22, 2024
Merged

Add zip-plugin-builds command #1228

merged 8 commits into from
May 22, 2024

Conversation

westonruter
Copy link
Member

This is a sub-PR of #1227. Merge it before merging this PR.

In order to facilitate testing of pre-release builds, it is very helpful to have ZIP archives of the plugins to manually install on a WordPress site. This PR adds a npm run zip-plugin-builds command which is run automatically as part of npm run build-plugins. The ZIP files are located in the build directory alongside the built plugin directories.

@westonruter westonruter added Infrastructure Issues for the overall performance plugin infrastructure no milestone PRs that do not have a defined milestone for release skip changelog PRs that should not be mentioned in changelogs labels May 18, 2024
Copy link

github-actions bot commented May 18, 2024

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 props-bot label.

If you're merging code through a pull request on GitHub, copy and paste the following into the bottom of the merge commit message.

Co-authored-by: westonruter <westonruter@git.wordpress.org>
Co-authored-by: thelovekesh <thelovekesh@git.wordpress.org>

To understand the WordPress project's expectations around crediting contributors, please review the Contributor Attribution page in the Core Handbook.

@westonruter westonruter added [Type] Enhancement A suggestion for improvement of an existing feature and removed Infrastructure Issues for the overall performance plugin infrastructure labels May 18, 2024
@westonruter westonruter mentioned this pull request May 18, 2024
7 tasks
@thelovekesh
Copy link
Member

thelovekesh commented May 20, 2024

Should this process be a part of webpack?

[example diff]
diff --git a/bin/webpack/utils.js b/bin/webpack/utils.js
index d0a909df..629dbdfa 100644
--- a/bin/webpack/utils.js
+++ b/bin/webpack/utils.js
@@ -1,5 +1,6 @@
 const fs = require( 'fs' );
 const path = require( 'path' );
+const { execSync } = require( 'child_process' );
 
 /**
  * Return plugin root path.
@@ -97,10 +98,24 @@ const assetDataTransformer = ( content, absoluteFrom ) => {
 	return `<?php return array('dependencies' => array(), 'version' => '${ version }');`;
 };
 
+/**
+ * Create plugins zip file using `zip` command.
+ *
+ * @param {string} pluginPath The path where the plugin build is located.
+ * @param {string} pluginName The name of the plugin.
+ *
+ * @return {void}
+ */
+const createPluginZip = ( pluginPath, pluginName ) => {
+	const command = `cd ${ pluginPath } && zip -r ${ pluginName }.zip ${ pluginName }`;
+	execSync( command );
+};
+
 module.exports = {
 	getPluginRootPath,
 	deleteFileOrDirectory,
 	getPluginVersion,
 	generateBuildManifest,
 	assetDataTransformer,
+	createPluginZip,
 };
diff --git a/package.json b/package.json
index c5c9405c..92e8c338 100644
--- a/package.json
+++ b/package.json
@@ -30,15 +30,15 @@
     "readme": "./bin/plugin/cli.js readme",
     "versions": "./bin/plugin/cli.js versions",
     "build": "wp-scripts build",
-    "build-plugins": "npm-run-all 'build:plugin:!(performance-lab)' 'zip-plugin-builds'",
-    "build:plugin:performance-lab": "rm -rf build/performance-lab && mkdir -p build/performance-lab && git archive HEAD | tar -x -C build/performance-lab",
+    "build-plugins": "npm-run-all 'build:plugin:!(performance-lab)'",
+    "build:plugin:performance-lab": "rm -rf build/performance-lab && mkdir -p build/performance-lab && git archive HEAD | tar -x -C build/performance-lab && cd build && zip -qr performance-lab.zip performance-lab",
     "build:plugin:auto-sizes": "webpack --mode production --env plugin=auto-sizes",
     "build:plugin:dominant-color-images": "webpack --mode production --env plugin=dominant-color-images",
     "build:plugin:embed-optimizer": "webpack --mode production --env plugin=embed-optimizer",
     "build:plugin:optimization-detective": "webpack --mode production --env plugin=optimization-detective",
     "build:plugin:speculation-rules": "webpack --mode production --env plugin=speculation-rules",
     "build:plugin:webp-uploads": "webpack --mode production --env plugin=webp-uploads",
-    "zip-plugin-builds": "bin/zip-plugin-builds.sh",
+    "zip-plugin-builds": "npm-run-all 'build:plugin:!(performance-lab) --env zip=true' 'build:plugin:performance-lab'",
     "generate-pending-release-diffs": "bin/generate-pending-release-diffs.sh",
     "format-js": "wp-scripts format",
     "lint-js": "wp-scripts lint-js",
diff --git a/webpack.config.js b/webpack.config.js
index 2de1217b..a2c6cc79 100644
--- a/webpack.config.js
+++ b/webpack.config.js
@@ -10,6 +10,7 @@ const CopyWebpackPlugin = require( 'copy-webpack-plugin' );
  */
 const { plugins: standalonePlugins } = require( './plugins.json' );
 const {
+	createPluginZip,
 	assetDataTransformer,
 	deleteFileOrDirectory,
 	generateBuildManifest,
@@ -102,7 +103,8 @@ const buildPlugin = ( env ) => {
 		};
 	}
 
-	const to = path.resolve( __dirname, 'build', env.plugin );
+	const buildDir = path.resolve( __dirname, 'build' );
+	const to = path.resolve( buildDir, env.plugin );
 	const from = path.resolve( __dirname, 'plugins', env.plugin );
 	const dependencies = pluginsWithBuild.includes( env.plugin )
 		? [ `${ env.plugin }` ]
@@ -138,6 +140,11 @@ const buildPlugin = ( env ) => {
 					// After emit, generate build manifest.
 					compiler.hooks.afterEmit.tap( 'AfterEmitPlugin', () => {
 						generateBuildManifest( env.plugin, from );
+
+						// If zip flag is passed, create a zip file.
+						if ( env.zip ) {
+							createPluginZip( buildDir, env.plugin );
+						}
 					} );
 				},
 			},

@westonruter
Copy link
Member Author

@thelovekesh I guess so. It's just I know Bash, not Webpack.

@thelovekesh
Copy link
Member

I suggest incorporating this so that building the zip file becomes part of the asset-building lifecycle itself. And the above diff should work as expected.

@westonruter
Copy link
Member Author

Feel free to amend my PR with a revert of my Bash script and inclusion your patch

@westonruter westonruter added this to the performance-lab n.e.x.t milestone May 20, 2024
bin/webpack/utils.js Fixed Show fixed Hide fixed
/.github @mukeshpanchal27 @felixarntz @thelovekesh
/bin @mukeshpanchal27 @felixarntz @thelovekesh
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added @thelovekesh to bin codeowners.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Who?

Comment on lines +5 to +11
# Performance Lab Plugin Admin
/includes/admin @mukeshpanchal27 @felixarntz
/tests/includes/admin @mukeshpanchal27 @felixarntz

# Server Timing API
/includes/server-timing @felixarntz
/tests/includes/server-timing @felixarntz
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fixed paths and include server timing API files.

Comment on lines +24 to +29
const defaultBuildConfig = {
entry: {},
output: {
path: path.resolve( __dirname, 'build' ),
},
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

During each build step, Webpack creates an empty dist directory when output: {} is specified. This update prevents that and also addresses code duplication in several places below.

package.json Outdated
@@ -31,7 +31,8 @@
"versions": "./bin/plugin/cli.js versions",
"build": "wp-scripts build",
"build-plugins": "npm-run-all 'build:plugin:!(performance-lab)'",
"build:plugin:performance-lab": "rm -rf build/performance-lab && mkdir -p build/performance-lab && git archive HEAD | tar -x -C build/performance-lab",
"build-plugins:zip": "npm-run-all 'build:plugin:!(performance-lab) --env zip=true' 'build:plugin:performance-lab'",
"build:plugin:performance-lab": "rm -rf build/performance-lab && mkdir -p build/performance-lab && git archive HEAD | tar -x -C build/performance-lab && cd build && zip -qr performance-lab.zip performance-lab",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TODO: Remove it once #1182 is merged.

@thelovekesh
Copy link
Member

@westonruter Can you please test it once?

@westonruter
Copy link
Member Author

@thelovekesh Looks good! Tested and it works well.

@westonruter
Copy link
Member Author

@thelovekesh I guess you'll have to approve this pull request since I'm the one who opened it originally. But before we merge, we'll need to merge #1227 first.

Base automatically changed from add/generate-pending-release-diffs-command to trunk May 22, 2024 19:53
… add/zip-builds-command

* 'trunk' of https://github.com/WordPress/performance: (52 commits)
  Remove obsolete performance-lab special cases
  Update test command in php-test-plugins workflow
  Update PHP tests commands
  Add github-linguist docs for reference
  Remove obsolete condition in since command
  Remove redundant export-ignores
  Update monorepo notices
  Update lint:all script
  Update ruleset name
  Fix cwd in wp-env tests
  Mount plugin root files in tests
  Remove edge case of PL plugin to be refered as the root plugin
  Set default testsuite
  Add perflab load.php in paths to get loaded early
  Move perflab main plugin file tests to its tests dir
  Move test data images from modules to test/data
  Move perflab tests to includes dir; Move all site health tests under perflab tests
  Fix type hints
  Update testsuite config for performance-lab plugin
  Move perflab tests to tests/plugins/performance-lab
  ...
@westonruter
Copy link
Member Author

westonruter commented May 22, 2024

@thelovekesh I got the PHPStan error when attempting to commit the merge conflict:

Script phpstan analyse --memory-limit=2048M handling the phpstan event returned with error code 1
 ------ ---------------------------------------------------------------------- 
  Line   tests/plugins/performance-lab/load-tests.php                          
 ------ ---------------------------------------------------------------------- 
  35     Call to method PHPUnit\Framework\Assert::assertFalse() with int will  
         always evaluate to false.                                             
  50     Call to method PHPUnit\Framework\Assert::assertFalse() with int will  
         always evaluate to false.                                             
  73     Call to method PHPUnit\Framework\Assert::assertFalse() with int will  
         always evaluate to false.                                             
  144    Call to method PHPUnit\Framework\Assert::assertFalse() with int will  
         always evaluate to false.                                             
 ------ ---------------------------------------------------------------------- 

It may be due to the usage with lint-staged in which it is passing individual files to check rather than the entire repo, because once I forced the commit and ran npm run phpstan, then there was no error.

I can reproduce the issue with the same command that lint-staged invoked:

npm run phpstan -- performance.php plugins/dominant-color-images/class-dominant-color-image-editor-imagick.php plugins/optimization-detective/optimization.php plugins/performance-lab/includes/admin/load.php plugins/performance-lab/includes/admin/plugins.php plugins/performance-lab/includes/admin/server-timing.php plugins/performance-lab/includes/server-timing/class-perflab-server-timing-metric.php plugins/performance-lab/includes/server-timing/class-perflab-server-timing.php plugins/performance-lab/includes/server-timing/defaults.php plugins/performance-lab/includes/server-timing/hooks.php plugins/performance-lab/includes/server-timing/load.php plugins/performance-lab/includes/server-timing/object-cache.copy.php plugins/performance-lab/includes/site-health/audit-autoloaded-options/helper.php plugins/performance-lab/includes/site-health/audit-autoloaded-options/hooks.php plugins/performance-lab/includes/site-health/audit-enqueued-assets/helper.php plugins/performance-lab/includes/site-health/audit-enqueued-assets/hooks.php plugins/performance-lab/includes/site-health/avif-support/helper.php plugins/performance-lab/includes/site-health/avif-support/hooks.php plugins/performance-lab/includes/site-health/load.php plugins/performance-lab/includes/site-health/webp-support/helper.php plugins/performance-lab/includes/site-health/webp-support/hooks.php plugins/performance-lab/load.php plugins/speculation-rules/helper.php plugins/speculation-rules/load.php plugins/webp-uploads/deprecated.php plugins/webp-uploads/settings.php tests/bootstrap.php tests/plugins/auto-sizes/auto-sizes-test.php tests/plugins/performance-lab/includes/admin/load-tests.php tests/plugins/performance-lab/includes/admin/server-timing-tests.php tests/plugins/performance-lab/includes/server-timing/load-tests.php tests/plugins/performance-lab/includes/server-timing/perflab-server-timing-metric-tests.php tests/plugins/performance-lab/includes/server-timing/perflab-server-timing-tests.php tests/plugins/performance-lab/includes/site-health/audit-autoloaded-options/audit-autoloaded-options-test.php tests/plugins/performance-lab/includes/site-health/audit-enqueued-assets/audit-enqueued-assets-helper-test.php tests/plugins/performance-lab/includes/site-health/audit-enqueued-assets/audit-enqueued-assets-test.php tests/plugins/performance-lab/includes/site-health/audit-enqueued-assets/class-audit-assets-transients-set.php tests/plugins/performance-lab/includes/site-health/audit-enqueued-assets/class-site-health-mock-responses.php tests/plugins/performance-lab/load-tests.php tests/plugins/webp-uploads/helper-tests.php tests/plugins/webp-uploads/image-edit-tests.php tests/plugins/webp-uploads/load-tests.php tests/plugins/webp-uploads/rest-api-tests.php tests/utils/TestCase/DominantColorTestCase.php

The issue I think is that we shouldn't be running lint-staged on individual paths. We avoided doing this for the AMP plugin in the .lint-staged.js as follows:

'*.php': () => 'composer analyze',

I think we can do the same once we finally pick up the lint-staged work again in #1090.

@westonruter westonruter merged commit 1493572 into trunk May 22, 2024
15 checks passed
@westonruter westonruter deleted the add/zip-builds-command branch May 22, 2024 20:04
@thelovekesh
Copy link
Member

Makes sense. Additionally, PHPStan recommends running the analysis on the entire codebase rather than on specific paths. This is because changes in one file might affect other parts of the project, potentially hiding errors if only a subset of files is analyzed.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
no milestone PRs that do not have a defined milestone for release skip changelog PRs that should not be mentioned in changelogs [Type] Enhancement A suggestion for improvement of an existing feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants