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

Add unminified source JS files to builds #1643

Merged
merged 19 commits into from
Nov 13, 2024
Merged

Add unminified source JS files to builds #1643

Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
a15db87
Skip minification when creating the production build
ShyamGadde Nov 10, 2024
9ba7400
Create minified script files when building Optimization Detective Assets
ShyamGadde Nov 10, 2024
236c61f
Use unminified dev version of scripts when `SCRIPT_DEBUG` is enabled
ShyamGadde Nov 10, 2024
b686515
Remove redundant comment
ShyamGadde Nov 11, 2024
a3ab16a
Refactor detection script file handling to use wp_scripts_get_suffix
ShyamGadde Nov 12, 2024
1b0731d
Disable minification attempt of `web-vitals.js`
ShyamGadde Nov 12, 2024
b7e8e30
Disable minification of unminified partytown script files under `lib/…
ShyamGadde Nov 12, 2024
b7127d7
Add webpack config to create minified versions of script files in Emb…
ShyamGadde Nov 12, 2024
888ecdb
Add minified file exists check to load.php in Embed Optimizer plugin
ShyamGadde Nov 12, 2024
693e1db
Conditionally load minified version of scripts in Embed Optimizer plugin
ShyamGadde Nov 12, 2024
163082f
Add webpack config to create minified versions of script files in Ima…
ShyamGadde Nov 12, 2024
232e227
Add minified file exists check to load.php in Image Prioritizer plugin
ShyamGadde Nov 12, 2024
9747f7b
Conditionally load minified version of scripts in Image Prioritizer p…
ShyamGadde Nov 12, 2024
c34ac17
Merge branch 'trunk' into add/unminified-js-to-prod-builds
ShyamGadde Nov 12, 2024
37d8078
Merge branch 'trunk' into add/unminified-js-to-prod-builds
ShyamGadde Nov 13, 2024
0a1354b
Load unminified version of partytown scripts when `SCRIPT_DEBUG` is e…
ShyamGadde Nov 13, 2024
6e18901
Refactor conditional logic for script path assignment
ShyamGadde Nov 13, 2024
04f7a75
Merge branch 'trunk' into add/unminified-js-to-prod-builds
ShyamGadde Nov 13, 2024
7af4b07
Add changelog entries
westonruter Nov 13, 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
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ nbproject/

build
.wp-env.override.json
*.min.js
*.asset.php

############
Expand Down
4 changes: 2 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__ ) . 'detect.js' );
$extension_module_urls[] = add_query_arg( 'ver', EMBED_OPTIMIZER_VERSION, plugin_dir_url( __FILE__ ) . sprintf( 'detect%s.js', wp_scripts_get_suffix() ) );
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__ . '/lazy-load.js' ); // 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__ . 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.

if ( false === $script ) {
return '';
Expand Down
18 changes: 18 additions & 0 deletions plugins/embed-optimizer/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,24 @@ static function ( string $version ): void {
return;
}

if (
( is_admin() || ( defined( 'WP_CLI' ) && WP_CLI ) ) &&
! file_exists( __DIR__ . '/detect.min.js' )
) {
// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
trigger_error(
esc_html(
sprintf(
/* translators: 1: File path. 2: CLI command. */
'[Embed Optimizer] ' . __( 'Unable to load %1$s. Please make sure you have run %2$s.', 'embed-optimizer' ),
'detect.min.js',
'`npm install && npm run build:plugin:embed-optimizer`'
)
),
E_USER_ERROR
);
}

define( 'EMBED_OPTIMIZER_VERSION', $version );

// Load in the Embed Optimizer plugin hooks.
Expand Down
2 changes: 1 addition & 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,7 @@
* @since 0.2.0
*/
function image_prioritizer_get_lazy_load_script(): string {
$script = file_get_contents( __DIR__ . '/lazy-load.js' ); // 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__ . 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.

if ( false === $script ) {
return '';
Expand Down
18 changes: 18 additions & 0 deletions plugins/image-prioritizer/load.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,24 @@ static function ( string $version ): void {
return;
}

if (
( is_admin() || ( defined( 'WP_CLI' ) && WP_CLI ) ) &&
! file_exists( __DIR__ . '/lazy-load.min.js' )
) {
// phpcs:ignore WordPress.PHP.DevelopmentFunctions.error_log_trigger_error
trigger_error(
esc_html(
sprintf(
/* translators: 1: File path. 2: CLI command. */
'[Image Prioritizer] ' . __( 'Unable to load %1$s. Please make sure you have run %2$s.', 'image-prioritizer' ),
'lazy-load.min.js',
'`npm install && npm run build:plugin:image-prioritizer`'
)
),
E_USER_ERROR
);
}

define( 'IMAGE_PRIORITIZER_VERSION', $version );

require_once __DIR__ . '/helper.php';
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 @@ -61,7 +61,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__ ) . 'detect.js' ) ),
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( $detect_args )
),
array( 'type' => 'module' )
Expand Down
7 changes: 6 additions & 1 deletion plugins/web-worker-offloading/hooks.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@
function plwwo_register_default_scripts( WP_Scripts $scripts ): void {
// The source code for partytown.js is built from <https://github.com/BuilderIO/partytown/blob/b292a14047a0c12ca05ba97df1833935d42fdb66/src/lib/main/snippet.ts>.
// See webpack config in the WordPress/performance repo: <https://github.com/WordPress/performance/blob/282a068f3eb2575d37aeb9034e894e7140fcddca/webpack.config.js#L84-L130>.
$partytown_js = file_get_contents( __DIR__ . '/build/partytown.js' ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- It's a local filesystem path not a remote request.
$partytown_js_path = '/build/partytown.js';
if ( defined( 'SCRIPT_DEBUG' ) && SCRIPT_DEBUG ) {
$partytown_js_path = '/build/debug/partytown.js';
}
ShyamGadde marked this conversation as resolved.
Show resolved Hide resolved

$partytown_js = file_get_contents( __DIR__ . $partytown_js_path ); // phpcs:ignore WordPress.WP.AlternativeFunctions.file_get_contents_file_get_contents -- It's a local filesystem path not a remote request.
if ( false === $partytown_js ) {
return;
}
Expand Down
98 changes: 93 additions & 5 deletions webpack.config.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,82 @@ const sharedConfig = {
};

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

/**
* Webpack Config: Embed Optimizer
*
* @param {*} env Webpack environment
* @return {Object} Webpack configuration
*/
const embedOptimizer = ( env ) => {
if ( env.plugin && env.plugin !== 'embed-optimizer' ) {
return defaultBuildConfig;
}

const pluginDir = path.resolve( __dirname, 'plugins/embed-optimizer' );

return {
...sharedConfig,
name: 'embed-optimizer',
plugins: [
new CopyWebpackPlugin( {
patterns: [
{
from: `${ pluginDir }/detect.js`,
to: `${ pluginDir }/detect.min.js`,
},
{
from: `${ pluginDir }/lazy-load.js`,
to: `${ pluginDir }/lazy-load.min.js`,
},
],
} ),
new WebpackBar( {
name: 'Building Embed Optimizer Assets',
color: '#2196f3',
} ),
],
};
};

/**
* Webpack Config: Image Prioritizer
*
* @param {*} env Webpack environment
* @return {Object} Webpack configuration
*/
const imagePrioritizer = ( env ) => {
if ( env.plugin && env.plugin !== 'image-prioritizer' ) {
return defaultBuildConfig;
}

const pluginDir = path.resolve( __dirname, 'plugins/image-prioritizer' );

return {
...sharedConfig,
name: 'image-prioritizer',
plugins: [
new CopyWebpackPlugin( {
patterns: [
{
from: `${ pluginDir }/lazy-load.js`,
to: `${ pluginDir }/lazy-load.min.js`,
},
],
} ),
new WebpackBar( {
name: 'Building Image Prioritizer Assets',
color: '#2196f3',
} ),
],
};
};

/**
* Webpack Config: Optimization Detective
Expand All @@ -50,7 +125,7 @@ const optimizationDetective = ( env ) => {
const source = path.resolve( __dirname, 'node_modules/web-vitals' );
const destination = path.resolve(
__dirname,
'plugins/optimization-detective/build'
'plugins/optimization-detective'
);

return {
Expand All @@ -61,16 +136,21 @@ const optimizationDetective = ( env ) => {
patterns: [
{
from: `${ source }/dist/web-vitals.js`,
to: `${ destination }/web-vitals.js`,
to: `${ destination }/build/web-vitals.js`,
Copy link
Member

Choose a reason for hiding this comment

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

Is this missing the flag to say it is minified?

Suggested change
to: `${ destination }/build/web-vitals.js`,
to: `${ destination }/build/web-vitals.js`,
info: { minimized: true }`,

Copy link
Contributor Author

@ShyamGadde ShyamGadde Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Yes! I just realized this. I should probably make this change in the config for Web Worker Offloading as well, which is copying the partytown files.

info: { minimized: true },
},
{
from: `${ source }/package.json`,
to: `${ destination }/web-vitals.asset.php`,
to: `${ destination }/build/web-vitals.asset.php`,
transform: {
transformer: assetDataTransformer,
cache: false,
},
},
{
from: `${ destination }/detect.js`,
to: `${ destination }/detect.min.js`,
},
],
} ),
new WebpackBar( {
Expand Down Expand Up @@ -110,6 +190,7 @@ const webWorkerOffloading = ( env ) => {
{
from: `${ source }/lib/`,
to: `${ destination }`,
info: { minimized: true },
},
{
from: `${ source }/package.json`,
Expand Down Expand Up @@ -164,6 +245,7 @@ const buildPlugin = ( env ) => {
{
from,
to,
info: { minimized: true },
Copy link
Member

Choose a reason for hiding this comment

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

This won't attempt to create a minified version of web-vitals.js, will it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, actually it's the opposite - setting info: { minimized: true } tells Webpack to skip running these files through the minimizer. It's a way to tell Webpack to copy the files 'as is' without trying to minimize them. The documentation specifically mentions this is 'Useful if you need to simply copy *.js files to destination "as is" without evaluating and minimizing them using Terser.'

ref: https://webpack.js.org/plugins/copy-webpack-plugin/#skip-running-javascript-files-through-a-minimizer

Copy link
Member

Choose a reason for hiding this comment

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

OK, thank you. I'm a little confused. We want to create minified versions of all the scripts, except for those which are copied from node_modules, right? So we'd want to copy from node_modules/web-vitals/dist and node_modules/@builder.io/partytown as pre-minified, but everything else we'd want to create minified versions for. I don't see how that is all configured here. (I'm a webpack novice!)

Copy link
Contributor Author

@ShyamGadde ShyamGadde Nov 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Let me clarify the current state and what needs to be fixed:

  1. For files from node_modules (web-vitals and partytown):

    • These come pre-minified, but when copying them to the build directory for their respective plugins, we're not telling Webpack that. Currently, I have configured minimization skipping only for the production build.
    • Need to add info: { minimized: true } to prevent re-minification attempts

  2. For plugin JS files that need minified versions:

    • Minified version for optimization-detective/detect.js is being created (detect.min.js)
    • Need to add minified versions for (I missed these as they were not initially part of the issue description):
      • embed-optimizer/detect.js
      • embed-optimizer/lazy-load.js
      • image-prioritizer/lazy-load.js

I'll update this PR to handle both these cases. Would that address your concerns?

Copy link
Member

Choose a reason for hiding this comment

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

Yes, I believe so!

globOptions: {
dot: true,
ignore: [
Expand Down Expand Up @@ -203,4 +285,10 @@ const buildPlugin = ( env ) => {
};
};

module.exports = [ optimizationDetective, webWorkerOffloading, buildPlugin ];
module.exports = [
embedOptimizer,
imagePrioritizer,
optimizationDetective,
webWorkerOffloading,
buildPlugin,
];
Loading