Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

misc(build): replace rollup with esbuild #15239

Merged
merged 79 commits into from
Jul 14, 2023
Merged
Show file tree
Hide file tree
Changes from 76 commits
Commits
Show all changes
79 commits
Select commit Hold shift + click to select a range
1722e38
report
connorjclark Aug 30, 2022
1901cf9
gh-pages
connorjclark Aug 30, 2022
12301ac
wip: build lr bundles
connorjclark Aug 31, 2022
ffc3bad
fix lr buildReportGenerator plugin order
connorjclark Aug 31, 2022
8720d6c
build-bundle pretty much done
connorjclark Aug 31, 2022
cf45b60
rm build-report-es.js
connorjclark Aug 31, 2022
aff685a
dt report resources
connorjclark Aug 31, 2022
45a6583
smokehouse bundle
connorjclark Aug 31, 2022
9273f70
extension
connorjclark Aug 31, 2022
cb0847c
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Oct 4, 2022
cf101f9
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Oct 6, 2022
a4a3321
fix lr gen
connorjclark Oct 6, 2022
5d802e9
improve
connorjclark Oct 6, 2022
10720cb
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Oct 7, 2022
65e62fc
preloads
connorjclark Oct 7, 2022
a7f960f
charset
connorjclark Oct 7, 2022
2954458
include a working node polyfill plugin
connorjclark Oct 12, 2022
47ce129
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Oct 18, 2022
a876857
tweak
connorjclark Oct 18, 2022
0392b62
move to third-party
connorjclark Oct 18, 2022
b6ff768
rm rollup; fix inflate shim
connorjclark Oct 18, 2022
0f4e50c
better document replaceModules plugin
connorjclark Oct 19, 2022
d581317
allow return null in partial loader
connorjclark Oct 19, 2022
5bdd19c
big wip - page fns
connorjclark Nov 2, 2022
2062ab0
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Nov 15, 2022
eb121e5
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Nov 15, 2022
445874f
wip
connorjclark Nov 15, 2022
90e5490
page fns need a refactor...
connorjclark Nov 16, 2022
ff801e9
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Jul 6, 2023
a15cf24
rm
connorjclark Jul 6, 2023
6cdba38
just use terser for minify
connorjclark Jul 6, 2023
9eddca6
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Jul 7, 2023
7a4f6fd
tests: save smokehouse failures, and impove bundle runner logging
connorjclark Jul 7, 2023
3d06234
Merge branch 'improve-smokehouse-failing' into build-esbuild
connorjclark Jul 7, 2023
2f2f25e
fix __name
connorjclark Jul 7, 2023
bc1af61
updates
connorjclark Jul 7, 2023
609e15d
latest esbuild
connorjclark Jul 7, 2023
ccffe5f
remove unused text replacements
connorjclark Jul 7, 2023
4221b11
clean up build-bundle a bit
connorjclark Jul 7, 2023
bebd5cc
add comments to replace plugin
connorjclark Jul 10, 2023
6c7ec5e
make umd plugin
connorjclark Jul 10, 2023
6b0adc4
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Jul 10, 2023
bcf62e3
delete commented out code
connorjclark Jul 10, 2023
c2531f4
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Jul 10, 2023
133f3c7
remove unused code; fix smokehouse bundle
connorjclark Jul 10, 2023
a6277f2
fix umd plugin writing a <stdout> file
connorjclark Jul 10, 2023
924b7a4
fix exec context test
connorjclark Jul 10, 2023
681febf
fix types
connorjclark Jul 10, 2023
0ce0745
fix umd bundle
connorjclark Jul 10, 2023
60e521c
incl assert/strict for smokehouse bundle
connorjclark Jul 10, 2023
3e6cc60
fix assert/strict for smokehouse bundle
connorjclark Jul 10, 2023
62d8995
support nested modules for umd
connorjclark Jul 10, 2023
d7f76b3
fix types
connorjclark Jul 10, 2023
af5ca19
strictEqual
connorjclark Jul 10, 2023
9b59988
i hate modules
connorjclark Jul 10, 2023
4a6f0f9
lets just use assert
connorjclark Jul 10, 2023
502076d
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Jul 10, 2023
9c9bf58
fix merge error
connorjclark Jul 10, 2023
0860d8f
dont need this
connorjclark Jul 11, 2023
4c76a8e
fix eval on new doc script
connorjclark Jul 11, 2023
b512fae
fix exec tests
connorjclark Jul 11, 2023
f7a0698
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Jul 11, 2023
0a27e45
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Jul 12, 2023
907739a
fix process polyfilling
connorjclark Jul 12, 2023
bbcd02b
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Jul 12, 2023
d893d23
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Jul 13, 2023
ae6bf17
bring back those zlib things
connorjclark Jul 13, 2023
853f9e9
use inject
connorjclark Jul 13, 2023
6370622
forgot that
connorjclark Jul 14, 2023
419e423
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Jul 14, 2023
3c8c2e3
use esbuild to minify viewer report gen
connorjclark Jul 14, 2023
7d4f200
rm old plugin
connorjclark Jul 14, 2023
2eb0367
move chrome launch to before injecting bundled lh for test bundle
connorjclark Jul 14, 2023
a69635b
restore process
connorjclark Jul 14, 2023
80db263
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Jul 14, 2023
2e98743
bad merge
connorjclark Jul 14, 2023
5432245
Merge remote-tracking branch 'origin/main' into build-esbuild
connorjclark Jul 14, 2023
8e89da1
empty commit bc all of gha just broke
connorjclark Jul 14, 2023
9e70bdc
comment
connorjclark Jul 14, 2023
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .github/workflows/smoke.yml
Original file line number Diff line number Diff line change
Expand Up @@ -156,7 +156,7 @@ jobs:

- run: sudo apt-get install xvfb
- name: yarn test-bundle
run: xvfb-run --auto-servernum yarn test-bundle --shard=${{ matrix.smoke-test-shard }}/$SHARD_TOTAL
run: xvfb-run --auto-servernum yarn test-bundle --shard=${{ matrix.smoke-test-shard }}/$SHARD_TOTAL --retries=2
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm definitely concerned if changing our bundler introduces flakiness

Copy link
Collaborator Author

@connorjclark connorjclark Jul 11, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This option was present on the test-bundle script already.

It's not good to apply default options to the script here. I certainly did not expect it. Note that I added this flag to CI invocation, which is where it matters.


# Fail if any changes were written to source files.
- run: git diff --exit-code
Expand Down
214 changes: 121 additions & 93 deletions build/build-bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,16 +15,18 @@ import {execSync} from 'child_process';
import {createRequire} from 'module';

import esMain from 'es-main';
import {rollup} from 'rollup';
import esbuild from 'esbuild';
// @ts-expect-error: plugin has no types.
import PubAdsPlugin from 'lighthouse-plugin-publisher-ads';
// @ts-expect-error: plugin has no types.
import SoftNavPlugin from 'lighthouse-plugin-soft-navigation';
import * as terser from 'terser';

import * as rollupPlugins from './rollup-plugins.js';
import * as plugins from './esbuild-plugins.js';
import {Runner} from '../core/runner.js';
import {LH_ROOT} from '../root.js';
import {readJson} from '../core/test/test-utils.js';
import {nodeModulesPolyfillPlugin} from '../third-party/esbuild-plugins-polyfills/esbuild-polyfills.js';

const require = createRequire(import.meta.url);

Expand Down Expand Up @@ -81,10 +83,6 @@ const banner = `
* @return {Promise<void>}
*/
async function buildBundle(entryPath, distPath, opts = {minify: true}) {
if (fs.existsSync(LH_ROOT + '/lighthouse-logger/node_modules')) {
throw new Error('delete `lighthouse-logger/node_modules` because it messes up rollup bundle');
}

// List of paths (absolute / relative to config-helpers.js) to include
// in bundle and make accessible via config-helpers.js `requireWrapper`.
const dynamicModulePaths = [
Expand All @@ -110,12 +108,20 @@ async function buildBundle(entryPath, distPath, opts = {minify: true}) {
}).join(',\n');

/** @type {Record<string, string>} */
const shimsObj = {};
const shimsObj = {
// zlib's decompression code is very large and we don't need it.
// We export empty functions, instead of an empty module, simply to silence warnings
// about no exports.
'__zlib-lib/inflate': `
export function inflateInit2() {};
export function inflate() {};
export function inflateEnd() {};
export function inflateReset() {};
`,
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is this a fix we could upstream to rollup?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This must have been necessary for a previous iteration of the build script. I reduced it to an empty string just fine.

};

const modulesToIgnore = [
'puppeteer-core',
'intl-pluralrules',
'intl',
'pako/lib/zlib/inflate.js',
'@sentry/node',
'source-map',
Expand All @@ -125,55 +131,37 @@ async function buildBundle(entryPath, distPath, opts = {minify: true}) {
// Don't include the stringified report in DevTools - see devtools-report-assets.js
// Don't include in Lightrider - HTML generation isn't supported, so report assets aren't needed.
if (isDevtools(entryPath) || isLightrider(entryPath)) {
shimsObj[require.resolve('../report/generator/report-assets.js')] =
shimsObj[`${LH_ROOT}/report/generator/report-assets.js`] =
'export const reportAssets = {}';
}

// Don't include locales in DevTools.
if (isDevtools(entryPath)) {
shimsObj['./locales.js'] = 'export const locales = {};';
shimsObj[`${LH_ROOT}/shared/localization/locales.js`] = 'export const locales = {};';
}

for (const modulePath of modulesToIgnore) {
shimsObj[modulePath] = 'export default {}';
}

const bundle = await rollup({
input: entryPath,
context: 'globalThis',
const result = await esbuild.build({
entryPoints: [entryPath],
outfile: distPath,
write: false,
format: 'iife',
charset: 'utf8',
bundle: true,
// For now, we defer to terser.
minify: false,
treeShaking: true,
sourcemap: DEBUG,
banner: {js: banner},
// Because of page-functions!
keepNames: true,
inject: ['./build/process-global.js'],
/** @type {esbuild.Plugin[]} */
plugins: [
rollupPlugins.replace({
delimiters: ['', ''],
values: {
'/* BUILD_REPLACE_BUNDLED_MODULES */': `[\n${bundledMapEntriesCode},\n]`,
// This package exports to default in a way that causes Rollup to get confused,
// resulting in MessageFormat being undefined.
'require(\'intl-messageformat\').default': 'require(\'intl-messageformat\')',
// Below we replace lighthouse-logger with a local copy, which is ES modules. Need
// to change every require of the package to reflect this.
'require(\'lighthouse-logger\');': 'require(\'lighthouse-logger\').default;',
// Rollup doesn't replace this, so let's manually change it to false.
'require.main === module': 'false',
// TODO: Use globalThis directly.
'global.isLightrider': 'globalThis.isLightrider',
'global.isDevtools': 'globalThis.isDevtools',
// For some reason, `shim` doesn't work to force this module to return false, so instead
// just replace usages of it with false.
'esMain(import.meta)': 'false',
'import esMain from \'es-main\'': '',
// By default Rollup converts `import.meta` to a big mess of `document.currentScript && ...`,
// and uses the output name as the url. Instead, do a simpler conversion and use the
// module path.
'import.meta': (id) => `{url: '${path.relative(LH_ROOT, id)}'}`,
},
}),
rollupPlugins.alias({
entries: {
'debug': require.resolve('debug/src/browser.js'),
'lighthouse-logger': require.resolve('../lighthouse-logger/index.js'),
},
}),
rollupPlugins.shim({
plugins.replaceModules({
...shimsObj,
'url': `
export const URL = globalThis.URL;
Expand All @@ -189,59 +177,99 @@ async function buildBundle(entryPath, distPath, opts = {minify: true}) {
};
};
`,
}, {
// buildBundle is used in a lot of different contexts. Some share the same modules
// that need to be replaced, but others don't use those modules at all.
disableUnusedError: true,
}),
rollupPlugins.json(),
rollupPlugins.removeModuleDirCalls(),
rollupPlugins.inlineFs({
verbose: Boolean(process.env.DEBUG),
ignorePaths: [
require.resolve('puppeteer-core/lib/esm/puppeteer/common/Page.js'),
],
}),
rollupPlugins.commonjs({
// https://github.com/rollup/plugins/issues/922
ignoreGlobal: true,
}),
rollupPlugins.nodePolyfills(),
rollupPlugins.nodeResolve({preferBuiltins: true}),
// Rollup sees the usages of these functions in page functions (ex: see AnchorElements)
// and treats them as globals. Because the names are "taken" by the global, Rollup renames
// the actual functions (getNodeDetails$1). The page functions expect a certain name, so
// here we undo what Rollup did.
rollupPlugins.postprocess([
[/getBoundingClientRect\$1/, 'getBoundingClientRect'],
[/getElementsInDocument\$1/, 'getElementsInDocument'],
[/getNodeDetails\$1/, 'getNodeDetails'],
[/getRectCenterPoint\$1/, 'getRectCenterPoint'],
[/isPositionFixed\$1/, 'isPositionFixed'],
nodeModulesPolyfillPlugin(),
plugins.bulkLoader([
// TODO: when we used rollup, various things were tree-shaken out before inlineFs did its
// thing. Now treeshaking only happens at the end, so the plugin sees more cases than it
// did before. Some of those new cases emit warnings. Safe to ignore, but should be
// resolved eventually.
plugins.partialLoaders.inlineFs({
verbose: Boolean(process.env.DEBUG),
ignorePaths: [require.resolve('puppeteer-core/lib/esm/puppeteer/common/Page.js')],
}),
plugins.partialLoaders.rmGetModuleDirectory,
plugins.partialLoaders.replaceText({
'/* BUILD_REPLACE_BUNDLED_MODULES */': `[\n${bundledMapEntriesCode},\n]`,
// TODO: Use globalThis directly.
'global.isLightrider': 'globalThis.isLightrider',
'global.isDevtools': 'globalThis.isDevtools',
// By default esbuild converts `import.meta` to an empty object.
// We need at least the url property for i18n things.
/** @param {string} id */
'import.meta': (id) => `{url: '${path.relative(LH_ROOT, id)}'}`,
}),
]),
opts.minify && rollupPlugins.terser({
ecma: 2019,
output: {
comments: (node, comment) => {
const text = comment.value;
if (text.includes('The Lighthouse Authors') && comment.line > 1) return false;
return /@ts-nocheck - Prevent tsc|@preserve|@license|@cc_on|^!/i.test(text);
},
max_line_len: 1000,
{
name: 'alias',
setup({onResolve}) {
onResolve({filter: /\.*/}, (args) => {
/** @type {Record<string, string>} */
const entries = {
'debug': require.resolve('debug/src/browser.js'),
'lighthouse-logger': require.resolve('../lighthouse-logger/index.js'),
};
if (args.path in entries) {
return {path: entries[args.path]};
}
});
},
// The config relies on class names for gatherers.
keep_classnames: true,
// Runtime.evaluate errors if function names are elided.
keep_fnames: true,
}),
},
{
name: 'postprocess',
setup({onEnd}) {
onEnd(result => {
if (!result.outputFiles) throw new Error();

let code = result.outputFiles[0].text;

// Get rid of our extra license comments.
// https://stackoverflow.com/a/35923766
const re = /\/\*\*\s*\n([^*]|(\*(?!\/)))*\*\/\n/g;
let hasSeenFirst = false;
code = code.replace(re, (match) => {
if (match.includes('@license') && match.match(/Lighthouse Authors|Google/)) {
if (hasSeenFirst) {
return '';
}

hasSeenFirst = true;
}

return match;
});

result.outputFiles[0].contents = new TextEncoder().encode(code);
});
},
},
],
});

await bundle.write({
file: distPath,
banner,
format: 'iife',
sourcemap: DEBUG,
// Suppress code splitting.
inlineDynamicImports: true,
});
await bundle.close();
let code = result.outputFiles[0].text;

// Just make sure the above shimming worked.
if (code.includes('inflate_fast')) {
throw new Error('Expected zlib inflate code to have been removed');
}

// Ideally we'd let esbuild minify, but we need to disable variable name mangling otherwise
// code generated dynamically to run inside the browser (pageFunctions) breaks. For example,
// the `truncate` function is unable to properly reference `Util`.
if (opts.minify) {
code = (await terser.minify(result.outputFiles[0].text, {
mangle: false,
format: {
max_line_len: 1000,
},
})).code || '';
}

await fs.promises.writeFile(result.outputFiles[0].path, code);
}

/**
Expand Down
29 changes: 14 additions & 15 deletions build/build-dt-report-resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import fs from 'fs';
import path from 'path';
import assert from 'assert/strict';

import {rollup} from 'rollup';
import esbuild from 'esbuild';

import * as rollupPlugins from './rollup-plugins.js';
import * as plugins from './esbuild-plugins.js';
import {LH_ROOT} from '../root.js';

const distDir = path.join(LH_ROOT, 'dist', 'dt-report-resources');
Expand All @@ -31,24 +31,23 @@ fs.mkdirSync(distDir, {recursive: true});
writeFile('report-generator.mjs.d.ts', 'export {}');

async function buildReportGenerator() {
const bundle = await rollup({
input: 'report/generator/report-generator.js',
await esbuild.build({
entryPoints: ['report/generator/report-generator.js'],
outfile: bundleOutFile,
bundle: true,
minify: false,
plugins: [
rollupPlugins.shim({
plugins.umd('Lighthouse.ReportGenerator'),
plugins.replaceModules({
[`${LH_ROOT}/report/generator/flow-report-assets.js`]: 'export const flowReportAssets = {}',
}),
rollupPlugins.nodeResolve(),
rollupPlugins.removeModuleDirCalls(),
rollupPlugins.inlineFs({verbose: Boolean(process.env.DEBUG)}),
plugins.bulkLoader([
plugins.partialLoaders.inlineFs({verbose: Boolean(process.env.DEBUG)}),
plugins.partialLoaders.rmGetModuleDirectory,
]),
plugins.ignoreBuiltins(),
],
});

await bundle.write({
file: bundleOutFile,
format: 'umd',
name: 'Lighthouse.ReportGenerator',
});
await bundle.close();
}

await buildReportGenerator();
35 changes: 13 additions & 22 deletions build/build-extension.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@ import fs from 'fs';

import archiver from 'archiver';
import cpy from 'cpy';
import {rollup} from 'rollup';
import esbuild from 'esbuild';

import * as rollupPlugins from './rollup-plugins.js';
import * as plugins from './esbuild-plugins.js';
import {LH_ROOT} from '../root.js';
import {readJson} from '../core/test/test-utils.js';

Expand All @@ -26,30 +26,21 @@ const packagePath = `${distDir}/../extension-${browserBrand}-package`;

const manifestVersion = readJson(`${sourceDir}/manifest.json`).version;

/**
* Bundle and minify entry point.
*/
async function buildEntryPoint() {
const bundle = await rollup({
input: `${sourceDir}/scripts/${sourceName}`,
await esbuild.build({
entryPoints: [`${sourceDir}/scripts/${sourceName}`],
outfile: `${distDir}/scripts/${distName}`,
format: 'iife',
bundle: true,
minify: !process.env.DEBUG,
plugins: [
rollupPlugins.shim({
[`${LH_ROOT}/report/generator/flow-report-assets.js`]: 'export default {}',
}),
rollupPlugins.replace({
'___BROWSER_BRAND___': browserBrand,
}),
rollupPlugins.nodeResolve(),
rollupPlugins.inlineFs({verbose: Boolean(process.env.DEBUG)}),
rollupPlugins.terser(),
plugins.bulkLoader([
plugins.partialLoaders.replaceText({
'___BROWSER_BRAND___': browserBrand,
}),
]),
],
});

await bundle.write({
file: `${distDir}/scripts/${distName}`,
format: 'iife',
});
await bundle.close();
}

function copyAssets() {
Expand Down
Loading
Loading