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.

Sign up for GitHub

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

Minify script used for ajax activation of features; warn if absent and serve original file when SCRIPT_DEBUG is enabled #1658

Merged
merged 5 commits into from
Nov 26, 2024
Merged

Minify script used for ajax activation of features; warn if absent and serve original file when SCRIPT_DEBUG is enabled #1658

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
cc3b95f
Add missing JS minification for new PL script
westonruter Nov 17, 2024
1ca7c77
Introduce perflab_get_asset_src()
westonruter Nov 19, 2024
f54c488
Change perflab_get_asset_url() to return path instead of URL
westonruter Nov 19, 2024
be0401c
Eliminate local env check
westonruter Nov 20, 2024
5509f2c
Update other plugins to warn when minified asset is missing
westonruter Nov 20, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 38 additions & 2 deletions plugins/embed-optimizer/hooks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ function embed_optimizer_filter_extension_module_urls( $extension_module_urls ):
if ( ! is_array( $extension_module_urls ) ) {
$extension_module_urls = array();
}
$extension_module_urls[] = add_query_arg( 'ver', EMBED_OPTIMIZER_VERSION, plugin_dir_url( __FILE__ ) . sprintf( 'detect%s.js', wp_scripts_get_suffix() ) );
$extension_module_urls[] = add_query_arg( 'ver', EMBED_OPTIMIZER_VERSION, plugin_dir_url( __FILE__ ) . embed_optimizer_get_asset_path( 'detect.js' ) );
return $extension_module_urls;
}

Expand Down Expand Up @@ -326,7 +326,7 @@ function embed_optimizer_lazy_load_scripts(): void {
* @since 0.2.0
*/
function embed_optimizer_get_lazy_load_script(): string {
$script = file_get_contents( __DIR__ . sprintf( '/lazy-load%s.js', wp_scripts_get_suffix() ) ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- It's a local filesystem path not a remote request.
$script = file_get_contents( __DIR__ . '/' . embed_optimizer_get_asset_path( 'lazy-load.js' ) ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- It's a local filesystem path not a remote request.

if ( false === $script ) {
return '';
Expand Down Expand Up @@ -424,3 +424,39 @@ function embed_optimizer_render_generator(): void {
// Use the plugin slug as it is immutable.
echo '<meta name="generator" content="embed-optimizer ' . esc_attr( EMBED_OPTIMIZER_VERSION ) . '">' . "\n";
}

/**
* Gets the path to a script or stylesheet.
*
* @since n.e.x.t
*
* @param string $src_path Source path, relative to plugin root.
* @param string|null $min_path Minified path. If not supplied, then '.min' is injected before the file extension in the source path.
* @return string URL to script or stylesheet.
*/
function embed_optimizer_get_asset_path( string $src_path, ?string $min_path = null ): string {
if ( null === $min_path ) {
// Note: wp_scripts_get_suffix() is not used here because we need access to both the source and minified paths.
$min_path = (string) preg_replace( '/(?=\.\w+$)/', '.min', $src_path );
}

$force_src = false;
if ( WP_DEBUG && ! file_exists( trailingslashit( __DIR__ ) . $min_path ) ) {
$force_src = true;
wp_trigger_error(
__FUNCTION__,
sprintf(
/* translators: %s is the minified asset path */
__( 'Minified asset has not been built: %s', 'embed-optimizer' ),
$min_path
),
E_USER_WARNING
);
}

if ( SCRIPT_DEBUG || $force_src ) {
return $src_path;
}

return $min_path;
}
36 changes: 36 additions & 0 deletions plugins/image-prioritizer/helper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,3 +76,39 @@ function image_prioritizer_register_tag_visitors( OD_Tag_Visitor_Registry $regis
$video_visitor = new Image_Prioritizer_Video_Tag_Visitor();
$registry->register( 'image-prioritizer/video', $video_visitor );
}

/**
* Gets the path to a script or stylesheet.
*
* @since n.e.x.t
*
* @param string $src_path Source path, relative to plugin root.
* @param string|null $min_path Minified path. If not supplied, then '.min' is injected before the file extension in the source path.
* @return string URL to script or stylesheet.
*/
function image_prioritizer_get_asset_path( string $src_path, ?string $min_path = null ): string {
if ( null === $min_path ) {
// Note: wp_scripts_get_suffix() is not used here because we need access to both the source and minified paths.
$min_path = (string) preg_replace( '/(?=\.\w+$)/', '.min', $src_path );
}

$force_src = false;
if ( WP_DEBUG && ! file_exists( trailingslashit( __DIR__ ) . $min_path ) ) {
$force_src = true;
wp_trigger_error(
__FUNCTION__,
sprintf(
/* translators: %s is the minified asset path */
__( 'Minified asset has not been built: %s', 'image-prioritizer' ),
$min_path
),
E_USER_WARNING
);
}

if ( SCRIPT_DEBUG || $force_src ) {
return $src_path;
}

return $min_path;
}
3 changes: 2 additions & 1 deletion plugins/image-prioritizer/hooks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,8 @@
* @since 0.2.0
*/
function image_prioritizer_get_lazy_load_script(): string {
$script = file_get_contents( __DIR__ . sprintf( '/lazy-load%s.js', wp_scripts_get_suffix() ) ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- It's a local filesystem path not a remote request.
$path = image_prioritizer_get_asset_path( 'lazy-load.js' );
$script = file_get_contents( __DIR__ . '/' . $path ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- It's a local filesystem path not a remote request.

if ( false === $script ) {
return '';
Expand Down
2 changes: 1 addition & 1 deletion plugins/optimization-detective/detection.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -112,7 +112,7 @@ static function ( OD_URL_Metric_Group $group ): array {
return wp_get_inline_script_tag(
sprintf(
'import detect from %s; detect( %s );',
wp_json_encode( add_query_arg( 'ver', OPTIMIZATION_DETECTIVE_VERSION, plugin_dir_url( __FILE__ ) . sprintf( 'detect%s.js', wp_scripts_get_suffix() ) ) ),
wp_json_encode( add_query_arg( 'ver', OPTIMIZATION_DETECTIVE_VERSION, plugin_dir_url( __FILE__ ) . od_get_asset_path( 'detect.js' ) ) ),
wp_json_encode( $detect_args )
),
array( 'type' => 'module' )
Expand Down
36 changes: 36 additions & 0 deletions plugins/optimization-detective/helper.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,3 +64,39 @@ function od_render_generator_meta_tag(): void {
// Use the plugin slug as it is immutable.
echo '<meta name="generator" content="optimization-detective ' . esc_attr( OPTIMIZATION_DETECTIVE_VERSION ) . '">' . "\n";
}

/**
* Gets the path to a script or stylesheet.
*
* @since n.e.x.t
*
* @param string $src_path Source path, relative to plugin root.
* @param string|null $min_path Minified path. If not supplied, then '.min' is injected before the file extension in the source path.
* @return string URL to script or stylesheet.
*/
function od_get_asset_path( string $src_path, ?string $min_path = null ): string {
if ( null === $min_path ) {
// Note: wp_scripts_get_suffix() is not used here because we need access to both the source and minified paths.
$min_path = (string) preg_replace( '/(?=\.\w+$)/', '.min', $src_path );
}

$force_src = false;
if ( WP_DEBUG && ! file_exists( trailingslashit( __DIR__ ) . $min_path ) ) {
$force_src = true;
wp_trigger_error(
__FUNCTION__,
sprintf(
/* translators: %s is the minified asset path */
__( 'Minified asset has not been built: %s', 'optimization-detective' ),
$min_path
),
E_USER_WARNING
);
}

if ( SCRIPT_DEBUG || $force_src ) {
return $src_path;
}

return $min_path;
}
38 changes: 37 additions & 1 deletion plugins/performance-lab/includes/admin/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,42 @@ function perflab_dismiss_wp_pointer_wrapper(): void {
}
add_action( 'wp_ajax_dismiss-wp-pointer', 'perflab_dismiss_wp_pointer_wrapper', 0 );

/**
* Gets the path to a script or stylesheet.
*
* @since n.e.x.t
*
* @param string $src_path Source path.
* @param string|null $min_path Minified path. If not supplied, then '.min' is injected before the file extension in the source path.
* @return string URL to script or stylesheet.
*/
function perflab_get_asset_path( string $src_path, ?string $min_path = null ): string {
if ( null === $min_path ) {
// Note: wp_scripts_get_suffix() is not used here because we need access to both the source and minified paths.
$min_path = (string) preg_replace( '/(?=\.\w+$)/', '.min', $src_path );
}

$force_src = false;
if ( WP_DEBUG && ! file_exists( trailingslashit( PERFLAB_PLUGIN_DIR_PATH ) . $min_path ) ) {
$force_src = true;
wp_trigger_error(
Copy link
Member

Choose a reason for hiding this comment

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

If someone get these Warning: perflab_get_asset_path(): Minified asset has not been built: includes/admin/plugin-activate-ajax.min.js in /var/www/html/wp-includes/functions.php on line 6114 error in production they didn't understand how to solve these or from which plugin the error comes.

Copy link
Member

Choose a reason for hiding this comment

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

Ideally this shouldn't be checked in a production build of the plugin because there those files will always exist, so it is indeed only for development of the plugin.

Using WP_DEBUG is not an ideal solution because it doesn't give any indication for how this plugin is being used (e.g. in development version like git checkout, or production build). That said, a better way to indicate that may involve some extra work that I'm not sure would be worth it.

cc @westonruter

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, it would be extremely unlikely that someone would install the git repo directly onto production. The purpose of this code here is exclusively targeting plugin contributors who are cloning the repo anew or checking out a new branch in which a new asset appears. This error will inform them of the need to do a build while at the same time falling back to the source file rather than the minified file. And when WP_DEBUG is enabled (as it normally should be during development) then the user will get the notices. If someone has SCRIPT_DEBUG enabled, they'll get the source versions anyway, but if WP_DEBUG is enabled it will still do the checks to see if the minified file is present as this indicates they need to do a build.

__FUNCTION__,
sprintf(
/* translators: %s is the minified asset path */
__( 'Minified asset has not been built: %s', 'performance-lab' ),
$min_path
),
E_USER_WARNING
);
}

if ( SCRIPT_DEBUG || $force_src ) {
return $src_path;
}

return $min_path;
}

/**
* Callback function to handle admin scripts.
*
Expand All @@ -228,7 +264,7 @@ function perflab_enqueue_features_page_scripts(): void {
// Enqueue plugin activate AJAX script and localize script data.
wp_enqueue_script(
'perflab-plugin-activate-ajax',
plugin_dir_url( PERFLAB_MAIN_FILE ) . 'includes/admin/plugin-activate-ajax.js',
plugin_dir_url( PERFLAB_MAIN_FILE ) . perflab_get_asset_path( 'includes/admin/plugin-activate-ajax.js' ),
array( 'wp-i18n', 'wp-a11y', 'wp-api-fetch' ),
PERFLAB_VERSION,
true
Expand Down
2 changes: 2 additions & 0 deletions tools/phpstan/constants.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,3 +17,5 @@
define( 'IMAGE_PRIORITIZER_VERSION', '0.0.0' );

define( 'EMBED_OPTIMIZER_VERSION', '0.0.0' );

define( 'PERFLAB_PLUGIN_DIR_PATH', __DIR__ . '/../../plugins/performance-lab/' );
35 changes: 35 additions & 0 deletions webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,46 @@ const sharedConfig = {

// Store plugins that require build process.
const pluginsWithBuild = [
'performance-lab',
'embed-optimizer',
'image-prioritizer',
'optimization-detective',
'web-worker-offloading',
];

/**
* Webpack Config: Performance Lab
*
* @param {*} env Webpack environment
* @return {Object} Webpack configuration
*/
const performanceLab = ( env ) => {
if ( env.plugin && env.plugin !== 'performance-lab' ) {
return defaultBuildConfig;
}

const pluginDir = path.resolve( __dirname, 'plugins/performance-lab' );

return {
...sharedConfig,
name: 'performance-lab',
plugins: [
new CopyWebpackPlugin( {
patterns: [
{
from: `${ pluginDir }/includes/admin/plugin-activate-ajax.js`,
to: `${ pluginDir }/includes/admin/plugin-activate-ajax.min.js`,
},
],
} ),
new WebpackBar( {
name: 'Building Performance Lab Assets',
color: '#2196f3',
} ),
],
};
};

/**
* Webpack Config: Embed Optimizer
*
Expand Down Expand Up @@ -286,6 +320,7 @@ const buildPlugin = ( env ) => {
};

module.exports = [
performanceLab,
embedOptimizer,
imagePrioritizer,
optimizationDetective,
Expand Down
Loading