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): use rollup to build lighthouse-core bundles #12771

Merged
merged 54 commits into from
Nov 1, 2021
Merged
Show file tree
Hide file tree
Changes from 39 commits
Commits
Show all changes
54 commits
Select commit Hold shift + click to select a range
f5cda27
wip
connorjclark Jul 2, 2021
f9ddfa4
wip
connorjclark Jul 3, 2021
3050c68
pubads working
connorjclark Jul 7, 2021
aaaf2de
wip sourcemap
connorjclark Jul 7, 2021
229ae00
update
connorjclark Jul 7, 2021
bc23310
clean up build bundle file
connorjclark Jul 7, 2021
b3d9037
banner
connorjclark Jul 7, 2021
823c6f5
working again
connorjclark Jul 7, 2021
dffb095
Merge remote-tracking branch 'origin/master' into esmodules-rollup
connorjclark Jul 9, 2021
d7e7c1b
fix getBoundingClientRect mangle
connorjclark Jul 9, 2021
0fda799
tweak
connorjclark Jul 9, 2021
213d949
fix asset-saver
connorjclark Jul 10, 2021
b975d11
fix lighthouse logger type check
connorjclark Jul 10, 2021
f90efbf
dolla dolla bills
connorjclark Jul 10, 2021
950e1eb
Merge remote-tracking branch 'origin/master' into esmodules-rollup
connorjclark Jul 21, 2021
7cdce5e
Merge remote-tracking branch 'origin/master' into esmodules-rollup
connorjclark Jul 23, 2021
4c96a26
postprocess package
connorjclark Jul 23, 2021
dc0fdd1
Merge remote-tracking branch 'origin/master' into esmodules-rollup
connorjclark Aug 13, 2021
03ca850
readfile
connorjclark Aug 13, 2021
1dfa92d
merge from master and fixed most issues. broken in devtools now.
connorjclark Sep 14, 2021
a1a0ff6
wip
connorjclark Sep 15, 2021
3cf5303
fix
connorjclark Sep 15, 2021
667b536
build: create rollup-plugins.js helper module
connorjclark Sep 24, 2021
db22bea
Merge remote-tracking branch 'origin/master' into esmodules-rollup
connorjclark Sep 24, 2021
b1a3906
Merge branch 'rollup-plugins-helper' into esmodules-rollup
connorjclark Sep 24, 2021
9cb65d1
update
connorjclark Sep 25, 2021
2a99682
fix
connorjclark Sep 27, 2021
ed47297
Merge remote-tracking branch 'origin/master' into esmodules-rollup
connorjclark Sep 27, 2021
e74eba3
fix version
connorjclark Sep 28, 2021
e5d5782
rm old comment
connorjclark Sep 28, 2021
1741e04
tweak dt bundle nocheck
connorjclark Sep 28, 2021
b52af24
fix
connorjclark Sep 28, 2021
4051326
fix error in stack pack
connorjclark Sep 28, 2021
031781e
Merge remote-tracking branch 'origin/master' into esmodules-rollup
connorjclark Sep 28, 2021
fb203e1
fix mkdir
connorjclark Sep 28, 2021
41512b9
fix require.main
connorjclark Sep 28, 2021
655c69c
globalThis
connorjclark Sep 28, 2021
507e38c
fix smokehouse bundle
connorjclark Sep 28, 2021
806744c
Update build/build-bundle.js
connorjclark Sep 29, 2021
36056c4
Merge remote-tracking branch 'origin/master' into esmodules-rollup
connorjclark Oct 6, 2021
fb5a762
Merge remote-tracking branch 'origin/master' into esmodules-rollup
connorjclark Oct 20, 2021
a250469
Merge remote-tracking branch 'origin/master' into esmodules-rollup
connorjclark Oct 21, 2021
ab3f498
Merge remote-tracking branch 'origin/master' into esmodules-rollup
connorjclark Oct 28, 2021
5c91976
locales
connorjclark Oct 29, 2021
37065bf
attempt fix
connorjclark Oct 29, 2021
3f57709
oops
connorjclark Oct 29, 2021
f4494ab
test source-maps bundled
connorjclark Oct 30, 2021
0b6d25c
fix type
connorjclark Oct 30, 2021
8d1d7a1
Merge remote-tracking branch 'origin/master' into esmodules-rollup
connorjclark Oct 30, 2021
38cac7e
ok
connorjclark Oct 30, 2021
546685d
Merge remote-tracking branch 'origin/master' into esmodules-rollup
connorjclark Oct 30, 2021
0adea44
Merge branch 'master' into esmodules-rollup
connorjclark Nov 1, 2021
dfa0cdc
Merge brt meranch 'esmodules-rollup' of github.com:GoogleChrome/light…
connorjclark Nov 1, 2021
1ed7697
Merge remote-tracking branch 'origin/master' into esmodules-rollup
connorjclark Nov 1, 2021
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
7 changes: 0 additions & 7 deletions build/banner.txt

This file was deleted.

295 changes: 152 additions & 143 deletions build/build-bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,29 +12,16 @@

const fs = require('fs');
const path = require('path');
const assert = require('assert').strict;
const stream = require('stream');
const mkdir = fs.promises.mkdir;
const LighthouseRunner = require('../lighthouse-core/runner.js');
const exorcist = require('exorcist');
const browserify = require('browserify');
const terser = require('terser');
const rollup = require('rollup');
const rollupPlugins = require('./rollup-plugins.js');
const {minifyFileTransform} = require('./build-utils.js');
const Runner = require('../lighthouse-core/runner.js');
const {LH_ROOT} = require('../root.js');

const COMMIT_HASH = require('child_process')
.execSync('git rev-parse HEAD')
.toString().trim();

const audits = LighthouseRunner.getAuditList()
.map(f => './lighthouse-core/audits/' + f.replace(/\.js$/, ''));

const gatherers = LighthouseRunner.getGathererList()
.map(f => './lighthouse-core/gather/gatherers/' + f.replace(/\.js$/, ''));

const locales = fs.readdirSync(LH_ROOT + '/lighthouse-core/lib/i18n/locales/')
.map(f => require.resolve(`../lighthouse-core/lib/i18n/locales/${f}`));

// HACK: manually include the lighthouse-plugin-publisher-ads audits.
/** @type {Array<string>} */
// @ts-expect-error
Expand All @@ -49,155 +36,177 @@ const isLightrider = file => path.basename(file).includes('lightrider');
// Set to true for source maps.
const DEBUG = false;

const today = (() => {
const date = new Date();
const year = new Intl.DateTimeFormat('en', {year: 'numeric'}).format(date);
const month = new Intl.DateTimeFormat('en', {month: 'short'}).format(date);
const day = new Intl.DateTimeFormat('en', {day: '2-digit'}).format(date);
return `${month} ${day} ${year}`;
})();
const pkg = JSON.parse(fs.readFileSync(LH_ROOT + '/package.json', 'utf-8'));
const banner = `
/**
* Lighthouse v${pkg.version} ${COMMIT_HASH} (${today})
*
* ${pkg.description}
*
* @homepage ${pkg.homepage}
* @author ${pkg.author}
* @license ${pkg.license}
*/
`.trim();

/**
* Browserify starting at the file at entryPath. Contains entry-point-specific
* ignores (e.g. for DevTools or the extension) to trim the bundle depending on
* the eventual use case.
* Bundle starting at entryPath, writing the minified result to distPath.
* @param {string} entryPath
* @param {string} distPath
* @param {{minify: boolean}=} opts
* @return {Promise<void>}
*/
async function browserifyFile(entryPath, distPath) {
let bundle = browserify(entryPath, {debug: DEBUG});

bundle
.plugin('browserify-banner', {
pkg: Object.assign({COMMIT_HASH}, require('../package.json')),
file: require.resolve('./banner.txt'),
})
// Transform the fs.readFile etc into inline strings.
.transform('@wardpeet/brfs', {
/** @param {string} file */
readFileTransform: (file) => {
// Don't include locales in DevTools.
if (isDevtools(entryPath) && locales.includes(file)) {
return new stream.Transform({
transform(chunk, enc, next) {
next();
},
final(next) {
this.push('{}');
next();
},
});
}

return minifyFileTransform(file);
},
global: true,
parserOpts: {ecmaVersion: 12},
})
// Strip everything out of package.json includes except for the version.
.transform('package-json-versionify');

// scripts will need some additional transforms, ignores and requires…
bundle.ignore('source-map')
.ignore('debug/node')
.ignore('intl')
.ignore('intl-pluralrules')
.ignore('raven')
.ignore('pako/lib/zlib/inflate.js');

// Don't include the desktop protocol connection.
bundle.ignore(require.resolve('../lighthouse-core/gather/connections/cri.js'));

// 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)) {
bundle.ignore(require.resolve('../report/generator/report-assets.js'));
async function build(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');
}

// Expose the audits, gatherers, and computed artifacts so they can be dynamically loaded.
// Exposed path must be a relative path from lighthouse-core/config/config-helpers.js (where loading occurs).
const corePath = './lighthouse-core/';
const driverPath = `${corePath}gather/`;
audits.forEach(audit => {
bundle = bundle.require(audit, {expose: audit.replace(corePath, '../')});
});
gatherers.forEach(gatherer => {
bundle = bundle.require(gatherer, {expose: gatherer.replace(driverPath, '../gather/')});
});
// List of paths (absolute / relative to config-helpers.js) to include
// in bundle and make accessible via config-helpers.js `requireWrapper`.
const dynamicModulePaths = [
...Runner.getGathererList().map(gatherer => `../gather/gatherers/${gatherer}`),
...Runner.getAuditList().map(gatherer => `../audits/${gatherer}`),
];

// HACK: manually include the lighthouse-plugin-publisher-ads audits.
// Include lighthouse-plugin-publisher-ads.
if (isDevtools(entryPath) || isLightrider(entryPath)) {
bundle.require('lighthouse-plugin-publisher-ads');
dynamicModulePaths.push('lighthouse-plugin-publisher-ads');
pubAdsAudits.forEach(pubAdAudit => {
bundle = bundle.require(pubAdAudit);
dynamicModulePaths.push(pubAdAudit);
});
}

// browerify's url shim doesn't work with .URL in node_modules,
// and within robots-parser, it does `var URL = require('url').URL`, so we expose our own.
// @see https://github.com/GoogleChrome/lighthouse/issues/5273
const pathToURLShim = require.resolve('../lighthouse-core/lib/url-shim.js');
bundle = bundle.require(pathToURLShim, {expose: 'url'});
const bundledMapEntriesCode = dynamicModulePaths.map(modulePath => {
const pathNoExt = modulePath.replace('.js', '');
return `['${pathNoExt}', require('${modulePath}')]`;
}).join(',\n');

let bundleStream = bundle.bundle();
/** @type {Record<string, string>} */
const shimsObj = {};

// Make sure path exists.
await mkdir(path.dirname(distPath), {recursive: true});
await new Promise((resolve, reject) => {
const writeStream = fs.createWriteStream(distPath);
writeStream.on('finish', resolve);
writeStream.on('error', reject);
const modulesToIgnore = [
'intl-pluralrules',
'intl',
'pako/lib/zlib/inflate.js',
'raven',
'source-map',
'ws',
require.resolve('../lighthouse-core/gather/connections/cri.js'),
];

// Extract the inline source map to an external file.
if (DEBUG) bundleStream = bundleStream.pipe(exorcist(`${distPath}.map`));
bundleStream.pipe(writeStream);
});

if (isDevtools(distPath)) {
let code = fs.readFileSync(distPath, 'utf-8');
// Add a comment for TypeScript, but not if in DEBUG mode so that source maps are not affected.
// See lighthouse-cli/test/smokehouse/lighthouse-runners/bundle.js
if (!DEBUG) {
code = '// @ts-nocheck - Prevent tsc stepping into any required bundles.\n' + code;
}
// 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)) {
modulesToIgnore.push(require.resolve('../report/generator/report-assets.js'));
}

// DevTools build system expects `globalThis` rather than setting global variables.
assert.ok(code.includes('\nrequire='), 'missing browserify require stub');
code = code.replace('\nrequire=', '\nglobalThis.require=');
assert.ok(!code.includes('\nrequire='), 'contained unexpected browserify require stub');
// Don't include locales in DevTools.
if (isDevtools(entryPath)) {
const localeKeys = Object.keys(require('../lighthouse-core/lib/i18n/locales.js'));
/** @type {Record<string, {}>} */
const localesShim = {};
for (const key of localeKeys) localesShim[key] = {};
shimsObj['./locales.js'] = `export default ${JSON.stringify(localesShim)}`;
}

fs.writeFileSync(distPath, code);
for (const modulePath of modulesToIgnore) {
shimsObj[modulePath] = 'export default {}';
}
}

/**
* Minify a javascript file, in place.
* @param {string} filePath
*/
async function minifyScript(filePath) {
const code = fs.readFileSync(filePath, 'utf-8');
const result = await terser.minify(code, {
ecma: 2019,
output: {
comments: /^!|Prevent tsc/,
max_line_len: 1000,
},
// The config relies on class names for gatherers.
keep_classnames: true,
// Runtime.evaluate errors if function names are elided.
keep_fnames: true,
sourceMap: DEBUG && {
content: JSON.parse(fs.readFileSync(`${filePath}.map`, 'utf-8')),
url: path.basename(`${filePath}.map`),
},
shimsObj[require.resolve('../package.json')] =
`export const version = ${JSON.stringify(require('../package.json').version)}`;
Copy link
Member

Choose a reason for hiding this comment

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

Could do this from the above.

Suggested change
`export const version = ${JSON.stringify(require('../package.json').version)}`;
`export const version = ${pkg.version)}`;

Copy link
Collaborator Author

@connorjclark connorjclark Sep 29, 2021

Choose a reason for hiding this comment

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

It's wrapped in stringify because it emits valid JS.

version = ${pkg.version} results in version = 8.5.1.

one could argue for version = '${pkg.version}' but the quotes are a little subtle. also, not relevant here but that pattern breaks when there are invalid string characters (this was a big motivator for the ExecutionContext.evaluate api).


const bundle = await rollup.rollup({
input: entryPath,
context: 'globalThis',
plugins: [
rollupPlugins.replace({
delimiters: ['', ''],
values: {
'/* BUILD_REPLACE_BUNDLED_MODULES */': `[\n${bundledMapEntriesCode},\n]`,
'__dirname': (id) => `'${path.relative(LH_ROOT, path.dirname(id))}'`,
'__filename': (id) => `'${path.relative(LH_ROOT, id)}'`,
// 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\')',
// 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',
},
}),
rollupPlugins.alias({
entries: {
'debug': require.resolve('debug/src/browser.js'),
'lighthouse-logger': require.resolve('../lighthouse-logger/index.js'),
'url': require.resolve('../lighthouse-core/lib/url-shim.js'),
},
}),
rollupPlugins.shim({
...shimsObj,
// Allows for plugins to import lighthouse.
'lighthouse': `
import Audit from '${require.resolve('../lighthouse-core/audits/audit.js')}';
export {Audit};
`,
}),
rollupPlugins.json(),
// Currently must run before commonjs (brfs does not support import).
// This currenty messes up source maps.
rollupPlugins.brfs({
readFileTransform: minifyFileTransform,
global: true,
parserOpts: {ecmaVersion: 12, sourceType: 'module'},
}),
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)
Copy link
Collaborator

Choose a reason for hiding this comment

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

yikes, this is going to be really annoying to remember. maybe we should tweak our evaluate API to support a keyed-object so we don't have to think about it?

or better yet always force deps to be function parameters? there are just too many gotchas these days with the .toString name reliance

Copy link
Collaborator Author

@connorjclark connorjclark Jul 12, 2021

Choose a reason for hiding this comment

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

I'm for it. link should do the trick.

Copy link
Member

@brendankenny brendankenny Jul 15, 2021

Choose a reason for hiding this comment

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

IMO if we're going to keep the magically-interspersed Node and evaluate() browser code (which #10816 was a culmination of us deciding it was a lot nicer to go that way) we should embrace explicit import of functions and not require everything to live in page-functions (a lot of times it's a lot nicer just to have support functions defined locally).

This would allow type checking of functions we currently @ts-expect-error and make evaluated code more robust all without added ceremony. The downside is minifiers are still kind of dumb about keeping consistent minified functions names across modules even after bundling, but there are ways around that or we could just live with somewhat worse minification.

I have an old branch for this I can put up for discussion since I know there's a variety of opinions on each of these points :)

edit: up at #12795

Copy link
Collaborator Author

@connorjclark connorjclark Jul 16, 2021

Choose a reason for hiding this comment

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

(...expanding on the fact that custom page functions outside page-functions.js exists ...)

My linked idea (having a pageFunctions namespace) can support custom functions like this:

const pageFunctions = {
  ...require('../../lib/page-functions.js'),
  myFnDeclaredHere,
};

function myFnDeclaredHere() { }

// ...

function getMetaElements() {
  return pageFunctions.getElementsInDocument('head meta').map(el => {
    const meta = /** @type {HTMLMetaElement} */ (el);
    pageFunctions.myFnDeclaredHere(); // also i exist.
    return {
      name: meta.name.toLowerCase(),
      content: meta.content,
      property: meta.attributes.property ? meta.attributes.property.value : undefined,
      httpEquiv: meta.httpEquiv ? meta.httpEquiv.toLowerCase() : undefined,
      charset: meta.attributes.charset ? meta.attributes.charset.value : undefined,
    };
  });
}

const code = pageFunctions.createEvalCode(getMetaElements, {
      deps: [
        pageFunctions.getElementsInDocument,
		pageFunctions.myFnDeclaredHere,
      ],
    });

// `pageFunctions.createEvalCode` would inject a var `pageFunctions` with all values passed to deps, using the `.name` as keys

in theory (plus removing the *String usages), that should keep the mangled names the same

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

#12795 has not been resolved–is this blocking for this PR?

// 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'],
]),
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,
},
// The config relies on class names for gatherers.
keep_classnames: true,
// Runtime.evaluate errors if function names are elided.
keep_fnames: true,
}),
],
});

fs.writeFileSync(filePath, result.code);
if (DEBUG) fs.writeFileSync(`${filePath}.map`, result.map);
}

/**
* Browserify starting at entryPath, writing the minified result to distPath.
* @param {string} entryPath
* @param {string} distPath
* @return {Promise<void>}
*/
async function build(entryPath, distPath) {
await browserifyFile(entryPath, distPath);
await minifyScript(distPath);
await bundle.write({
file: distPath,
banner,
format: 'iife',
sourcemap: DEBUG,
});
}

/**
Expand Down
7 changes: 2 additions & 5 deletions build/build-lightrider-bundles.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,15 +22,12 @@ const generatorFilename = `./report/generator/report-generator.js`;
const entrySourceName = 'lightrider-entry.js';
const entryDistName = 'lighthouse-lr-bundle.js';

fs.mkdirSync(path.dirname(distDir), {recursive: true});
fs.mkdirSync(distDir, {recursive: true});

/**
* Browserify and minify entry point.
*/
function buildEntryPoint() {
const inFile = `${sourceDir}/${entrySourceName}`;
const outFile = `${distDir}/${entryDistName}`;
return bundleBuilder.build(inFile, outFile);
return bundleBuilder.build(inFile, outFile, {minify: false});
}

/**
Expand Down
11 changes: 6 additions & 5 deletions build/build-smokehouse-bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,19 +12,20 @@ const {LH_ROOT} = require('../root.js');
const distDir = `${LH_ROOT}/dist`;
const bundleOutFile = `${distDir}/smokehouse-bundle.js`;
const smokehouseLibFilename = './lighthouse-cli/test/smokehouse/frontends/lib.js';
const smokehouseCliFilename =
require.resolve('../lighthouse-cli/test/smokehouse/lighthouse-runners/cli.js');
const smokehouseCliFilename = `${LH_ROOT}/lighthouse-cli/test/smokehouse/lighthouse-runners/cli.js`;

async function build() {
const bundle = await rollup.rollup({
input: smokehouseLibFilename,
context: 'globalThis',
plugins: [
rollupPlugins.nodeResolve(),
rollupPlugins.commonjs(),
rollupPlugins.shim({
[smokehouseCliFilename]: 'export default {}',
[smokehouseCliFilename]:
'export function runLighthouse() { throw new Error("not supported"); }',
}),
rollupPlugins.commonjs(),
rollupPlugins.nodePolyfills(),
rollupPlugins.nodeResolve(),
],
});

Expand Down
Loading