From 4e04c42493ca29269976e8cf2e1cbe6f18ac8d8f Mon Sep 17 00:00:00 2001 From: K Adam White Date: Tue, 21 Jul 2020 15:33:16 -0400 Subject: [PATCH 1/3] Use is_readable instead of file_exists --- inc/manifest.php | 6 +++--- inc/namespace.php | 8 ++++---- 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/inc/manifest.php b/inc/manifest.php index d42bbf1..b60d7b8 100644 --- a/inc/manifest.php +++ b/inc/manifest.php @@ -106,10 +106,10 @@ function get_version( string $asset_uri, string $manifest_path ) : ?string { if ( $manifest_hash ) { $manifest_hashes[ $manifest_path ] = $manifest_hash; return $manifest_hashes[ $manifest_path ]; + } else { + // Set "null" to prevent trying to re-hash after hashing has failed once. + $manifest_hashes[ $manifest_path ] = null; } - } else { - // Set "null" to prevent subsequent file_exists checks during this request. - $manifest_hashes[ $manifest_path ] = null; } // Finally, use the Altis deployment revision when available. This is a diff --git a/inc/namespace.php b/inc/namespace.php index ff2739f..180887e 100644 --- a/inc/namespace.php +++ b/inc/namespace.php @@ -185,7 +185,7 @@ function( $script_key ) use ( $target_bundle ) { $css_bundle = $build_path . preg_replace( '/\.js$/', '.css', $target_bundle ); // Production mode. Manually register script bundles. - if ( file_exists( $js_bundle ) ) { + if ( is_readable( $js_bundle ) ) { wp_register_script( $options['handle'], Paths\get_file_uri( $js_bundle ), @@ -196,7 +196,7 @@ function( $script_key ) use ( $target_bundle ) { $registered['scripts'][] = $options['handle']; } - if ( file_exists( $css_bundle ) ) { + if ( is_readable( $css_bundle ) ) { wp_register_style( $options['handle'], Paths\get_file_uri( $css_bundle ), @@ -338,8 +338,8 @@ function register_asset( string $manifest_path, string $target_asset, array $opt // If asset is not present in manifest, attempt to resolve the $target_asset // relative to the folder containing the manifest file. - if ( empty( $asset_uri ) && file_exists( $manifest_folder . $target_asset ) ) { - // TODO: Consider checking file_exists( $manifest_folder . $target_asset ) + if ( empty( $asset_uri ) && is_readable( $manifest_folder . $target_asset ) ) { + // TODO: Consider checking is_readable( $manifest_folder . $target_asset ) // and warning (in console or error log) if it is not present on disk. $asset_uri = $target_asset; } From 07ac1aa92c555c5885f2d3849153b4109e236acc Mon Sep 17 00:00:00 2001 From: K Adam White Date: Tue, 21 Jul 2020 15:41:03 -0400 Subject: [PATCH 2/3] Align array assignments in long test file --- tests/inc/class-test-asset-loader.php | 58 +++++++++++++-------------- 1 file changed, 29 insertions(+), 29 deletions(-) diff --git a/tests/inc/class-test-asset-loader.php b/tests/inc/class-test-asset-loader.php index 923c31a..f8a1bf7 100644 --- a/tests/inc/class-test-asset-loader.php +++ b/tests/inc/class-test-asset-loader.php @@ -115,16 +115,16 @@ public function provide_is_css_cases() { */ public function test_register_prod_script() : void { Asset_Loader\enqueue_asset( $this->prod_manifest, 'editor.js', [ - 'handle' => 'custom-handle', + 'handle' => 'custom-handle', 'dependencies' => [ 'wp-data' ], ] ); $this->assertEquals( [ - 'handle' => 'custom-handle', - 'src' => 'https://my.theme/uri/fixtures/editor.03bfa96fd1c694ca18b3.js', - 'deps' => [ 'wp-data' ], - 'ver' => null, + 'handle' => 'custom-handle', + 'src' => 'https://my.theme/uri/fixtures/editor.03bfa96fd1c694ca18b3.js', + 'deps' => [ 'wp-data' ], + 'ver' => null, ], $this->scripts->get_registered( 'custom-handle' ) ); @@ -135,16 +135,16 @@ public function test_register_prod_script() : void { */ public function test_enqueue_prod_script() : void { Asset_Loader\enqueue_asset( $this->prod_manifest, 'editor.js', [ - 'handle' => 'custom-handle', + 'handle' => 'custom-handle', 'dependencies' => [ 'wp-data' ], ] ); $this->assertEquals( [ - 'handle' => 'custom-handle', - 'src' => 'https://my.theme/uri/fixtures/editor.03bfa96fd1c694ca18b3.js', - 'deps' => [ 'wp-data' ], - 'ver' => null, + 'handle' => 'custom-handle', + 'src' => 'https://my.theme/uri/fixtures/editor.03bfa96fd1c694ca18b3.js', + 'deps' => [ 'wp-data' ], + 'ver' => null, ], $this->scripts->get_registered( 'custom-handle' ) ); @@ -157,16 +157,16 @@ public function test_register_css_asset_production() : void { $this->prod_manifest, 'frontend-styles.css', [ - 'handle' => 'frontend-styles', + 'handle' => 'frontend-styles', 'dependencies' => [ 'dependency-style' ], ] ); $this->assertEquals( [ 'handle' => 'frontend-styles', - 'src' => 'https://my.theme/uri/fixtures/frontend-styles.96a500e3dd1eb671f25e.css', - 'deps' => [ 'dependency-style' ], - 'ver' => null, + 'src' => 'https://my.theme/uri/fixtures/frontend-styles.96a500e3dd1eb671f25e.css', + 'deps' => [ 'dependency-style' ], + 'ver' => null, ], $this->styles->get_registered( 'frontend-styles' ) ); @@ -177,25 +177,25 @@ public function test_register_css_asset_dev() : void { $this->dev_manifest, 'frontend-styles.css', [ - 'handle' => 'frontend-styles', + 'handle' => 'frontend-styles', 'dependencies' => [ 'dependency-style' ], ] ); $this->assertEquals( [ 'handle' => 'frontend-styles', - 'src' => 'https://localhost:9090/build/frontend-styles.js', - 'deps' => [], - 'ver' => '499bb147f8e7234d957a47ac983e19e7', + 'src' => 'https://localhost:9090/build/frontend-styles.js', + 'deps' => [], + 'ver' => '499bb147f8e7234d957a47ac983e19e7', ], $this->scripts->get_registered( 'frontend-styles' ) ); $this->assertEquals( [ 'handle' => 'frontend-styles', - 'src' => false, - 'deps' => [ 'dependency-style' ], - 'ver' => '499bb147f8e7234d957a47ac983e19e7', + 'src' => false, + 'deps' => [ 'dependency-style' ], + 'ver' => '499bb147f8e7234d957a47ac983e19e7', ], $this->styles->get_registered( 'frontend-styles' ) ); @@ -206,7 +206,7 @@ public function test_register_css_asset_then_corresponding_js_asset_dev() : void $this->dev_manifest, 'editor.css', [ - 'handle' => 'editor', + 'handle' => 'editor', 'dependencies' => [ 'style-dependency' ], ] ); @@ -214,25 +214,25 @@ public function test_register_css_asset_then_corresponding_js_asset_dev() : void $this->dev_manifest, 'editor.js', [ - 'handle' => 'editor', + 'handle' => 'editor', 'dependencies' => [ 'script-dependency' ], ] ); $this->assertEquals( [ 'handle' => 'editor', - 'src' => 'https://localhost:9090/build/editor.js', - 'deps' => [ 'script-dependency' ], - 'ver' => '499bb147f8e7234d957a47ac983e19e7', + 'src' => 'https://localhost:9090/build/editor.js', + 'deps' => [ 'script-dependency' ], + 'ver' => '499bb147f8e7234d957a47ac983e19e7', ], $this->scripts->get_registered( 'editor' ) ); $this->assertEquals( [ 'handle' => 'editor', - 'src' => false, - 'deps' => [ 'style-dependency' ], - 'ver' => '499bb147f8e7234d957a47ac983e19e7', + 'src' => false, + 'deps' => [ 'style-dependency' ], + 'ver' => '499bb147f8e7234d957a47ac983e19e7', ], $this->styles->get_registered( 'editor' ) ); From 5b47abbf0e4319aa482793c211ad5afba48b4b05 Mon Sep 17 00:00:00 2001 From: K Adam White Date: Tue, 21 Jul 2020 18:51:48 -0400 Subject: [PATCH 3/3] Permit enqueueing css in dev environments --- inc/namespace.php | 47 +++-- tests/inc/class-test-asset-loader.php | 256 ++++++++++++++++++++++---- 2 files changed, 243 insertions(+), 60 deletions(-) diff --git a/inc/namespace.php b/inc/namespace.php index 180887e..fddff40 100644 --- a/inc/namespace.php +++ b/inc/namespace.php @@ -310,11 +310,12 @@ function _register_or_update_script( string $handle, string $asset_uri, array $d * @param string $manifest_path File system path for an asset manifest JSON file. * @param string $target_asset Asset to retrieve within the specified manifest. * @param array $options { - * @type string $handle Handle to use when enqueuing the asset. Required. + * @type string $handle Handle to use when enqueuing the asset. Optional. * @type array $dependencies Script or Style dependencies. Optional. * } + * @return array Array detailing which script and/or style handles got registered. */ -function register_asset( string $manifest_path, string $target_asset, array $options = [] ) : void { +function register_asset( string $manifest_path, string $target_asset, array $options = [] ) : array { $defaults = [ 'dependencies' => [], ]; @@ -338,82 +339,88 @@ function register_asset( string $manifest_path, string $target_asset, array $opt // If asset is not present in manifest, attempt to resolve the $target_asset // relative to the folder containing the manifest file. - if ( empty( $asset_uri ) && is_readable( $manifest_folder . $target_asset ) ) { + if ( empty( $asset_uri ) ) { // TODO: Consider checking is_readable( $manifest_folder . $target_asset ) // and warning (in console or error log) if it is not present on disk. $asset_uri = $target_asset; } - // Use the asset URI as the asset handle if no handle was provided. - if ( empty( $options['handle'] ) ) { - $options['handle'] = $asset_uri; - } - // Reconcile static asset build paths relative to the manifest's directory. if ( strpos( $asset_uri, '//' ) === false ) { $asset_uri = Paths\get_file_uri( $manifest_folder . $asset_uri ); } + // Use the requested asset as the asset handle if no handle was provided. + $asset_handle = $options['handle'] ?? $target_asset; $asset_version = Manifest\get_version( $asset_uri, $manifest_path ); + // Track registered handles so we can enqueue the correct assets later. + $handles = []; + if ( is_css( $asset_uri ) ) { // Register a normal CSS bundle. wp_register_style( - $options['handle'], + $asset_handle, $asset_uri, $options['dependencies'], $asset_version ); + $handles['style'] = $asset_handle; } elseif ( $is_js_style_fallback ) { // We're registering a JS bundle when we originally asked for a CSS bundle. // Register the JS, but if any dependencies were passed in, also register a // dummy style bundle so that those style dependencies still get loaded. Admin\maybe_setup_ssl_cert_error_handling( $asset_uri ); _register_or_update_script( - $options['handle'], + $asset_handle, $asset_uri, [], $asset_version, true ); + $handles['script'] = $asset_handle; if ( ! empty( $options['dependencies'] ) ) { wp_register_style( - $options['handle'], + $asset_handle, false, $options['dependencies'], $asset_version ); + $handles['style'] = $asset_handle; } } else { // Register a normal JS bundle. Admin\maybe_setup_ssl_cert_error_handling( $asset_uri ); _register_or_update_script( - $options['handle'], + $asset_handle, $asset_uri, $options['dependencies'], $asset_version, true ); + $handles['script'] = $asset_handle; } + + return $handles; } /** - * Attempt to register and then enqueue a particular script bundle from a manifest. + * Register and immediately enqueue a particular asset within a manifest. * * @param string $manifest_path File system path for an asset manifest JSON file. * @param string $target_asset Asset to retrieve within the specified manifest. * @param array $options { - * @type string $handle Handle to use when enqueuing the asset. Required. + * @type string $handle Handle to use when enqueuing the asset. Optional. * @type array $dependencies Script or Style dependencies. Optional. * } */ function enqueue_asset( string $manifest_path, string $target_asset, array $options = [] ) : void { - register_asset( $manifest_path, $target_asset, $options ); + $registered_handles = register_asset( $manifest_path, $target_asset, $options ); - // $target_asset will share a filename extension with the enqueued asset. - if ( is_css( $target_asset ) ) { - wp_enqueue_style( $options['handle'] ); - } else { - wp_enqueue_script( $options['handle'] ); + if ( isset( $registered_handles['script'] ) ) { + wp_enqueue_script( $registered_handles['script'] ); + } + if ( isset( $registered_handles['style'] ) ) { + wp_enqueue_style( $registered_handles['style'] ); } } diff --git a/tests/inc/class-test-asset-loader.php b/tests/inc/class-test-asset-loader.php index f8a1bf7..af0cda4 100644 --- a/tests/inc/class-test-asset-loader.php +++ b/tests/inc/class-test-asset-loader.php @@ -111,50 +111,182 @@ public function provide_is_css_cases() { } /** - * Test register_asset() behavior with production JS assets. + * Test register_asset() behavior for script assets. + * + * @dataProvider provide_script_asset_cases */ - public function test_register_prod_script() : void { - Asset_Loader\enqueue_asset( $this->prod_manifest, 'editor.js', [ - 'handle' => 'custom-handle', - 'dependencies' => [ 'wp-data' ], - ] ); + public function test_register_script( string $manifest, string $resource, array $options, array $expected ) : void { + Asset_Loader\register_asset( $this->{$manifest}, $resource, $options ); - $this->assertEquals( - [ - 'handle' => 'custom-handle', - 'src' => 'https://my.theme/uri/fixtures/editor.03bfa96fd1c694ca18b3.js', - 'deps' => [ 'wp-data' ], - 'ver' => null, + $this->assertEquals( $expected, $this->scripts->get_registered( $expected['handle'] ) ); + } + + /** + * Test enqueue_asset() behavior for script assets. + * + * @dataProvider provide_script_asset_cases + */ + public function test_enqueue_script( string $manifest, string $resource, array $options, array $expected ) : void { + Asset_Loader\enqueue_asset( $this->{$manifest}, $resource, $options ); + + $this->assertEquals( $expected, $this->scripts->get_registered( $expected['handle'] ) ); + $this->assertEmpty( $this->styles->registered ); + + $this->assertEquals( [ $expected['handle'] ], $this->scripts->get_enqueued() ); + } + + /** + * Script test cases for register_asset and enqueue_asset. + */ + public function provide_script_asset_cases() : array { + return [ + 'production script' => [ + 'prod_manifest', + 'editor.js', + [ + 'handle' => 'custom-handle', + 'dependencies' => [ 'wp-data' ], + ], + [ + 'handle' => 'custom-handle', + 'src' => 'https://my.theme/uri/fixtures/editor.03bfa96fd1c694ca18b3.js', + 'deps' => [ 'wp-data' ], + 'ver' => null, + ], ], - $this->scripts->get_registered( 'custom-handle' ) - ); + 'development script' => [ + 'dev_manifest', + 'editor.js', + [ + 'handle' => 'editor-bundle', + 'dependencies' => [ 'wp-data', 'wp-element' ], + ], + [ + 'handle' => 'editor-bundle', + 'src' => 'https://localhost:9090/build/editor.js', + 'deps' => [ 'wp-data', 'wp-element' ], + 'ver' => '499bb147f8e7234d957a47ac983e19e7', + ], + ], + ]; + } + + /** + * Test register_asset() behavior for style assets. + * + * @dataProvider provide_style_asset_cases + */ + public function test_register_style( string $manifest, string $resource, array $options, array $expected ) : void { + Asset_Loader\register_asset( $this->{$manifest}, $resource, $options ); + + $this->assertEquals( $expected, $this->styles->get_registered( $expected['handle'] ) ); } /** - * Test enqueue_asset() behavior with production JS assets. + * Test enqueue_asset() behavior for style assets. + * + * @dataProvider provide_style_asset_cases */ - public function test_enqueue_prod_script() : void { - Asset_Loader\enqueue_asset( $this->prod_manifest, 'editor.js', [ - 'handle' => 'custom-handle', - 'dependencies' => [ 'wp-data' ], - ] ); + public function test_enqueue_style( string $manifest, string $resource, array $options, array $expected ) : void { + Asset_Loader\enqueue_asset( $this->{$manifest}, $resource, $options ); + + $this->assertEquals( $expected, $this->styles->get_registered( $expected['handle'] ) ); + $this->assertEmpty( $this->scripts->registered ); + $this->assertEquals( [ $expected['handle'] ], $this->styles->get_enqueued() ); + } + + /** + * Stylesheet test cases for register_asset and enqueue_asset. + */ + public function provide_style_asset_cases() : array { + return [ + 'production stylesheet' => [ + 'prod_manifest', + 'frontend-styles.css', + [ + 'handle' => 'frontend-styles', + 'dependencies' => [ 'dependency-style' ], + ], + [ + 'handle' => 'frontend-styles', + 'src' => 'https://my.theme/uri/fixtures/frontend-styles.96a500e3dd1eb671f25e.css', + 'deps' => [ 'dependency-style' ], + 'ver' => null, + ], + ], + 'production stylesheet with no explicit handle' => [ + 'prod_manifest', + 'frontend-styles.css', + [ + 'dependencies' => [ 'dependency-style' ], + ], + [ + 'handle' => 'frontend-styles.css', + 'src' => 'https://my.theme/uri/fixtures/frontend-styles.96a500e3dd1eb671f25e.css', + 'deps' => [ 'dependency-style' ], + 'ver' => null, + ], + ], + 'production stylesheet not in manifest' => [ + 'prod_manifest', + 'theme.css', + [], + [ + 'handle' => 'theme.css', + 'src' => 'https://my.theme/uri/fixtures/theme.css', + 'deps' => [], + 'ver' => '2a9dea09d6ed09f7c4ce052b82cc4999', + ], + ], + ]; + } + + public function test_register_dev_stylesheet() : void { + Asset_Loader\register_asset( + $this->dev_manifest, + 'frontend-styles.css', + [ + 'handle' => 'frontend-styles', + ] + ); + $this->assertEquals( + [ + 'handle' => 'frontend-styles', + 'src' => 'https://localhost:9090/build/frontend-styles.js', + 'deps' => [], + 'ver' => '499bb147f8e7234d957a47ac983e19e7', + ], + $this->scripts->get_registered( 'frontend-styles' ) + ); + $this->assertEquals( [], $this->styles->registered ); + } + + public function test_enqueue_dev_stylesheet() : void { + Asset_Loader\enqueue_asset( + $this->dev_manifest, + 'frontend-styles.css', + [ + 'handle' => 'frontend-styles', + ] + ); $this->assertEquals( [ - 'handle' => 'custom-handle', - 'src' => 'https://my.theme/uri/fixtures/editor.03bfa96fd1c694ca18b3.js', - 'deps' => [ 'wp-data' ], - 'ver' => null, + 'handle' => 'frontend-styles', + 'src' => 'https://localhost:9090/build/frontend-styles.js', + 'deps' => [], + 'ver' => '499bb147f8e7234d957a47ac983e19e7', ], - $this->scripts->get_registered( 'custom-handle' ) + $this->scripts->get_registered( 'frontend-styles' ) ); + $this->assertEquals( [], $this->styles->registered ); - $this->assertEquals( [ 'custom-handle' ], $this->scripts->get_enqueued() ); + $this->assertEquals( [ 'frontend-styles' ], $this->scripts->get_enqueued() ); } - public function test_register_css_asset_production() : void { + public function test_register_dev_stylesheet_with_dependencies() : void { Asset_Loader\register_asset( - $this->prod_manifest, + $this->dev_manifest, 'frontend-styles.css', [ 'handle' => 'frontend-styles', @@ -164,16 +296,25 @@ public function test_register_css_asset_production() : void { $this->assertEquals( [ 'handle' => 'frontend-styles', - 'src' => 'https://my.theme/uri/fixtures/frontend-styles.96a500e3dd1eb671f25e.css', + 'src' => 'https://localhost:9090/build/frontend-styles.js', + 'deps' => [], + 'ver' => '499bb147f8e7234d957a47ac983e19e7', + ], + $this->scripts->get_registered( 'frontend-styles' ) + ); + $this->assertEquals( + [ + 'handle' => 'frontend-styles', + 'src' => false, 'deps' => [ 'dependency-style' ], - 'ver' => null, + 'ver' => '499bb147f8e7234d957a47ac983e19e7', ], $this->styles->get_registered( 'frontend-styles' ) ); } - public function test_register_css_asset_dev() : void { - Asset_Loader\register_asset( + public function test_enqueue_dev_stylesheet_with_dependencies() : void { + Asset_Loader\enqueue_asset( $this->dev_manifest, 'frontend-styles.css', [ @@ -199,9 +340,12 @@ public function test_register_css_asset_dev() : void { ], $this->styles->get_registered( 'frontend-styles' ) ); + + $this->assertEquals( [ 'frontend-styles' ], $this->scripts->get_enqueued() ); + $this->assertEquals( [ 'frontend-styles' ], $this->styles->get_enqueued() ); } - public function test_register_css_asset_then_corresponding_js_asset_dev() : void { + public function test_register_dev_stylesheet_then_corresponding_dev_script() : void { Asset_Loader\register_asset( $this->dev_manifest, 'editor.css', @@ -238,11 +382,43 @@ public function test_register_css_asset_then_corresponding_js_asset_dev() : void ); } - /* - - Load dev JS file (simple case) - - Load prod JS file (simple case) - - Load production CSS file (simple case) - - enqueue CSS that falls back to JS - - enqueue CSS that falls back to JS *and() JS and ensure both their dependencies get handled - */ + public function test_enqueue_dev_stylesheet_then_corresponding_dev_script() : void { + Asset_Loader\enqueue_asset( + $this->dev_manifest, + 'editor.css', + [ + 'handle' => 'editor', + 'dependencies' => [ 'style-dependency' ], + ] + ); + Asset_Loader\enqueue_asset( + $this->dev_manifest, + 'editor.js', + [ + 'handle' => 'editor', + 'dependencies' => [ 'script-dependency' ], + ] + ); + $this->assertEquals( + [ + 'handle' => 'editor', + 'src' => 'https://localhost:9090/build/editor.js', + 'deps' => [ 'script-dependency' ], + 'ver' => '499bb147f8e7234d957a47ac983e19e7', + ], + $this->scripts->get_registered( 'editor' ) + ); + $this->assertEquals( + [ + 'handle' => 'editor', + 'src' => false, + 'deps' => [ 'style-dependency' ], + 'ver' => '499bb147f8e7234d957a47ac983e19e7', + ], + $this->styles->get_registered( 'editor' ) + ); + + $this->assertEquals( [ 'editor' ], $this->scripts->get_enqueued() ); + $this->assertEquals( [ 'editor' ], $this->styles->get_enqueued() ); + } }