diff --git a/.changeset/olive-roses-judge.md b/.changeset/olive-roses-judge.md new file mode 100644 index 000000000..50de379f3 --- /dev/null +++ b/.changeset/olive-roses-judge.md @@ -0,0 +1,5 @@ +--- +'preact-cli': patch +--- + +Fixed bug in push-manifest that would result in undefined entries diff --git a/packages/cli/lib/lib/webpack/create-load-manifest.js b/packages/cli/lib/lib/webpack/create-load-manifest.js index da18f2609..a66d87e38 100644 --- a/packages/cli/lib/lib/webpack/create-load-manifest.js +++ b/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; }; diff --git a/packages/cli/lib/lib/webpack/push-manifest.js b/packages/cli/lib/lib/webpack/push-manifest.js index 968864432..d45c9a675 100644 --- a/packages/cli/lib/lib/webpack/push-manifest.js +++ b/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(); + } + ); } }; diff --git a/packages/cli/lib/lib/webpack/render-html-plugin.js b/packages/cli/lib/lib/webpack/render-html-plugin.js index 1c1fa3cb6..00114ac8f 100644 --- a/packages/cli/lib/lib/webpack/render-html-plugin.js +++ b/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, diff --git a/packages/cli/lib/lib/webpack/webpack-base-config.js b/packages/cli/lib/lib/webpack/webpack-base-config.js index f10adcf4c..b56c30aee 100644 --- a/packages/cli/lib/lib/webpack/webpack-base-config.js +++ b/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({ diff --git a/packages/cli/lib/lib/webpack/webpack-client-config.js b/packages/cli/lib/lib/webpack/webpack-client-config.js index 6e2c45e7c..646e76566 100644 --- a/packages/cli/lib/lib/webpack/webpack-client-config.js +++ b/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 && diff --git a/packages/cli/package.json b/packages/cli/package.json index dbb2dc29b..9288bbf27 100644 --- a/packages/cli/package.json +++ b/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", diff --git a/packages/cli/tests/build.test.js b/packages/cli/tests/build.test.js index 76934f1ca..14e258ddc 100644 --- a/packages/cli/tests/build.test.js +++ b/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) + ) + ); + }); }); }); diff --git a/packages/cli/tests/images/build.js b/packages/cli/tests/images/build.js index 03a2b5a55..ab4371d7a 100644 --- a/packages/cli/tests/images/build.js +++ b/packages/cli/tests/images/build.js @@ -15,6 +15,7 @@ exports.default = Object.assign({}, common, { 'ssr-build/ssr-bundle.aaacf.css.map': 2070, 'ssr-build/ssr-bundle.js': 11937, 'ssr-build/ssr-bundle.js.map': 32557, + 'ssr-build/asset-manifest.json': 178, 'bundle.2da73.css': 901, 'bundle.44866.js': 21429, 'bundle.44866.js.map': 111801, @@ -23,6 +24,7 @@ exports.default = Object.assign({}, common, { 'manifest.json': 455, 'preact_prerender_data.json': 11, 'push-manifest.json': 450, + 'asset-manifest.json': 1074, 'route-home.chunk.bcb8a.css': 58, 'route-home.chunk.3cec8.js': 327, 'route-home.chunk.3cec8.js.map': 483, @@ -42,6 +44,7 @@ exports['default-esm'] = Object.assign({}, exports.default, { 'route-profile.chunk.*.esm.js.map': 15392, 'index.html': 2193, 'push-manifest.json': 466, + 'asset-manifest.json': 1106, }); exports.sass = ` @@ -253,3 +256,55 @@ exports.pushManifest = ` } } `; + +exports.pushManifestEsm = ` +{ + "/":{ + "bundle.\\w{5}.css":{ + "type":"style", + "weight":1 + }, + "bundle.\\w{5}.esm.js":{ + "type":"script", + "weight":1 + }, + "route-home.chunk.\\w{5}.esm.js":{ + "type":"script", + "weight":0.9 + }, + "route-home.chunk.\\w{5}.css":{ + "type":"style", + "weight":0.9 + } + }, + "/profile":{ + "bundle.\\w{5}.css":{ + "type":"style", + "weight":1 + }, + "bundle.\\w{5}.esm.js":{ + "type":"script", + "weight":1 + }, + "route-profile.chunk.\\w{5}.esm.js":{ + "type":"script", + "weight":0.9 + }, + "route-profile.chunk.\\w{5}.css":{ + "type":"style", + "weight":0.9 + } + } +} +`; + +exports.pushManifestAlteredFilenames = ` +{ + "/":{ + "scripts/bundle.js":{ + "type":"script", + "weight":1 + } + } +} +`; diff --git a/yarn.lock b/yarn.lock index f56ff40b0..9f10b0738 100644 --- a/yarn.lock +++ b/yarn.lock @@ -13481,7 +13481,7 @@ sorted-union-stream@~2.1.3: from2 "^1.3.0" stream-iterate "^1.1.0" -source-list-map@^2.0.0: +source-list-map@^2.0.0, source-list-map@^2.0.1: version "2.0.1" resolved "https://registry.yarnpkg.com/source-list-map/-/source-list-map-2.0.1.tgz#3993bd873bfc48479cca9ea3a547835c7c154b34" integrity sha512-qnQ7gVMxGNxsiL4lEuJwe/To8UnK7fAnmbGEEH8RpLouuKbeEm0lhbQVFIrNSuB+G7tVrAlVsZgETT5nljf+Iw== @@ -14087,6 +14087,11 @@ tapable@^1.0.0, tapable@^1.1.3: resolved "https://registry.yarnpkg.com/tapable/-/tapable-1.1.3.tgz#a1fccc06b58db61fd7a45da2da44f5f3a3e67ba2" integrity sha512-4WK/bYZmj8xLr+HUCODHGF1ZFzsYffasLUgEiMBY4fgtltdO6B4WJtlSbPaDTLpYTcGVwM2qLnFTICEcNxs3kA== +tapable@^2.0.0: + version "2.2.1" + resolved "https://registry.yarnpkg.com/tapable/-/tapable-2.2.1.tgz#1967a73ef4060a82f12ab96af86d52fdb76eeca0" + integrity sha512-GNzQvQTOIP6RyTfE2Qxb8ZVlNmw0n88vp1szwWRimP02mnTsx3Wtn5qRdqY9w2XduFNUgvOwhNnQsjwCp+kqaQ== + tar-fs@^2.0.0: version "2.1.1" resolved "https://registry.yarnpkg.com/tar-fs/-/tar-fs-2.1.1.tgz#489a15ab85f1f0befabb370b7de4f9eb5cbe8784" @@ -15062,6 +15067,14 @@ webpack-log@^2.0.0: ansi-colors "^3.0.0" uuid "^3.3.2" +webpack-manifest-plugin@^4.1.1: + version "4.1.1" + resolved "https://registry.yarnpkg.com/webpack-manifest-plugin/-/webpack-manifest-plugin-4.1.1.tgz#10f8dbf4714ff93a215d5a45bcc416d80506f94f" + integrity sha512-YXUAwxtfKIJIKkhg03MKuiFAD72PlrqCiwdwO4VEXdRO5V0ORCNwaOwAZawPZalCbmH9kBDmXnNeQOw+BIEiow== + dependencies: + tapable "^2.0.0" + webpack-sources "^2.2.0" + webpack-merge@^5.3.0: version "5.7.3" resolved "https://registry.yarnpkg.com/webpack-merge/-/webpack-merge-5.7.3.tgz#2a0754e1877a25a8bbab3d2475ca70a052708213" @@ -15083,6 +15096,14 @@ webpack-sources@^1.1.0, webpack-sources@^1.3.0, webpack-sources@^1.4.0, webpack- source-list-map "^2.0.0" source-map "~0.6.1" +webpack-sources@^2.2.0: + version "2.3.1" + resolved "https://registry.yarnpkg.com/webpack-sources/-/webpack-sources-2.3.1.tgz#570de0af163949fe272233c2cefe1b56f74511fd" + integrity sha512-y9EI9AO42JjEcrTJFOYmVywVZdKVUfOvDUPsJea5GIr1JOEGFVqwlY2K098fFoIjOkDzHn2AjRvM8dsBZu+gCA== + dependencies: + source-list-map "^2.0.1" + source-map "^0.6.1" + webpack@^4.38.0: version "4.46.0" resolved "https://registry.yarnpkg.com/webpack/-/webpack-4.46.0.tgz#bf9b4404ea20a073605e0a011d188d77cb6ad542"