From c992b2fa7c0f2c2f8daface4d846439e83fc175e Mon Sep 17 00:00:00 2001 From: Timmy Crawford Date: Mon, 6 Jul 2020 10:39:58 -0700 Subject: [PATCH] Build: Don't Attempt to Load non-minified Assets in Core Build (https://github.com/woocommerce/woocommerce-admin/pull/4747) * Build: Only load non-minified js assets when they are available in the build. * Avoid multiple Loader instances. * Fix feature filter name in test bootstrap. * Fix feature filter used in Loader tests. Co-authored-by: Jeff Stieler --- plugins/woocommerce-admin/config/core.json | 1 + .../woocommerce-admin/config/development.json | 1 + plugins/woocommerce-admin/config/plugin.json | 1 + .../woocommerce-admin/src/FeaturePlugin.php | 2 +- plugins/woocommerce-admin/src/Loader.php | 19 ++++- plugins/woocommerce-admin/tests/bootstrap.php | 8 +- plugins/woocommerce-admin/tests/loader.php | 81 +++++++++++++++++++ 7 files changed, 108 insertions(+), 5 deletions(-) create mode 100644 plugins/woocommerce-admin/tests/loader.php diff --git a/plugins/woocommerce-admin/config/core.json b/plugins/woocommerce-admin/config/core.json index 983ca5c8100..074cca192f8 100644 --- a/plugins/woocommerce-admin/config/core.json +++ b/plugins/woocommerce-admin/config/core.json @@ -11,6 +11,7 @@ "remote-inbox-notifications": false, "shipping-label-banner": true, "store-alerts": true, + "unminified-js": false, "wcpay": true, "homescreen": true } diff --git a/plugins/woocommerce-admin/config/development.json b/plugins/woocommerce-admin/config/development.json index b7840f23cc2..9b97ab2f0f6 100644 --- a/plugins/woocommerce-admin/config/development.json +++ b/plugins/woocommerce-admin/config/development.json @@ -11,6 +11,7 @@ "remote-inbox-notifications": true, "shipping-label-banner": true, "store-alerts": true, + "unminified-js": true, "wcpay": true, "homescreen": true } diff --git a/plugins/woocommerce-admin/config/plugin.json b/plugins/woocommerce-admin/config/plugin.json index 1e36536e96e..4b722826f60 100644 --- a/plugins/woocommerce-admin/config/plugin.json +++ b/plugins/woocommerce-admin/config/plugin.json @@ -11,6 +11,7 @@ "remote-inbox-notifications": false, "shipping-label-banner": true, "store-alerts": true, + "unminified-js": true, "wcpay": true, "homescreen": true } diff --git a/plugins/woocommerce-admin/src/FeaturePlugin.php b/plugins/woocommerce-admin/src/FeaturePlugin.php index e32e8784df2..6f146929dd5 100644 --- a/plugins/woocommerce-admin/src/FeaturePlugin.php +++ b/plugins/woocommerce-admin/src/FeaturePlugin.php @@ -204,7 +204,7 @@ class FeaturePlugin { add_filter( 'woocommerce_admin_features', array( $this, 'replace_supported_features' ), 0 ); add_action( 'admin_menu', array( $this, 'register_devdocs_page' ) ); - new Loader(); + Loader::get_instance(); } /** diff --git a/plugins/woocommerce-admin/src/Loader.php b/plugins/woocommerce-admin/src/Loader.php index b569d620536..8ba5e34a341 100644 --- a/plugins/woocommerce-admin/src/Loader.php +++ b/plugins/woocommerce-admin/src/Loader.php @@ -192,6 +192,22 @@ class Loader { return false; } + /** + * Determines if a minified JS file should be served. + * + * @param boolean $script_debug Only serve unminified files if script debug is on. + * @return boolean If js asset should use minified version. + */ + public static function should_use_minified_js_file( $script_debug ) { + // un-minified files are only shipped in non-core versions of wc-admin, return true if unminified files are not available. + if ( ! self::is_feature_enabled( 'unminified-js' ) ) { + return true; + } + + // Otherwise we will serve un-minified files if SCRIPT_DEBUG is on, or if anything truthy is passed in-lieu of SCRIPT_DEBUG. + return ! $script_debug; + } + /** * Gets the URL to an asset file. * @@ -204,7 +220,8 @@ class Loader { // Potentially enqueue minified JavaScript. if ( 'js' === $ext ) { - $suffix = defined( 'SCRIPT_DEBUG' ) && SCRIPT_DEBUG ? '' : '.min'; + $script_debug = defined( 'SCRIPT_DEBUG' ) && SCRIPT_DEBUG; + $suffix = self::should_use_minified_js_file( $script_debug ) ? '.min' : ''; } return plugins_url( self::get_path( $ext ) . $file . $suffix . '.' . $ext, WC_ADMIN_PLUGIN_FILE ); diff --git a/plugins/woocommerce-admin/tests/bootstrap.php b/plugins/woocommerce-admin/tests/bootstrap.php index 13107ded540..a6ea59c3062 100755 --- a/plugins/woocommerce-admin/tests/bootstrap.php +++ b/plugins/woocommerce-admin/tests/bootstrap.php @@ -60,7 +60,7 @@ class WC_Admin_Unit_Tests_Bootstrap { tests_add_filter( 'setup_theme', array( $this, 'install_wc' ) ); // Set up WC-Admin config. - tests_add_filter( 'wc_admin_get_feature_config', array( $this, 'add_development_features' ) ); + tests_add_filter( 'woocommerce_admin_get_feature_config', array( $this, 'add_development_features' ) ); // load the WP testing environment. require_once $this->wp_tests_dir . '/includes/bootstrap.php'; @@ -152,10 +152,12 @@ class WC_Admin_Unit_Tests_Bootstrap { /** * Use the `development` features for testing. + * + * @param array $flags Existing feature flags. + * @return array Filtered feature flags. */ - public function add_development_features() { + public function add_development_features( $flags ) { $config = json_decode( file_get_contents( $this->plugin_dir . '/config/development.json' ) ); // @codingStandardsIgnoreLine. - $flags = array(); foreach ( $config->features as $feature => $bool ) { $flags[ $feature ] = $bool; } diff --git a/plugins/woocommerce-admin/tests/loader.php b/plugins/woocommerce-admin/tests/loader.php new file mode 100644 index 00000000000..8c782167646 --- /dev/null +++ b/plugins/woocommerce-admin/tests/loader.php @@ -0,0 +1,81 @@ +get_url( 'flavortown', 'js' ); + + // All we are concerned about in this test is the js filename. Pop it off the end of the asset url. + $parts = explode( '/', $result ); + $final_file_name = array_pop( $parts ); + + // Since this can vary depending on the env the tests are running in, we will make this assertion based upon the SCRIPT_DEBUG value. + $expected_value = defined( 'SCRIPT_DEBUG' ) && SCRIPT_DEBUG ? 'flavortown.js' : 'flavortown.min.js'; + + $this->assertEquals( + $expected_value, + $final_file_name, + 'the anticipated js file name should use .min when SCRIPT_DEBUG is off, and have no .min when SCRIPT_DEBUG is on.' + ); + } + + /** + * Tests for should_use_minified_js_file + */ + public function test_should_use_minified_js_file() { + $loader = Loader::get_instance(); + + // We will simulate a call with SCRIPT_DEBUG on. + $script_debug = true; + + $this->assertFalse( + $loader->should_use_minified_js_file( $script_debug ), + 'Since unminifed js feature is TRUE/on, and script_debug is true, should_use_minified_js_file should return false' + ); + + // Now we will simulate SCRIPT_DEBUG off/false. + $script_debug = false; + $this->assertTrue( + $loader->should_use_minified_js_file( $script_debug ), + 'Since unminifed js feature is TRUE/on, and script_debug is false, should_use_minified_js_file should return true' + ); + } +}