From a55bd6fd0e6e59fb4e3724464bcfc05af3060150 Mon Sep 17 00:00:00 2001 From: Katerina Koukiou Date: Fri, 3 Feb 2023 18:43:21 +0100 Subject: [PATCH] maint: Drop webpack in favor of esbuild Esbuild performance: $ time npm run build > build > node build.js real 0m7.747s user 0m12.328s sys 0m0.508s Webpack performance: [kkoukiou@sequioa cockpit-podman]$ time npm run build > build > webpack Webpack compiled successfully real 0m26.832s user 0m47.778s sys 0m1.640s For eslint integration there are two existing plugins [1], [2] but none can be used because of unsatisfied peer dependency version of esbuild. Let's just use our own eslint plugin for now. [1] https://github.com/to-codando/esbuild-plugin-linter/issues/1 [2] https://github.com/robinloeffel/esbuild-plugin-eslint/issues/5 TODO: [ ] Bundle sizes are much bigger for esbuild as files are not compressed, consider if this is needed [ ] fail on warnings eslint [0] [0] https://github.com/eslint/eslint/issues/16804 --- HACKING.md | 2 +- Makefile | 9 +- build.js | 67 +++++++++++++ esbuild-cockpit-po-plugin.js | 99 +++++++++++++++++++ esbuild-eslint-plugin.js | 28 ++++++ esbuild-rsync-plugin.js | 24 +++++ esbuild-stylelint-plugin.js | 31 ++++++ node_modules | 2 +- package.json | 25 ++--- packaging/cockpit-podman.spec.in | 2 +- webpack.config.js | 164 ------------------------------- 11 files changed, 263 insertions(+), 190 deletions(-) create mode 100644 build.js create mode 100644 esbuild-cockpit-po-plugin.js create mode 100644 esbuild-eslint-plugin.js create mode 100644 esbuild-rsync-plugin.js create mode 100644 esbuild-stylelint-plugin.js delete mode 100644 webpack.config.js diff --git a/HACKING.md b/HACKING.md index 5a3e2a8c3..5470f7659 100644 --- a/HACKING.md +++ b/HACKING.md @@ -19,7 +19,7 @@ After changing the code and running `make` again, reload the Cockpit page in your browser. You can also use -[watch mode](https://webpack.js.org/guides/development/#using-watch-mode) to +[watch mode](https://esbuild.github.io/api/#watch) to automatically update the bundle on every code change with $ make watch diff --git a/Makefile b/Makefile index 5833f1488..3dc203889 100644 --- a/Makefile +++ b/Makefile @@ -91,11 +91,12 @@ packaging/arch/PKGBUILD: packaging/arch/PKGBUILD.in packaging/debian/changelog: packaging/debian/changelog.in sed 's/VERSION/$(VERSION)/' $< > $@ -$(DIST_TEST): $(COCKPIT_REPO_STAMP) $(shell find src/ -type f) package.json webpack.config.js - $(MAKE) package-lock.json && NODE_ENV=$(NODE_ENV) node_modules/.bin/webpack - # In development mode terser does not run and so no LICENSE.txt.gz is generated, so we explictly create it as it is required for building rpm's +$(DIST_TEST): $(COCKPIT_REPO_STAMP) $(shell find src/ -type f) package.json build.js + $(MAKE) package-lock.json && NODE_ENV=$(NODE_ENV) node build.js + # In development mode minification does not run and so no LICENSE.txt.gz is generated, so we explictly create it as it is required for building rpm's if [ "$$NODE_ENV" = "development" ]; then \ - gzip dist/index.js.LICENSE.txt.gz; \ + touch dist/index.js.LEGAL.txt; \ + touch dist/index.css.LEGAL.txt; \ fi watch: diff --git a/build.js b/build.js new file mode 100644 index 000000000..dd11d93bc --- /dev/null +++ b/build.js @@ -0,0 +1,67 @@ +import copy from 'esbuild-plugin-copy'; +import esbuild from "esbuild"; +import fs from "fs"; +import path from "path"; +import { cockpitPoPlugin } from './esbuild-cockpit-po-plugin.js'; +import { cockpitRsyncPlugin } from './esbuild-rsync-plugin.js'; +import { eslintPlugin } from './esbuild-eslint-plugin.js'; +import { sassPlugin } from 'esbuild-sass-plugin'; +import { stylelintPlugin } from './esbuild-stylelint-plugin.js'; + +const production = process.env.NODE_ENV === 'production'; +/* Default to disable csslint for faster production builds */ +const stylelint = process.env.STYLELINT ? (process.env.STYLELINT !== '0') : !production; +/* List of directories to use when resolving import statements */ +const nodePaths=['pkg/lib'] + +const context = await esbuild.context({ + ... !production ? { sourcemap: "external" } : {}, + bundle: true, + entryPoints: ["./src/index.js"], + external: ['*.woff', '*.woff2', '*.jpg', '*.svg', '../../assets*'], // Allow external font files which live in ../../static/fonts + legalComments: 'external', // Move all legal comments to a .LEGAL.txt file + loader: { ".js": "jsx" }, + minify: production, + nodePaths, + outdir: "./dist", + target: ['es2020'], + plugins: [ + ... stylelint ? [stylelintPlugin()] : [], + ... process.env.ESLINT !== '0' ? [eslintPlugin] : [], + // Esbuild will only copy assets that are explicitly imported and used + // in the code. This is a problem for index.html and manifest.json which are not imported + copy({ + assets: [ + { from: ['./src/manifest.json'], to: [ './manifest.json' ] }, + { from: ['./src/index.html'], to: ['./index.html'] }, + ] + }), + sassPlugin({ + loadPaths: [...nodePaths, 'node_modules'], quietDeps: true, + async transform(source, resolveDir, path) { + if (path.includes('patternfly-4-cockpit.scss')) { + return source + .replace(/url.*patternfly-icons-fake-path.*;/g, 'url("../base1/fonts/patternfly.woff") format("woff");') + .replace(/@font-face[^}]*patternfly-fonts-fake-path[^}]*}/g, ''); + } + return source; + } + }), + cockpitPoPlugin, + ... process.env.RSYNC ? [cockpitRsyncPlugin()] : [], + ] +}) + +// Manually do an incremental build +const result = await context.rebuild() + +/* development options for faster iteration */ +const watchMode = process.env.ESBUILD_WATCH === "true" || false; +if(watchMode) { + console.log("Running in watch mode"); + // Enable watch mode + await context.watch() +} +else { + context.dispose(); +} diff --git a/esbuild-cockpit-po-plugin.js b/esbuild-cockpit-po-plugin.js new file mode 100644 index 000000000..0c7dfac1f --- /dev/null +++ b/esbuild-cockpit-po-plugin.js @@ -0,0 +1,99 @@ +import fs from "fs"; +import glob from "glob"; +import path from "path"; +import Jed from "jed"; +import gettext_parser from "gettext-parser"; + +const srcdir = process.env.SRCDIR || "./"; + +function get_po_files() { + try { + const linguas_file = path.resolve(srcdir, "po/LINGUAS"); + const linguas = fs.readFileSync(linguas_file, 'utf8').match(/\S+/g); + return linguas.map(lang => path.resolve(srcdir, 'po', lang + '.po')); + } catch (error) { + if (error.code !== 'ENOENT') { + throw error; + } + + /* No LINGUAS file? Fall back to globbing. + * Note: we won't detect .po files being added in this case. + */ + return glob.sync(path.resolve(srcdir, 'po/*.po')); + } +} + +function get_plural_expr(statement) { + try { + /* Check that the plural forms isn't being sneaky since we build a function here */ + Jed.PF.parse(statement); + } catch (ex) { + console.error("bad plural forms: " + ex.message); + process.exit(1); + } + + const expr = statement.replace(/nplurals=[1-9]; plural=([^;]*);?$/, '(n) => $1'); + if (expr === statement) { + console.error("bad plural forms: " + statement); + process.exit(1); + } + + return expr; +} + +function buildFile(po_file, outdir, subdir='', wrapper='cockpit.locale(PO_DATA)') { + return new Promise((resolve, reject) => { + const parsed = gettext_parser.po.parse(fs.readFileSync(po_file), 'utf8'); + delete parsed.translations[""][""]; // second header copy + + const rtl_langs = ["ar", "fa", "he", "ur"]; + const dir = rtl_langs.includes(parsed.headers.language) ? "rtl" : "ltr"; + + // cockpit.js only looks at "plural-forms" and "language" + const chunks = [ + '{\n', + ' "": {\n', + ` "plural-forms": ${get_plural_expr(parsed.headers['plural-forms'])},\n`, + ` "language": "${parsed.headers.language}",\n`, + ` "language-direction": "${dir}"\n`, + ' }' + ]; + for (const [msgctxt, context] of Object.entries(parsed.translations)) { + const context_prefix = msgctxt ? msgctxt + '\u0004' : ''; /* for cockpit.ngettext */ + + for (const [msgid, translation] of Object.entries(context)) { + /* Only include msgids which appear in this source directory */ + const references = translation.comments.reference.split(/\s/); + if (!references.some(str => str.startsWith(`pkg/${subdir}`) || str.startsWith('src'))) + continue; + + if (translation.comments.flag && translation.comments.flag.match(/\bfuzzy\b/)) + continue; + + const key = JSON.stringify(context_prefix + msgid); + // cockpit.js always ignores the first item + chunks.push(`,\n ${key}: [\n null`); + for (const str of translation.msgstr) { + chunks.push(',\n ' + JSON.stringify(str)); + } + chunks.push('\n ]'); + } + } + chunks.push('\n}'); + + const output = wrapper.replace('PO_DATA', chunks.join('')) + '\n'; + + const lang = path.basename(po_file).slice(0, -3); + fs.writeFileSync(path.resolve(outdir, subdir + 'po.' + lang + '.js'), output); + return resolve(); + }); +} + +export const cockpitPoPlugin = { + name: 'cockpitPoPlugin', + setup(build) { + build.onEnd(async () => { + await Promise.all(get_po_files().map(f => buildFile(f, build.initialOptions.outdir))); + }); + }, +} diff --git a/esbuild-eslint-plugin.js b/esbuild-eslint-plugin.js new file mode 100644 index 000000000..654e8d7dd --- /dev/null +++ b/esbuild-eslint-plugin.js @@ -0,0 +1,28 @@ +// FIXME: replace with plugin from npmjs if possible +// Candidate [1] https://github.com/to-codando/esbuild-plugin-linter/issues/1 +// Candidate [2] https://github.com/robinloeffel/esbuild-plugin-eslint/issues/5 + +import { ESLint } from 'eslint'; + +export const eslintPlugin = { + name: 'eslintPlugin', + setup(build) { + const filesToLint = []; + const eslint = new ESLint(); + const filter = /src\/.*\.(jsx?|js?)$/; + + build.onLoad({ filter }, ({ path }) => { + filesToLint.push(path); + }); + + build.onEnd(async () => { + const result = await eslint.lintFiles(filesToLint); + const formatter = await eslint.loadFormatter('stylish'); + const output = formatter.format(result); + if (output.length > 0) { + // eslint-disable-next-line no-console + console.log(output); + } + }); + }, +} diff --git a/esbuild-rsync-plugin.js b/esbuild-rsync-plugin.js new file mode 100644 index 000000000..86a4affa9 --- /dev/null +++ b/esbuild-rsync-plugin.js @@ -0,0 +1,24 @@ +import * as child_process from "child_process"; + +export const cockpitRsyncPlugin = (options = {}) => ({ + name: 'cockpitRsyncPlugin', + setup(build) { + const dest = options.dest || ""; + const source = options.source || "dist/"; + + // ensure the target directory exists + child_process.spawnSync("ssh", [process.env.RSYNC, "mkdir", "-p", "/usr/local/share/cockpit/"], { stdio: "inherit" }); + + build.onEnd(() => { + const proc = child_process.spawn( + "rsync", ["--recursive", "--info=PROGRESS2", "--delete", + source, process.env.RSYNC + ":/usr/local/share/cockpit/" + dest], { stdio: "inherit" } + ); + proc.on('close', (code) => { + if (code !== 0) { + process.exit(1); + } + }); + }); + }, +}); diff --git a/esbuild-stylelint-plugin.js b/esbuild-stylelint-plugin.js new file mode 100644 index 000000000..d5f6f4ee3 --- /dev/null +++ b/esbuild-stylelint-plugin.js @@ -0,0 +1,31 @@ +// FIXME: replace when issue get's fixed https://github.com/ordros/esbuild-plugin-stylelint/issues/1 + +import * as stylelint from 'stylelint'; +import * as formatter from 'stylelint-formatter-pretty'; + +export const stylelintPlugin = ({ + filter = /\.(s?css)$/, + ...stylelintOptions +} = {}) => ({ + name: 'stylelintPlugin', + setup(build) { + const targetFiles = []; + build.onLoad({ filter }, ({ path }) => { + if (!path.includes('node_modules')) { + targetFiles.push(path); + } + }); + + build.onEnd(async () => { + const result = await stylelint.default.lint({ + formatter: formatter.default, + ...stylelintOptions, + files: targetFiles, + }); + const { output } = result; + if (output.length > 0) { + console.log(output); + } + }); + } +}); diff --git a/node_modules b/node_modules index f1adb2843..0d36ca7e0 160000 --- a/node_modules +++ b/node_modules @@ -1 +1 @@ -Subproject commit f1adb2843cf6f1f2f96e5d895e26d25a1feaa220 +Subproject commit 0d36ca7e0861a288b4328e24bfaa09203b0c83fa diff --git a/package.json b/package.json index 129a0a2f2..66ff39271 100644 --- a/package.json +++ b/package.json @@ -7,25 +7,19 @@ "author": "", "license": "LGPL-2.1", "scripts": { - "watch": "webpack --watch --progress", - "build": "webpack", + "watch": "ESBUILD_WATCH='true' node build.js", + "build": "node build.js", "eslint": "eslint --ext .jsx --ext .js src/", "eslint:fix": "eslint --fix --ext .jsx --ext .js src/", "stylelint": "stylelint src/*{.css,scss}", "stylelint:fix": "stylelint --fix src/*{.css,scss}" }, "devDependencies": { - "@babel/core": "^7.6.0", - "@babel/eslint-parser": "^7.13.14", - "@babel/preset-env": "7.12.17", - "@babel/preset-react": "^7.0.0", "argparse": "^2.0.1", - "babel-loader": "^8.0.0", "chrome-remote-interface": "^0.31.2", - "compression-webpack-plugin": "^6.0.0", - "copy-webpack-plugin": "^6.1.0", - "css-loader": "^5.2.0", - "css-minimizer-webpack-plugin": "4.0.0", + "esbuild": "0.17.7", + "esbuild-plugin-copy": "^2.0.2", + "esbuild-sass-plugin": "2.4.5", "eslint": "^8.29.0", "eslint-config-standard": "^17.0.0", "eslint-config-standard-jsx": "^11.0.0", @@ -37,21 +31,14 @@ "eslint-plugin-react": "^7.32.2", "eslint-plugin-react-hooks": "^4.6.0", "eslint-plugin-standard": "^5.0.0", - "eslint-webpack-plugin": "^4.0.0", "gettext-parser": "^2.0.0", "htmlparser": "^1.7.7", "jed": "^1.1.1", - "mini-css-extract-plugin": "^0.11.0", "sass": "^1.35.1", - "sass-loader": "^12.1.0", "sizzle": "^2.3.3", - "string-replace-loader": "^3.0.0", "stylelint": "^14.9.1", "stylelint-config-standard-scss": "^5.0.0", - "stylelint-webpack-plugin": "^3.3.0", - "terser-webpack-plugin": "^5.1.3", - "webpack": "^5.31.0", - "webpack-cli": "^4.6.0" + "stylelint-formatter-pretty": "^3.1.1" }, "dependencies": { "@patternfly/patternfly": "4.224.2", diff --git a/packaging/cockpit-podman.spec.in b/packaging/cockpit-podman.spec.in index 8cafa1283..ab6a54654 100644 --- a/packaging/cockpit-podman.spec.in +++ b/packaging/cockpit-podman.spec.in @@ -53,7 +53,7 @@ appstream-util validate-relax --nonet %{buildroot}/%{_datadir}/metainfo/* %files %doc README.md -%license LICENSE dist/index.js.LICENSE.txt.gz +%license LICENSE dist/index.js.LEGAL.txt dist/index.css.LEGAL.txt %{_datadir}/cockpit/* %{_datadir}/metainfo/* diff --git a/webpack.config.js b/webpack.config.js deleted file mode 100644 index 2bc3b0dbd..000000000 --- a/webpack.config.js +++ /dev/null @@ -1,164 +0,0 @@ -import fs from "fs"; - -import copy from "copy-webpack-plugin"; -import extract from "mini-css-extract-plugin"; -import TerserJSPlugin from 'terser-webpack-plugin'; -import CssMinimizerPlugin from 'css-minimizer-webpack-plugin'; -import CompressionPlugin from "compression-webpack-plugin"; -import ESLintPlugin from 'eslint-webpack-plugin'; -import StylelintPlugin from 'stylelint-webpack-plugin'; - -import CockpitPoPlugin from "./pkg/lib/cockpit-po-plugin.js"; -import CockpitRsyncPlugin from "./pkg/lib/cockpit-rsync-plugin.js"; - -/* A standard nodejs and webpack pattern */ -const production = process.env.NODE_ENV === 'production'; - -/* development options for faster iteration */ -const eslint = process.env.ESLINT !== '0'; - -/* Default to disable csslint for faster production builds */ -const stylelint = process.env.STYLELINT ? (process.env.STYLELINT !== '0') : !production; - -// Obtain package name from package.json -const packageJson = JSON.parse(fs.readFileSync('package.json')); - -// Non-JS files which are copied verbatim to dist/ -const copy_files = [ - "./src/index.html", - "./src/manifest.json", -]; - -const plugins = [ - new copy({ patterns: copy_files }), - new extract({ filename: "[name].css" }), - new CockpitPoPlugin(), - new CockpitRsyncPlugin({ dest: packageJson.name }), -]; - -if (eslint) { - plugins.push(new ESLintPlugin({ extensions: ["js", "jsx"], failOnWarning: true, })); -} - -if (stylelint) { - plugins.push(new StylelintPlugin({ - context: "src/", - })); -} - -/* Only minimize when in production mode */ -if (production) { - plugins.unshift(new CompressionPlugin({ - test: /\.(js|html|css)$/, - deleteOriginalAssets: true - })); -} - -const config = { - mode: production ? 'production' : 'development', - resolve: { - modules: ['node_modules', 'pkg/lib'], - alias: { 'font-awesome': 'font-awesome-sass/assets/stylesheets' }, - }, - resolveLoader: { - modules: ['node_modules', 'pkg/lib'], - }, - watchOptions: { - ignored: /node_modules/, - }, - entry: { - index: "./src/index.js", - }, - devtool: "source-map", - stats: "errors-warnings", - - optimization: { - minimize: production, - minimizer: [ - new TerserJSPlugin({ - extractComments: { - condition: true, - filename: `[file].LICENSE.txt?query=[query]&filebase=[base]`, - banner(licenseFile) { - return `License information can be found in ${licenseFile}`; - }, - }, - }), - new CssMinimizerPlugin() - ], - }, - - module: { - rules: [ - { - exclude: /node_modules/, - use: "babel-loader", - test: /\.(js|jsx)$/ - }, - /* HACK: remove unwanted fonts from PatternFly's css */ - { - test: /patternfly-4-cockpit.scss$/, - use: [ - extract.loader, - { - loader: 'css-loader', - options: { - sourceMap: true, - url: false, - }, - }, - { - loader: 'string-replace-loader', - options: { - multiple: [ - { - search: /src:url\("patternfly-icons-fake-path\/pficon[^}]*/g, - replace: 'src:url("../base1/fonts/patternfly.woff") format("woff");', - }, - { - search: /@font-face[^}]*patternfly-fonts-fake-path[^}]*}/g, - replace: '', - }, - ] - }, - }, - { - loader: 'sass-loader', - options: { - sourceMap: !production, - sassOptions: { - outputStyle: production ? 'compressed' : undefined, - }, - }, - }, - ] - }, - { - test: /\.s?css$/, - exclude: /patternfly-4-cockpit.scss/, - use: [ - extract.loader, - { - loader: 'css-loader', - options: { - sourceMap: true, - url: false - } - }, - { - loader: 'sass-loader', - options: { - sourceMap: !production, - sassOptions: { - outputStyle: production ? 'compressed' : undefined, - }, - }, - }, - ] - }, - ] - }, - plugins -}; - -export default config;