fix: push manifest (#1680)

.changeset/olive-roses-judge.md

@@ -0,0 +1,5 @@
+---
+'preact-cli': patch
+---
+
+Fixed bug in push-manifest that would result in undefined entries

packages/cli/lib/lib/webpack/create-load-manifest.js

@@ -1,74 +1,67 @@
-const isESM = filename => /\.esm\.js$/.test(filename);
-const isMatch = (filename, condition) => isESM(filename) === condition;
+module.exports = (assets, namedChunkGroups) => {
+	/**
+	 * This is a mapping of generic/pre-build filenames to their postbuild output
+	 *
+	 * bundle.js -> bundle.29bec.esm.js
+	 * route-home.css -> styles/route-home.chunk.8aeee.css
+	 *
+	 * Even if a user alters the output name, we still have keys we can expect & rely on
+	 */
+	assets = JSON.parse(assets['asset-manifest.json']._value);
 
-module.exports = (assets, isESMBuild = false, namedChunkGroups) => {
-	let mainJs,
-		mainCss,
-		scripts = [],
-		styles = [];
-	for (let filename in assets) {
-		if (!/\.map$/.test(filename)) {
-			if (/^route-.*\.js$/.test(filename)) {
-				// both ESM & regular match here
-				isMatch(filename, isESMBuild) && scripts.push(filename);
-			} else if (/chunk\.(.+)\.css$/.test(filename)) {
-				styles.push(filename);
-			} else if (/^bundle(.+)\.css$/.test(filename)) {
-				mainCss = filename;
-			} else if (/^bundle(.+)\.js$/.test(filename)) {
-				// both ESM & regular bundles match here
-				if (isMatch(filename, isESMBuild)) {
-					mainJs = filename;
-				}
-			}
-		}
-	}
+	const mainJs = assets['bundle.js'];
+	const mainCss = assets['bundle.css'];
 
-	let defaults = {
-			[mainCss]: {
-				type: 'style',
-				weight: 1,
-			},
-			[mainJs]: {
-				type: 'script',
-				weight: 1,
-			},
+	const defaults = {
+			...(mainCss && {
+				[mainCss]: {
+					type: 'style',
+					weight: 1,
+				},
+			}),
+			...(mainJs && {
+				[mainJs]: {
+					type: 'script',
+					weight: 1,
+				},
+			}),
 		},
 		manifest = {
 			'/': defaults,
 		};
 
-	let path, css, obj;
-	scripts.forEach(filename => {
-		css = styles.find(asset => asset.startsWith(filename.replace(/\..*/, '')));
-		obj = Object.assign({}, defaults);
-		obj[filename] = { type: 'script', weight: 0.9 };
-		if (css) obj[css] = { type: 'style', weight: 0.9 };
-		path = filename
-			.replace(/route-/, '/')
-			.replace(/\.chunk(\.\w+)?(\.esm)?\.js$/, '')
-			.replace(/\/home/, '/');
-		if (namedChunkGroups) {
-			// async files to be loaded, generated by splitChunksPlugin
-			const asyncFiles =
-				namedChunkGroups.get(
-					filename.replace(/\.chunk(\.\w+)?(\.esm)?\.js$/, '')
-				) || {};
-			if (asyncFiles && asyncFiles.chunks) {
-				asyncFiles.chunks.forEach(asset => {
-					asset.files = asset.files || [];
-					asset.files.forEach(file => {
-						if (/\.css$/.test(file)) {
-							obj[file] = { type: 'style', weight: 0.9 };
-						} else if (/\.js$/.test(file)) {
-							obj[file] = { type: 'script', weight: 0.9 };
-						}
+	Object.keys(assets)
+		.filter(asset => /^route-.*\.js$/.test(asset))
+		.map(asset => asset.replace(/\.js$/, ''))
+		.forEach(route => {
+			const routeManifest = Object.assign({}, defaults);
+
+			const routeCss = assets[`${route}.css`];
+			const routeJs = assets[`${route}.js`];
+
+			routeManifest[routeJs] = { type: 'script', weight: 0.9 };
+			if (routeCss) routeManifest[routeCss] = { type: 'script', weight: 0.9 };
+
+			const path = route.replace(/^route-/, '/').replace(/^\/home/, '/');
+
+			if (namedChunkGroups) {
+				// async files to be loaded, generated by splitChunksPlugin
+				const asyncFiles = namedChunkGroups.get(route) || {};
+				if (asyncFiles && asyncFiles.chunks) {
+					asyncFiles.chunks.forEach(asset => {
+						asset.files = asset.files || [];
+						asset.files.forEach(file => {
+							if (/\.css$/.test(file)) {
+								routeManifest[file] = { type: 'style', weight: 0.9 };
+							} else if (/\.js$/.test(file)) {
+								routeManifest[file] = { type: 'script', weight: 0.9 };
+							}
+						});
 					});
-				});
+				}
 			}
-		}
-		manifest[path] = obj;
-	});
+			manifest[path] = routeManifest;
+		});
 
 	return manifest;
 };

packages/cli/lib/lib/webpack/push-manifest.js

@@ -1,30 +1,33 @@
+const webpack = require('webpack');
 const createLoadManifest = require('./create-load-manifest');
 
 module.exports = class PushManifestPlugin {
-	constructor(env = {}) {
-		this.isESMBuild_ = env.esm;
-	}
 	apply(compiler) {
-		compiler.hooks.emit.tap('PushManifestPlugin', compilation => {
-			const manifest = createLoadManifest(
-				compilation.assets,
-				this.isESMBuild_,
-				compilation.namedChunkGroups
-			);
+		compiler.hooks.emit.tap(
+			{
+				name: 'PushManifestPlugin',
+				stage: webpack.Compiler.PROCESS_ASSETS_STAGE_REPORT,
+			},
+			compilation => {
+				const manifest = createLoadManifest(
+					compilation.assets,
+					compilation.namedChunkGroups
+				);
 
-			let output = JSON.stringify(manifest);
-			compilation.assets['push-manifest.json'] = {
-				source() {
-					return output;
-				},
-				size() {
-					return output.length;
-				},
-			};
+				let output = JSON.stringify(manifest);
+				compilation.assets['push-manifest.json'] = {
+					source() {
+						return output;
+					},
+					size() {
+						return output.length;
+					},
+				};
 
-			return compilation;
+				return compilation;
 
-			// callback();
-		});
+				// callback();
+			}
+		);
 	}
 };

packages/cli/lib/lib/webpack/render-html-plugin.js

@@ -89,7 +89,7 @@ module.exports = async function (config) {
 				if (assets['push-manifest.json']) {
 					return JSON.parse(assets['push-manifest.json'].source());
 				}
-				return createLoadManifest(assets, config.esm, namedChunkGroups);
+				return createLoadManifest(assets, namedChunkGroups);
 			},
 			config,
 			url,

packages/cli/lib/lib/webpack/webpack-base-config.js

@@ -12,7 +12,8 @@ const ReplacePlugin = require('webpack-plugin-replace');
 const ForkTsCheckerWebpackPlugin = require('fork-ts-checker-webpack-plugin');
 const createBabelConfig = require('../babel-config');
 const loadPostcssConfig = require('postcss-load-config');
-const PnpWebpackPlugin = require(`pnp-webpack-plugin`);
+const PnpWebpackPlugin = require('pnp-webpack-plugin');
+const { WebpackManifestPlugin } = require('webpack-manifest-plugin');
 
 function readJson(file) {
 	try {
@@ -330,6 +331,15 @@ module.exports = function createBaseConfig(env) {
 				summary: false,
 				clear: true,
 			}),
+			new WebpackManifestPlugin({
+				fileName: 'asset-manifest.json',
+				assetHookStage: webpack.Compiler.PROCESS_ASSETS_STAGE_ANALYSE,
+				// TODO: Remove this next breaking change and use the full filepath from this manifest
+				// when referring to built assets, i.e.:
+				// https://github.com/preactjs/preact-cli/blob/master/packages/cli/lib/resources/head-end.ejs#L1
+				// This is just to avoid any potentially breaking changes for right now.
+				publicPath: '',
+			}),
 			...(tsconfig
 				? [
 						new ForkTsCheckerWebpackPlugin({

packages/cli/lib/lib/webpack/webpack-client-config.js

@@ -146,7 +146,7 @@ async function clientConfig(env) {
 				'process.env.ADD_SW': env.sw,
 				'process.env.PRERENDER': env.prerender,
 			}),
-			new PushManifestPlugin(env),
+			new PushManifestPlugin(),
 			...(await renderHTMLPlugin(env)),
 			...getBabelEsmPlugin(env),
 			copyPatterns.length !== 0 &&

packages/cli/package.json

@@ -140,6 +140,7 @@
     "webpack-bundle-analyzer": "^4.4.2",
     "webpack-dev-server": "^4.7.3",
     "webpack-fix-style-only-entries": "^0.6.1",
+    "webpack-manifest-plugin": "^4.1.1",
     "webpack-merge": "^5.3.0",
     "webpack-plugin-replace": "^1.2.0",
     "which": "^2.0.2",

packages/cli/tests/build.test.js

@@ -1,5 +1,5 @@
 const { join } = require('path');
-const { access, readdir, readFile } = require('fs').promises;
+const { access, readdir, readFile, writeFile } = require('fs').promises;
 const looksLike = require('html-looks-like');
 const { create, build } = require('./lib/cli');
 const { snapshot } = require('./lib/utils');
@@ -245,13 +245,68 @@ describe('preact build', () => {
 		mockExit.mockRestore();
 	});
 
-	it('should produce correct push-manifest', async () => {
-		let dir = await create('default');
+	describe('Push manifest plugin', () => {
+		it('should produce correct default `push-manifest.json`', async () => {
+			let dir = await create('default');
 
-		await build(dir);
-		const manifest = await readFile(`${dir}/build/push-manifest.json`, 'utf8');
-		expect(manifest).toEqual(
-			expect.stringMatching(getRegExpFromMarkup(images.pushManifest))
-		);
+			await build(dir);
+			const manifest = await readFile(
+				`${dir}/build/push-manifest.json`,
+				'utf8'
+			);
+			expect(manifest).toEqual(
+				expect.stringMatching(getRegExpFromMarkup(images.pushManifest))
+			);
+		});
+
+		it('should produce correct default `push-manifest.json` with esm', async () => {
+			let dir = await create('default');
+
+			await build(dir, { esm: true });
+			const manifest = await readFile(
+				`${dir}/build/push-manifest.json`,
+				'utf8'
+			);
+			expect(manifest).toEqual(
+				expect.stringMatching(getRegExpFromMarkup(images.pushManifestEsm))
+			);
+		});
+
+		it('should produce correct `push-manifest.json` when expected values are missing', async () => {
+			// In this subject, there is no source CSS which means no CSS asset is output.
+			// In the past, this would result in `"undefined": { type: "style" ... }` being added to the manifest.
+			let dir = await subject('custom-webpack');
+			await build(dir);
+			const manifest = await readFile(
+				`${dir}/build/push-manifest.json`,
+				'utf8'
+			);
+			expect(manifest).not.toMatch(/"undefined"/);
+		});
+
+		// Issue #1675
+		it('should produce correct `push-manifest.json` when user configures output filenames', async () => {
+			let dir = await subject('custom-webpack');
+
+			const config = await readFile(`${dir}/preact.config.js`, 'utf8');
+			await writeFile(
+				`${dir}/preact.config.js`,
+				config.replace(
+					"config.output.filename = '[name].js'",
+					"config.output.filename = 'scripts/[name].js'"
+				)
+			);
+
+			await build(dir, { prerender: false });
+			const manifest = await readFile(
+				`${dir}/build/push-manifest.json`,
+				'utf8'
+			);
+			expect(manifest).toEqual(
+				expect.stringMatching(
+					getRegExpFromMarkup(images.pushManifestAlteredFilenames)
+				)
+			);
+		});
 	});
 });