From 338594aeefa9ec15f964ec2fb036cb8e544992b6 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Tue, 8 Mar 2022 14:31:27 -0800 Subject: [PATCH 1/5] Replace plugin activation hook with module settings default to be reliable on environments like multisite as well. --- admin/load.php | 24 ------------------------ default-enabled-modules.php | 8 ++++++++ load.php | 15 +++++++++++++-- 3 files changed, 21 insertions(+), 26 deletions(-) create mode 100644 default-enabled-modules.php diff --git a/admin/load.php b/admin/load.php index 8549efe210..dfda69f04a 100644 --- a/admin/load.php +++ b/admin/load.php @@ -439,27 +439,3 @@ function perflab_plugin_action_links_add_settings( $links ) { return $links; } - -/** - * Enables all non-experimental modules on plugin activation. - * - * @since 1.0.0 - */ -function perflab_activation_hook() { - // Bail if option is already set with any value. - if ( false !== get_option( PERFLAB_MODULES_SETTING, false ) ) { - return; - } - - $modules = perflab_get_modules(); - $modules_settings = perflab_get_module_settings(); - - foreach ( $modules as $module_name => $module_data ) { - if ( ! $module_data['experimental'] ) { - $modules_settings[ $module_name ] = array( 'enabled' => true ); - } - } - - update_option( PERFLAB_MODULES_SETTING, $modules_settings ); -} -register_activation_hook( PERFLAB_MAIN_FILE, 'perflab_activation_hook' ); diff --git a/default-enabled-modules.php b/default-enabled-modules.php new file mode 100644 index 0000000000..fb2e6094a7 --- /dev/null +++ b/default-enabled-modules.php @@ -0,0 +1,8 @@ + true ); + return $module_settings; + }, + array() + ); + register_setting( PERFLAB_MODULES_SCREEN, PERFLAB_MODULES_SETTING, array( 'type' => 'object', 'sanitize_callback' => 'perflab_sanitize_modules_setting', - 'default' => array(), + 'default' => $default_option, ) ); } @@ -76,7 +87,7 @@ function( $module_settings ) { * @return array Associative array of module settings keyed by module slug. */ function perflab_get_module_settings() { - return (array) get_option( PERFLAB_MODULES_SETTING, array() ); + return (array) get_option( PERFLAB_MODULES_SETTING ); } /** From ebb5ba01d100c8e9e1328ec284165bf4755838d6 Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Tue, 8 Mar 2022 14:35:49 -0800 Subject: [PATCH 2/5] Implement script to automatically generate and update default-enabled-modules.php file. --- bin/plugin/cli.js | 12 +++ bin/plugin/commands/common.js | 34 +++++-- bin/plugin/commands/enabled-modules.js | 121 +++++++++++++++++++++++++ bin/plugin/commands/readme.js | 8 +- bin/plugin/commands/translations.js | 8 +- package.json | 1 + 6 files changed, 167 insertions(+), 17 deletions(-) create mode 100644 bin/plugin/commands/enabled-modules.js diff --git a/bin/plugin/cli.js b/bin/plugin/cli.js index 706168a399..7566b506ef 100755 --- a/bin/plugin/cli.js +++ b/bin/plugin/cli.js @@ -38,6 +38,10 @@ const { handler: translationsHandler, options: translationsOptions, } = require( './commands/translations' ); +const { + handler: enabledModulesHandler, + options: enabledModulesOptions, +} = require( './commands/enabled-modules' ); const { handler: sinceHandler, options: sinceOptions, @@ -65,4 +69,12 @@ withOptions( program.command( 'module-translations' ), translationsOptions ) ) .action( catchException( translationsHandler ) ); +withOptions( + program.command( 'default-enabled-modules' ), + enabledModulesOptions +) + .alias( 'enabled-modules' ) + .description( 'Generates a PHP file with non-experimental module slugs' ) + .action( catchException( enabledModulesHandler ) ); + program.parse( process.argv ); diff --git a/bin/plugin/commands/common.js b/bin/plugin/commands/common.js index 0ece4a595e..fc2d857695 100644 --- a/bin/plugin/commands/common.js +++ b/bin/plugin/commands/common.js @@ -6,32 +6,43 @@ const glob = require( 'fast-glob' ); const fs = require( 'fs' ); /** - * @typedef WPModuleDescription + * @typedef WPModuleData * - * @property {string} name Module name. - * @property {string} description Module description. + * @property {string} slug Module slug. + * @property {string} focus Module focus. + * @property {string} name Module name. + * @property {string} description Module description. + * @property {boolean} experimental Whether the module is experimental. */ /** - * Returns a promise resolving to the module description list string for the `readme.txt` file. + * Returns a promise resolving to the list of data for all modules. * * @param {string} modulesDir Modules directory. * - * @return {Promise<[]WPModuleDescription>} Promise resolving to module description list. + * @return {Promise<[]WPModuleData>} Promise resolving to module data list. */ -exports.getModuleDescriptions = async ( modulesDir ) => { +exports.getModuleData = async ( modulesDir ) => { const moduleFilePattern = path.join( modulesDir, '*/*/load.php' ); const moduleFiles = await glob( path.resolve( '.', moduleFilePattern ) ); return moduleFiles .map( ( moduleFile ) => { + // Populate slug and focus based on file path. + const moduleDir = path.dirname( moduleFile ); + const moduleData = { + slug: path.basename( moduleDir ), + focus: path.basename( path.dirname( moduleDir ) ), + }; + // Map of module header => object property. const headers = { 'Module Name': 'name', Description: 'description', + Experimental: 'experimental', }; - const moduleData = {}; + // Populate name, description and experimental based on module file headers. const fileContent = fs.readFileSync( moduleFile, 'utf8' ); const regex = new RegExp( `^(?:[ \t]* { match = regex.exec( fileContent ); } + // Parse experimental field into a boolean. + if ( typeof moduleData.experimental === 'string' ) { + if ( moduleData.experimental.toLowerCase() === 'yes' ) { + moduleData.experimental = true; + } else { + moduleData.experimental = false; + } + } + return moduleData; } ) .filter( ( moduleData ) => moduleData.name && moduleData.description ); diff --git a/bin/plugin/commands/enabled-modules.js b/bin/plugin/commands/enabled-modules.js new file mode 100644 index 0000000000..bd85c7b38b --- /dev/null +++ b/bin/plugin/commands/enabled-modules.js @@ -0,0 +1,121 @@ +/** + * External dependencies + */ +const path = require( 'path' ); +const fs = require( 'fs' ); +const { EOL } = require( 'os' ); + +/** + * Internal dependencies + */ +const { log, formats } = require( '../lib/logger' ); +const { getModuleData } = require( './common' ); + +const TAB = '\t'; +const NEWLINE = EOL; +const FILE_HEADER = `', + description: 'Modules directory', + }, + { + argname: '-d, --output ', + description: 'Output file', + }, +]; + +/** + * Command that generates a PHP file with non-experimental module slugs. + * + * @param {WPEnabledModulesCommandOptions} opt + */ +exports.handler = async ( opt ) => { + await createEnabledModules( { + directory: opt.directory || 'modules', + output: opt.output || 'default-enabled-modules.php', + } ); +}; + +/** + * Gathers the non-experimental modules as the default enabled modules. + * + * @param {WPEnabledModulesSettings} settings Default enabled modules settings. + * + * @return {[]string} List of default enabled module paths relative to modules directory. + */ +async function getDefaultEnabledModules( settings ) { + const modulesData = await getModuleData( settings.directory ); + return modulesData + .filter( ( moduleData ) => ! moduleData.experimental ) + .map( ( moduleData ) => `${ moduleData.focus }/${ moduleData.slug }` ); +} + +/** + * Creates PHP file with the given default enabled modules. + * + * @param {[]string} enabledModules List of default enabled module paths relative to modules directory. + * @param {WPEnabledModulesSettings} settings Default enabled modules settings. + */ +function createEnabledModulesPHPFile( enabledModules, settings ) { + const output = enabledModules.map( ( enabledModule ) => { + // Escape single quotes. + return `${ TAB }'${ enabledModule.replace( /'/g, "\\'" ) }',`; + } ); + + const fileOutput = `${ FILE_HEADER }${ output.join( + NEWLINE + ) }${ FILE_FOOTER }`; + fs.writeFileSync( path.join( '.', settings.output ), fileOutput ); +} + +/** + * Gathers non-experimental modules and generates a PHP file with them. + * + * @param {WPEnabledModulesSettings} settings Default enabled modules settings. + */ +async function createEnabledModules( settings ) { + log( + formats.title( + `\n💃Gathering non-experimental modules for "${ settings.directory }" in "${ settings.output }"\n\n` + ) + ); + + try { + const enabledModules = await getDefaultEnabledModules( settings ); + createEnabledModulesPHPFile( enabledModules, settings ); + } catch ( error ) { + if ( error instanceof Error ) { + log( formats.error( error.stack ) ); + return; + } + } + + log( + formats.success( + `\n💃Non-experimental modules successfully set in "${ settings.output }"\n\n` + ) + ); +} diff --git a/bin/plugin/commands/readme.js b/bin/plugin/commands/readme.js index f95b73d9bc..bc48ce6d20 100644 --- a/bin/plugin/commands/readme.js +++ b/bin/plugin/commands/readme.js @@ -9,7 +9,7 @@ const fs = require( 'fs' ); */ const { log, formats } = require( '../lib/logger' ); const config = require( '../config' ); -const { getModuleDescriptions } = require( './common' ); +const { getModuleData } = require( './common' ); const { getChangelog } = require( './changelog' ); /** @@ -68,11 +68,9 @@ exports.handler = async ( opt ) => { * @return {Promise} Promise resolving to module description list in markdown, with trailing newline. */ async function getModuleDescriptionList( settings ) { - const moduleDescriptions = await getModuleDescriptions( - settings.directory - ); + const modulesData = await getModuleData( settings.directory ); - return moduleDescriptions + return modulesData .map( ( moduleData ) => `* **${ moduleData.name }:** ${ moduleData.description }` diff --git a/bin/plugin/commands/translations.js b/bin/plugin/commands/translations.js index 6a63e1eeaa..cca33749c5 100644 --- a/bin/plugin/commands/translations.js +++ b/bin/plugin/commands/translations.js @@ -10,7 +10,7 @@ const { EOL } = require( 'os' ); */ const { log, formats } = require( '../lib/logger' ); const config = require( '../config' ); -const { getModuleDescriptions } = require( './common' ); +const { getModuleData } = require( './common' ); const TAB = '\t'; const NEWLINE = EOL; @@ -77,10 +77,8 @@ exports.handler = async ( opt ) => { * @return {[]WPTranslationEntry} List of translation entries. */ async function getTranslations( settings ) { - const moduleDescriptions = await getModuleDescriptions( - settings.directory - ); - const moduleTranslations = moduleDescriptions.map( ( moduleData ) => { + const modulesData = await getModuleData( settings.directory ); + const moduleTranslations = modulesData.map( ( moduleData ) => { return [ { text: moduleData.name, diff --git a/package.json b/package.json index be2e620486..1be0ba45c6 100644 --- a/package.json +++ b/package.json @@ -32,6 +32,7 @@ "since": "./bin/plugin/cli.js since", "readme": "./bin/plugin/cli.js readme", "translations": "./bin/plugin/cli.js translations", + "enabled-modules": "./bin/plugin/cli.js enabled-modules", "format-js": "wp-scripts format ./bin", "lint-js": "wp-scripts lint-js ./bin", "format-php": "wp-env run composer run-script format", From 5c398d3ce5b3a9d7399d4d001f477eef45f8645a Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Tue, 8 Mar 2022 14:36:14 -0800 Subject: [PATCH 3/5] Expand release instructions to include running command to refresh default enabled modules. --- docs/Releasing-the-plugin.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/docs/Releasing-the-plugin.md b/docs/Releasing-the-plugin.md index 17d54cc5a7..35486d0c49 100644 --- a/docs/Releasing-the-plugin.md +++ b/docs/Releasing-the-plugin.md @@ -20,6 +20,10 @@ The version number needs to be updated in the following files: In addition to those locations, run the `npm run since -- -r {version}` command to replace any occurrence of `@since n.e.x.t` with the version number. This ensures any code annotated with the "next" release will now have its proper version number on it. The only exception to this are pre-releases, such as a beta or RC: For those, the stable version number should be used. For example, if the milestone is `1.2.0-beta.2`, the version in e.g. `@since` annotations in the codebase should still be `1.2.0`. +### Update list of default enabled modules + +The default enabled modules are defined in a separate `default-enabled-modules.php` file which can be automatically generated and updated. Run `npm run enabled-modules` to update the file based on the latest available non-experimental modules. + ### Update translation strings The module headers from the plugin have to be translated using a separate `module-i18n.php` file which can be automatically generated and updated. Run `npm run translations` to update the file to reflect the latest available modules. From df55cd76877dd52edc4b21f4c1c11dbfdf918cdd Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Tue, 8 Mar 2022 14:53:54 -0800 Subject: [PATCH 4/5] Fix relevant load.php tests by updating expected default option. --- tests/load-tests.php | 29 +++++++++++++++++++++++++---- 1 file changed, 25 insertions(+), 4 deletions(-) diff --git a/tests/load-tests.php b/tests/load-tests.php index 30faa6a1de..db3388363c 100644 --- a/tests/load-tests.php +++ b/tests/load-tests.php @@ -28,7 +28,7 @@ public function test_perflab_register_modules_setting() { } // Assert that registered default works correctly. - $this->assertSame( array(), get_option( PERFLAB_MODULES_SETTING ) ); + $this->assertSame( $this->get_expected_default_option(), get_option( PERFLAB_MODULES_SETTING ) ); // Assert that most basic sanitization works correctly (an array is required). update_option( PERFLAB_MODULES_SETTING, 'invalid' ); @@ -59,7 +59,7 @@ public function test_perflab_sanitize_modules_setting() { public function test_perflab_get_module_settings() { // Assert that by default the settings are an empty array. $settings = perflab_get_module_settings(); - $this->assertSame( array(), $settings ); + $this->assertSame( $this->get_expected_default_option(), $settings ); // Assert that option updates are reflected in the settings correctly. $new_value = array( 'my-module' => array( 'enabled' => true ) ); @@ -70,8 +70,16 @@ public function test_perflab_get_module_settings() { public function test_perflab_get_active_modules() { // Assert that by default there are no active modules. - $active_modules = perflab_get_active_modules(); - $this->assertSame( array(), $active_modules ); + $active_modules = perflab_get_active_modules(); + $expected_active_modules = array_keys( + array_filter( + $this->get_expected_default_option(), + function( $module_settings ) { + return $module_settings['enabled']; + } + ) + ); + $this->assertSame( $expected_active_modules, $active_modules ); // Assert that option updates affect the active modules correctly. $new_value = array( @@ -82,4 +90,17 @@ public function test_perflab_get_active_modules() { $active_modules = perflab_get_active_modules(); $this->assertSame( array( 'active-module' ), $active_modules ); } + + private function get_expected_default_option() { + // This code is essentially copied over from the perflab_register_modules_setting() function. + $default_enabled_modules = require plugin_dir_path( PERFLAB_MAIN_FILE ) . 'default-enabled-modules.php'; + return array_reduce( + $default_enabled_modules, + function( $module_settings, $module_dir ) { + $module_settings[ $module_dir ] = array( 'enabled' => true ); + return $module_settings; + }, + array() + ); + } } From 8dce1e9b7d0e8bd6e04feadf8ab5ce7698299f5a Mon Sep 17 00:00:00 2001 From: Felix Arntz Date: Wed, 9 Mar 2022 15:10:07 -0800 Subject: [PATCH 5/5] Simplify code by removing if clause. --- bin/plugin/commands/common.js | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/bin/plugin/commands/common.js b/bin/plugin/commands/common.js index fc2d857695..83e7012144 100644 --- a/bin/plugin/commands/common.js +++ b/bin/plugin/commands/common.js @@ -62,11 +62,8 @@ exports.getModuleData = async ( modulesDir ) => { // Parse experimental field into a boolean. if ( typeof moduleData.experimental === 'string' ) { - if ( moduleData.experimental.toLowerCase() === 'yes' ) { - moduleData.experimental = true; - } else { - moduleData.experimental = false; - } + moduleData.experimental = + moduleData.experimental.toLowerCase() === 'yes'; } return moduleData;