Assets: Fix the order of the chunk files (2nd try) (#96100)

* Assets: Fix the order of the chunk files * Get rid of fallback * Fix tests * Update copy * Simplify the info in the assets.json file * Unify getFilesForEntrypoint and getFilesForChunk * Fix server unit tests * Exclude hot updates and sourcemaps from assets.json served by server * Remove gzip-size npm dependency --------- Co-authored-by: Jarda Snajdr <[email protected]>
Automattic · Feb 3, 2025 · c0cb77b · c0cb77b
1 parent afd11b8
commit c0cb77b
Show file tree

Hide file tree

Showing 7 changed files with 62 additions and 184 deletions.
diff --git a/bin/loader-stats.js b/bin/loader-stats.js
diff --git a/build-tools/webpack/assets-writer-plugin.js b/build-tools/webpack/assets-writer-plugin.js
@@ -43,50 +43,41 @@ Object.assign( AssetsWriter.prototype, {
 				reasons: false,
 			} );
 
+			function fixupPath( f ) {
+				return path.join( stats.publicPath, f );
+			}
+
+			// Exclude hot update files (info.hotModuleReplacement) and source maps (info.development)
+			function isDevelopmentAsset( name ) {
+				const asset = stats.assets.find( ( a ) => a.name === name );
+				if ( ! asset ) {
+					return false;
+				}
+
+				return asset.info.hotModuleReplacement || asset.info.development;
+			}
+
 			const statsToOutput = {};
-			statsToOutput.publicPath = stats.publicPath;
-			statsToOutput.manifests = {};
 
+			statsToOutput.manifests = {};
 			for ( const name in stats.assetsByChunkName ) {
 				// make the manifest inlineable
 				if ( String( name ).startsWith( this.options.runtimeChunk ) ) {
-					// Runtime chunk will have two files due ExtractManifestPlugin. Both need to be inlined.
-					// When we build with sourcemaps, we'll have another two extra files.
-					// Remove the sourcemap from the list and just take the js assets.
+					// Runtime chunk will have two files due to ExtractManifestPlugin. Both need to be inlined.
 					statsToOutput.manifests = stats.assetsByChunkName[ name ]
-						.filter( ( asset ) => ! asset.endsWith( '.map' ) )
+						.filter( ( asset ) => ! isDevelopmentAsset( asset ) ) // exclude hot updates and sourcemaps
 						.map( ( asset ) => compilation.assets[ asset ].source() );
 				}
 			}
 
-			function fixupPath( f ) {
-				return path.join( stats.publicPath, f );
-			}
-
-			statsToOutput.entrypoints = _.mapValues( stats.entrypoints, ( entry ) => ( {
-				chunks: _.reject( entry.chunks, ( chunk ) => {
-					String( chunk ).startsWith( this.options.runtimeChunk );
-				} ),
-				assets: _.reject(
-					entry.assets,
-					( asset ) =>
-						asset.name.startsWith( this.options.manifestFile ) ||
-						asset.name.startsWith( this.options.runtimeFile )
-				).map( ( asset ) => fixupPath( asset.name ) ),
-			} ) );
-
-			statsToOutput.assetsByChunkName = _.mapValues( stats.assetsByChunkName, ( asset ) => {
-				const assets = Array.isArray( asset ) ? asset : [ asset ];
-				return assets.map( fixupPath );
-			} );
-
-			statsToOutput.chunks = stats.chunks.map( ( chunk ) =>
-				Object.assign( {}, chunk, {
-					files: chunk.files.map( fixupPath ),
-					siblings: _.reject( chunk.siblings, ( sibling ) =>
-						String( sibling ).startsWith( this.options.runtimeChunk )
-					),
-				} )
+			statsToOutput.assets = _.mapValues( stats.namedChunkGroups, ( { assets } ) =>
+				_.reject(
+					assets,
+					( { name } ) =>
+						isDevelopmentAsset( name ) ||
+						name.startsWith( this.options.manifestFile ) ||
+						name.startsWith( this.options.runtimeFile )
+				).map( ( { name } ) => fixupPath( name ) )
 			);
 
 			self.outputStream.end( JSON.stringify( statsToOutput, null, '\t' ), callback );

diff --git a/client/server/middleware/assets.js b/client/server/middleware/assets.js
@@ -1,7 +1,7 @@
 import { readFile } from 'fs/promises';
 import path from 'path';
 import asyncHandler from 'express-async-handler';
-import { defaults, groupBy, flatten } from 'lodash';
+import { defaults, groupBy } from 'lodash';
 
 const ASSETS_PATH = path.resolve( __dirname, '../../../build' );
 const ASSETS_FILE = path.join( ASSETS_PATH, `assets.json` );
@@ -18,11 +18,6 @@ const getAssetType = ( asset ) => {
 	return 'js';
 };
 
-const getChunkByName = ( assets, chunkName ) =>
-	assets.chunks.find( ( chunk ) => chunk.names.some( ( name ) => name === chunkName ) );
-
-const getChunkById = ( assets, chunkId ) => assets.chunks.find( ( chunk ) => chunk.id === chunkId );
-
 const groupAssetsByType = ( assets ) => defaults( groupBy( assets, getAssetType ), EMPTY_ASSETS );
 
 export default () => {
@@ -39,30 +34,17 @@ export default () => {
 
 		req.getAssets = () => assets;
 
-		req.getFilesForEntrypoint = ( name ) => {
-			const entrypointAssets = assets.entrypoints[ name ].assets.filter(
-				( asset ) => ! asset.startsWith( 'manifest' )
-			);
-			return groupAssetsByType( entrypointAssets );
-		};
-
-		req.getFilesForChunk = ( chunkName ) => {
-			const chunk = getChunkByName( assets, chunkName );
-
-			if ( ! chunk ) {
-				console.warn( 'cannot find the chunk ' + chunkName );
-				console.warn( 'available chunks:' );
-				assets.chunks.forEach( ( c ) => {
-					console.log( '    ' + c.id + ': ' + c.names.join( ',' ) );
-				} );
+		req.getFilesForChunkGroup = ( name ) => {
+			const chunkGroupAssets = assets.assets[ name ];
+			if ( ! chunkGroupAssets ) {
+				console.warn( 'cannot find chunk group ' + chunkGroupAssets );
+				console.warn( 'available chunk groups:' );
+				for ( const availName of Object.keys( assets.assets ) ) {
+					console.log( '    ' + availName );
+				}
 				return EMPTY_ASSETS;
 			}
-
-			const allTheFiles = chunk.files.concat(
-				flatten( chunk.siblings.map( ( sibling ) => getChunkById( assets, sibling ).files ) )
-			);
-
-			return groupAssetsByType( allTheFiles );
+			return groupAssetsByType( chunkGroupAssets );
 		};
 
 		req.getEmptyAssets = () => EMPTY_ASSETS;

diff --git a/client/server/pages/index.js b/client/server/pages/index.js
@@ -163,8 +163,8 @@ function getDefaultContext( request, response, entrypoint = 'entry-main' ) {
 
 	const flags = ( request.query.flags || '' ).split( ',' );
 
-	performanceMark( request.context, 'getFilesForEntrypoint', true );
-	const entrypointFiles = request.getFilesForEntrypoint( entrypoint );
+	performanceMark( request.context, 'getFilesForChunkGroup', true );
+	const entrypointFiles = request.getFilesForChunkGroup( entrypoint );
 
 	performanceMark( request.context, 'getAssets', true );
 	const manifests = request.getAssets().manifests;
@@ -594,7 +594,7 @@ const setUpSectionContext = ( section, entrypoint ) => ( req, res, next ) => {
 	req.context.sectionName = section.name;
 
 	if ( ! entrypoint ) {
-		req.context.chunkFiles = req.getFilesForChunk( section.name );
+		req.context.chunkFiles = req.getFilesForChunkGroup( section.name );
 	} else {
 		req.context.chunkFiles = req.getEmptyAssets();
 	}
@@ -619,7 +619,7 @@ const render404 =
 	( entrypoint = 'entry-main' ) =>
 	( req, res ) => {
 		const ctx = {
-			entrypoint: req.getFilesForEntrypoint( entrypoint ),
+			entrypoint: req.getFilesForChunkGroup( entrypoint ),
 		};
 
 		res.status( 404 ).send( renderJsx( '404', ctx ) );
@@ -647,7 +647,7 @@ const renderServerError =
 		}
 
 		const ctx = {
-			entrypoint: req.getFilesForEntrypoint( entrypoint ),
+			entrypoint: req.getFilesForChunkGroup( entrypoint ),
 		};
 
 		res.status( err.status || 500 ).send( renderJsx( '500', ctx ) );
@@ -791,7 +791,7 @@ function wpcomPages( app ) {
 		const { from } = req.query;
 		const redirectLocation = from && validateRedirect( req, from ) ? from : '/';
 
-		req.context.entrypoint = req.getFilesForEntrypoint( 'entry-browsehappy' );
+		req.context.entrypoint = req.getFilesForChunkGroup( 'entry-browsehappy' );
 		req.context.from = redirectLocation;
 
 		res.send( renderJsx( 'browsehappy', req.context ) );

diff --git a/client/server/pages/test/index.js b/client/server/pages/test/index.js
@@ -198,41 +198,28 @@ const buildApp = ( environment ) => {
 					'/* webpack manifest for evergreen */',
 					'/* webpack runtime for evergreen */',
 				],
-				entrypoints: {
-					'entry-main': {
-						assets: [ ...assetsList ],
-					},
-					'entry-domains-landing': {
-						assets: [
-							...assetsList.map( ( asset ) =>
-								asset.replace( 'entry-main', 'entry-domains-landing' )
-							),
-						],
-					},
-					'entry-gutenboarding': {
-						assets: [
-							...assetsList.map( ( asset ) =>
-								asset.replace( 'entry-main', 'entry-gutenboarding' )
-							),
-						],
-					},
-					'entry-browsehappy': {
-						assets: [
-							...assetsList.map( ( asset ) => asset.replace( 'entry-main', 'entry-browsehappy' ) ),
-						],
-					},
+				assets: {
+					'entry-main': assetsList,
+					'entry-domains-landing': assetsList.map( ( asset ) =>
+						asset.replace( 'entry-main', 'entry-domains-landing' )
+					),
+					'entry-gutenboarding': assetsList.map( ( asset ) =>
+						asset.replace( 'entry-main', 'entry-gutenboarding' )
+					),
+					'entry-browsehappy': assetsList.map( ( asset ) =>
+						asset.replace( 'entry-main', 'entry-browsehappy' )
+					),
+					...Object.fromEntries(
+						sections.map( ( section ) => [
+							section.name,
+							[
+								`/calypso/evergreen/${ section.name }.js`,
+								`/calypso/evergreen/${ section.name }.css`,
+								`/calypso/evergreen/${ section.name }.rtl.css`,
+							],
+						] )
+					),
 				},
-				chunks: [
-					...sections.map( ( section ) => ( {
-						names: [ section.name ],
-						files: [
-							`/calypso/evergreen/${ section.name }.js`,
-							`/calypso/evergreen/${ section.name }.css`,
-							`/calypso/evergreen/${ section.name }.rtl.css`,
-						],
-						siblings: [],
-					} ) ),
-				],
 			};
 			mockFs( {
 				'./build/assets.json': JSON.stringify( assets ),

diff --git a/package.json b/package.json
@@ -285,7 +285,6 @@
 		"gettext-parser": "^4.0.3",
 		"glob": "^7.1.6",
 		"globby": "^10.0.2",
-		"gzip-size": "^6.0.0",
 		"husky": "^7.0.4",
 		"jest": "^29.7.0",
 		"jest-canvas-mock": "^2.5.2",

diff --git a/yarn.lock b/yarn.lock
@@ -35082,7 +35082,6 @@ __metadata:
     gettext-parser: "npm:^4.0.3"
     glob: "npm:^7.1.6"
     globby: "npm:^10.0.2"
-    gzip-size: "npm:^6.0.0"
     http-proxy-middleware: "npm:^3.0.0"
     husky: "npm:^7.0.4"
     i18n-calypso: "workspace:^"