Skip to content

Commit

Permalink
maint: Drop webpack in favor of esbuild
Browse files Browse the repository at this point in the history
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] to-codando/esbuild-plugin-linter#1
[2] robinloeffel/esbuild-plugin-eslint#5

TODO:
[ ] Bundle sizes are much bigger for eslint as files are not compressed
[ ] fail on warnings eslint [0]

[0] eslint/eslint#16804
  • Loading branch information
KKoukiou committed Feb 14, 2023
1 parent 7d66688 commit cd7f46d
Show file tree
Hide file tree
Showing 13 changed files with 289 additions and 222 deletions.
10 changes: 2 additions & 8 deletions .eslintrc.json
Expand Up @@ -2,16 +2,11 @@
"root": true,
"env": {
"browser": true,
"es6": true
"es2022": true
},
"extends": ["eslint:recommended", "standard", "standard-jsx", "standard-react", "plugin:jsx-a11y/recommended"],
"parser": "@babel/eslint-parser",
"parserOptions": {
"ecmaVersion": "7",
"ecmaFeatures": {
"jsx": true
},
"sourceType": "module"
"ecmaVersion": 2022
},
"plugins": ["flowtype", "react", "react-hooks", "jsx-a11y"],
"rules": {
Expand Down Expand Up @@ -51,7 +46,6 @@
"jsx-a11y/anchor-is-valid": "off",

"eqeqeq": "off",
"import/no-webpack-loader-syntax": "off",
"react/jsx-wrap-multilines": "off",
"react/jsx-no-bind": "off"
},
Expand Down
10 changes: 5 additions & 5 deletions HACKING.md
Expand Up @@ -19,12 +19,12 @@ 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
automatically update the webpack on every code change with
[watch mode](https://esbuild.github.io/api/#watch) to
automatically rebuild on every code change with

$ make watch

When developing against a virtual machine, webpack can also automatically upload
When developing against a virtual machine, esbuild can also automatically upload
the code changes by setting the `RSYNC` environment variable to
the remote hostname.

Expand All @@ -35,7 +35,7 @@ the remote hostname.
Cockpit Podman uses [ESLint](https://eslint.org/) to automatically check
JavaScript code style in `.jsx` and `.js` files.

The linter is executed within every build as a webpack preloader.
The linter is executed within every build as a preloader.

For developer convenience, the ESLint can be started explicitly by:

Expand All @@ -52,7 +52,7 @@ Rules configuration can be found in the `.eslintrc.json` file.
Cockpit uses [Stylelint](https://stylelint.io/) to automatically check CSS code
style in `.css` and `scss` files.

The linter is executed within every build as a webpack preloader.
The linter is executed within every build as a esbuild plugin.

For developer convenience, the Stylelint can be started explicitly by:

Expand Down
25 changes: 13 additions & 12 deletions Makefile
Expand Up @@ -13,8 +13,8 @@ APPSTREAMFILE=org.cockpit-project.$(PACKAGE_NAME).metainfo.xml
VM_IMAGE=$(CURDIR)/test/images/$(TEST_OS)
# stamp file to check for node_modules/
NODE_MODULES_TEST=package-lock.json
# one example file in dist/ from webpack to check if that already ran
WEBPACK_TEST=dist/manifest.json
# one example file in dist/ from esbuild to check if that already ran
DIST_TEST=dist/manifest.json
# one example file in pkg/lib to check if it was already checked out
COCKPIT_REPO_STAMP=pkg/lib/cockpit-po-plugin.js
# common arguments for tar, mostly to make the generated tarballs reproducible
Expand All @@ -25,7 +25,7 @@ RUN_TESTS_OPTIONS+=--coverage
NODE_ENV=development
endif

all: $(WEBPACK_TEST)
all: $(DIST_TEST)

# checkout common files from Cockpit repository required to build this project;
# this has no API stability guarantee, so check out a stable tag when you start
Expand Down Expand Up @@ -91,22 +91,23 @@ packaging/arch/PKGBUILD: packaging/arch/PKGBUILD.in
packaging/debian/changelog: packaging/debian/changelog.in
sed 's/VERSION/$(VERSION)/' $< > $@

$(WEBPACK_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) npm run build
# In development mode minification does not run and so no LEGAL.txt is generated, so we explictly create it as it is required for building rpm's
if [ "$$NODE_ENV" = "development" ]; then \
gzip </dev/null >dist/index.js.LICENSE.txt.gz; \
touch dist/index.js.LEGAL.txt; \
touch dist/index.css.LEGAL.txt; \
fi

watch:
NODE_ENV=$(NODE_ENV) node_modules/.bin/webpack --watch
NODE_ENV=$(NODE_ENV) npm run watch

clean:
rm -rf dist/
rm -f $(SPEC) packaging/arch/PKGBUILD packaging/debian/changelog
rm -f po/LINGUAS

install: $(WEBPACK_TEST) po/LINGUAS
install: $(DIST_TEST) po/LINGUAS
mkdir -p $(DESTDIR)/usr/share/cockpit/$(PACKAGE_NAME)
cp -r dist/* $(DESTDIR)/usr/share/cockpit/$(PACKAGE_NAME)
mkdir -p $(DESTDIR)/usr/share/metainfo/
Expand All @@ -115,7 +116,7 @@ install: $(WEBPACK_TEST) po/LINGUAS
-o $(DESTDIR)/usr/share/metainfo/$(APPSTREAMFILE)

# this requires a built source tree and avoids having to install anything system-wide
devel-install: $(WEBPACK_TEST)
devel-install: $(DIST_TEST)
mkdir -p ~/.local/share/cockpit
ln -s `pwd`/dist ~/.local/share/cockpit/$(PACKAGE_NAME)

Expand All @@ -138,11 +139,11 @@ TEST_NPMS = \
dist: $(TARFILE)
@ls -1 $(TARFILE)

# when building a distribution tarball, call webpack with a 'production' environment by default
# when building a distribution tarball, call esbuild with a 'production' environment by default
# we don't ship most node_modules for license and compactness reasons, only the ones necessary for running tests
# we ship a pre-built dist/ (so it's not necessary) and ship package-lock.json (so that node_modules/ can be reconstructed if necessary)
$(TARFILE): export NODE_ENV ?= production
$(TARFILE): $(WEBPACK_TEST) $(SPEC) packaging/arch/PKGBUILD packaging/debian/changelog
$(TARFILE): $(DIST_TEST) $(SPEC) packaging/arch/PKGBUILD packaging/debian/changelog
if type appstream-util >/dev/null 2>&1; then appstream-util validate-relax --nonet *.metainfo.xml; fi
tar --xz $(TAR_ARGS) -cf $(TARFILE) --transform 's,^,$(RPM_NAME)/,' \
--exclude '*.in' --exclude test/reference \
Expand Down
67 changes: 67 additions & 0 deletions 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();
}
99 changes: 99 additions & 0 deletions 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)));
});
},
}
28 changes: 28 additions & 0 deletions 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);
}
});
},
}
24 changes: 24 additions & 0 deletions 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);
}
});
});
},
});
31 changes: 31 additions & 0 deletions 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);
}
});
}
});
2 changes: 1 addition & 1 deletion node_modules
Submodule node_modules updated 8168 files

0 comments on commit cd7f46d

Please sign in to comment.