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 <jeff.m.stieler@gmail.com>

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
 	}

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
 	}

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
 	}

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();
 	}
 
 	/**

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 );

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;
 		}

plugins/woocommerce-admin/tests/loader.php

@@ -0,0 +1,81 @@
+<?php
+/**
+ * Loader tests
+ *
+ * @package WooCommerce Admin\Tests\Loader
+ */
+
+use \Automattic\WooCommerce\Admin\Loader;
+
+/**
+ * WC_Admin_Tests_Page_Loader Class
+ *
+ * @package WooCommerce Admin\Tests\Loader
+ */
+class WC_Admin_Tests_Loader extends WP_UnitTestCase {
+	/**
+	 * Setup
+	 */
+	public function setUp() {
+		add_filter( 'woocommerce_admin_features', array( $this, 'turn_on_unminified_js_feature' ), 20, 1 );
+	}
+
+	/**
+	 * Tear Down
+	 */
+	public function tearDown() {
+		remove_filter( 'woocommerce_admin_features', array( $this, 'turn_on_unminified_js_feature' ), 20 );
+	}
+
+	/**
+	 * Fitler to enable unminified-js feature.
+	 *
+	 * @param  array $features Array of active features.
+	 */
+	public static function turn_on_unminified_js_feature( $features ) {
+		return array_merge( $features, array( 'unminified-js' ) );
+	}
+
+	/**
+	 * Test get_url()
+	 */
+	public function test_get_url() {
+		$loader = Loader::get_instance();
+		$result = $loader->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'
+		);
+	}
+}