Skip to content

refactor(assets): align dist filenames with webpack manifest #488

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

Open
firestar300 wants to merge 4 commits into
base: master
Choose a base branch
from
+51 −57
Open

refactor(assets): align dist filenames with webpack manifest #488

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
5f5a2e3
refactor(assets): align dist filenames with webpack manifest
firestar300 Apr 24, 2026
f1b0696
fix(Editor): remove is_minified function condition
firestar300 Apr 24, 2026
cb08f43
fix(Editor): remove is_minified function condition
firestar300 Apr 24, 2026
d2775dc
fix(assets): guard login stylesheet path when assets.json is missing
firestar300 Apr 27, 2026
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
20 changes: 6 additions & 14 deletions config/plugins.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,12 @@ module.exports = {
defaultImageFormat: 'jpg', // Generated image format (jpg, png, webp, avif)
silence: true, // Suppress console output
}),
new MiniCssExtractPlugin({
filename: '[name].css',
}),
new WebpackManifestPlugin({
fileName: 'assets.json',
}),
]

if (mode === 'production') {
Expand All @@ -61,20 +67,6 @@ module.exports = {
generateStatsFile: true,
})
)
plugins.push(
new WebpackManifestPlugin({
fileName: 'assets.json',
}),
new MiniCssExtractPlugin({
filename: '[name].[contenthash:8].min.css',
})
)
} else {
plugins.push(
new MiniCssExtractPlugin({
filename: '[name].css',
})
)
}

return plugins
Expand Down
2 changes: 1 addition & 1 deletion config/webpack.prod.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ module.exports = merge(common, {
mode: mode,
stats: 'minimal',
output: {
filename: '[name]-min.js',
filename: '[name].js',
},
optimization: {
concatenateModules: true,
Expand Down
82 changes: 42 additions & 40 deletions inc/Services/Assets.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -55,13 +55,11 @@ public function register_assets(): void {
if ( is_admin() ) {
return;
}
$theme = wp_get_theme();

// Do not add a versioning query param in assets URLs if minified
$version = $this->is_minified() ? null : $theme->get( 'Version' );
$theme = wp_get_theme();

// Js
$file = $this->is_minified() ? $this->get_min_file( 'js' ) : 'app.js';
Copy link
Copy Markdown

@cursor cursor Bot Apr 27, 2026

Choose a reason for hiding this comment

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

JS registration lacks empty-file guard unlike CSS paths

Medium Severity

get_min_file('js') returns an empty string when assets.json is absent or lacks the app.js key. In that case register_script is called with path 'dist/', registering a broken script URL pointing to a directory. The CSS equivalents (stylesheet_uri and login_stylesheet_uri) both have proper fallback chains (check for empty, try a hardcoded dist/app.css, then fall back to the original URI), but the JS registration has no guard or fallback at all. The old ternary guaranteed a non-empty 'app.js' fallback in dev mode; removing it without adding an equivalent guard leaves the JS path unprotected.

Additional Locations (1)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d2775dc. Configure here.

// JavaScript
$file = $this->get_min_file( 'js' );
$asset_data = $this->get_asset_data( $file );
$this->assets_tools->register_script(
'scripts',
Expand All @@ -80,40 +78,39 @@ public function register_assets(): void {
),
);

// CSS
wp_register_style( 'theme-style', get_stylesheet_uri(), [], $version );
// Styles
wp_register_style( 'theme-style', get_stylesheet_uri(), [], $theme->get( 'Version' ) );
}

/**
* Enqueue the scripts
*/
public function enqueue_scripts(): void {
// JS
// JavaScript
$this->assets_tools->enqueue_script( 'scripts' );
}

/**
* Enqueue the styles
*/
public function enqueue_styles(): void {
// CSS
// Styles
$this->assets_tools->enqueue_style( 'theme-style' );
}

/**
* The stylesheet uri based on the dev or not constant
* Point the theme stylesheet to the built CSS in `dist/` when `assets.json` is present.
*
* @param string $stylesheet_uri
* @param string $stylesheet_uri Default theme stylesheet URI.
*
* @return string
* @author Nicolas Juen
*/
public function stylesheet_uri( string $stylesheet_uri ): string {
if ( $this->is_minified() ) {
$file = $this->get_min_file( 'css' );
if ( ! empty( $file ) && file_exists( \get_theme_file_path( '/dist/' . $file ) ) ) {
return \get_theme_file_uri( '/dist/' . $file );
}
$file = $this->get_min_file( 'css' );

if ( ! empty( $file ) && file_exists( \get_theme_file_path( '/dist/' . $file ) ) ) {
return \get_theme_file_uri( '/dist/' . $file );
}

if ( file_exists( \get_theme_file_path( '/dist/app.css' ) ) ) {
Expand All @@ -124,9 +121,9 @@ public function stylesheet_uri( string $stylesheet_uri ): string {
}

/**
* Return JS/CSS .min file based on assets.json
* Return the compiled asset filename for a type from `assets.json`.
*
* @param string $type
* @param string $type Asset type key (e.g. `js`, `css`, `editor.js`).
*
* @return string
*/
Expand Down Expand Up @@ -185,7 +182,7 @@ public function get_min_file( string $type ): string {
* Asset data are produced by the webpack dependencies extraction plugin. They contain for each asset the list of
* dependencies use by the asset and a hash representing the current version of the asset.
*
* @param string $file The asset name including its extension, eg: app.js, app-min.js
* @param string $file The asset name including its extension, e.g. `app.js`.
*
* @return array{dependencies: string[], version:string} The asset data if available or an array with the default keys.
*/
Expand All @@ -202,37 +199,42 @@ public function get_asset_data( string $file ): array {
return $empty_asset_data;
}

if ( isset( $cache_data[ $file ] ) ) {
return $cache_data[ $file ];
$cache_key = $file;

if ( isset( $cache_data[ $cache_key ] ) ) {
return $cache_data[ $cache_key ];
}

$filename = strtok( $file, '.' );
$file = sprintf( '/dist/%s.asset.php', $filename );
if ( ! file_exists( \get_theme_file_path( $file ) ) ) {
$cache_data[ $file ] = $empty_asset_data;
return $cache_data[ $file ];
$filename = strtok( $file, '.' );
$asset_php = sprintf( '/dist/%s.asset.php', $filename );
if ( ! file_exists( \get_theme_file_path( $asset_php ) ) ) {
$cache_data[ $cache_key ] = $empty_asset_data;
return $cache_data[ $cache_key ];
}

$cache_data[ $file ] = require \get_theme_file_path( $file );
$cache_data[ $cache_key ] = require \get_theme_file_path( $asset_php );

return $cache_data[ $file ];
return $cache_data[ $cache_key ];
Comment thread
firestar300 marked this conversation as resolved.
}

/**
* Check if we are on minified environment.
* Point the login page stylesheet to the built CSS in `dist/` when available.
*
* @return bool
* @author Nicolas JUEN
* @param string $stylesheet_path Default path relative to the theme root (from `wp_login_page_theme_css`).
*
* @return string Path relative to the theme root.
*/
Comment thread
firestar300 marked this conversation as resolved.
public function is_minified(): bool {
return ( ! defined( 'SCRIPT_DEBUG' ) || SCRIPT_DEBUG === false );
}
public function login_stylesheet_uri( string $stylesheet_path = '' ): string {
$file = $this->get_min_file( 'login' );

/**
* Change login CSS URL
* @return string
*/
public function login_stylesheet_uri(): string {
return $this->is_minified() ? 'dist/' . $this->get_min_file( 'login' ) : 'dist/login.css';
if ( ! empty( $file ) && file_exists( \get_theme_file_path( '/dist/' . $file ) ) ) {
return 'dist/' . $file;
}

if ( file_exists( \get_theme_file_path( '/dist/login.css' ) ) ) {
return 'dist/login.css';
}

return $stylesheet_path;
}
}
4 changes: 2 additions & 2 deletions inc/Services/Editor.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ public function boot( Service_Container $container ): void {
* editor style
*/
private function style(): void {
$file = $this->assets->is_minified() ? $this->assets->get_min_file( 'editor.css' ) : 'editor.css';
$file = $this->assets->get_min_file( 'editor.css' );
Comment thread
firestar300 marked this conversation as resolved.

/**
* Do not enqueue a inexistant file on admin
Expand All @@ -77,7 +77,7 @@ private function style(): void {
* Editor script
*/
public function admin_editor_script(): void {
$file = $this->assets->is_minified() ? $this->assets->get_min_file( 'editor.js' ) : 'editor.js';
$file = $this->assets->get_min_file( 'editor.js' );
Copy link
Copy Markdown

@cursor cursor Bot Apr 27, 2026

Choose a reason for hiding this comment

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

Editor script guard bypassed when file is empty

Medium Severity

When get_min_file('editor.js') returns an empty string, $filepath becomes 'dist/'. The guard on line 83 uses file_exists(), which returns true for directories, so the check is bypassed and a broken script is registered at 'dist/'. The sibling style() method correctly uses is_file() for its guard, which returns false for directories. Previously the hardcoded 'editor.js' fallback in dev mode prevented $file from ever being empty; removing that fallback makes this guard ineffective.

Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit d2775dc. Configure here.

$filepath = 'dist/' . $file;

if ( ! file_exists( get_theme_file_path( $filepath ) ) ) {
Expand Down
Loading