misc(build): bundle with esbuild minification instead of terser

.eslintignore

@@ -17,3 +17,5 @@ third-party/**
 # ignore d.ts files until we can properly lint them
 **/*.d.ts
 **/*.d.cts
+
+page-functions-test-case*out*.js

.gitignore

@@ -67,3 +67,4 @@ yarn.lock
 **/*.d.cts
 !**/types/**/*.d.ts
 
+page-functions-test-case*out*.js

build/build-bundle.js

@@ -20,7 +20,6 @@ import esbuild from 'esbuild';
 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 plugins from './esbuild-plugins.js';
 import {Runner} from '../core/runner.js';
@@ -146,13 +145,11 @@ async function buildBundle(entryPath, distPath, opts = {minify: true}) {
 
   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,
+    minify: opts.minify,
     treeShaking: true,
     sourcemap: DEBUG,
     banner: {js: banner},
@@ -250,26 +247,14 @@ async function buildBundle(entryPath, distPath, opts = {minify: true}) {
     ],
   });
 
-  let code = result.outputFiles[0].text;
+  const 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);
+  await fs.promises.writeFile(distPath, code);
 }
 
 /**

build/test/.eslintrc.cjs

@@ -0,0 +1,14 @@
+/**
+ * @license Copyright 2023 The Lighthouse Authors. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+ */
+
+module.exports = {
+  env: {
+    mocha: true,
+  },
+  globals: {
+    expect: true,
+  },
+};

build/test/page-functions-build-test.js

@@ -0,0 +1,38 @@
+/**
+ * @license Copyright 2023 The Lighthouse Authors. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+ */
+
+// Our page functions are very sensitive to mangling performed by bundlers. Incorrect
+// bundling will certainly result in `yarn test-bundle` or `yarn smoke --runner devtools` failing.
+// The bundled lighthouse is a huge beast and hard to debug, so instead we have these smaller bundles
+// which are much easier to reason about.
+
+import path from 'path';
+import {execFileSync} from 'child_process';
+
+import {LH_ROOT} from '../../root.js';
+import {buildBundle} from '../build-bundle.js';
+
+describe('page functions build', () => {
+  const testCases = [
+    `${LH_ROOT}/build/test/page-functions-test-case-computeBenchmarkIndex.js`,
+    `${LH_ROOT}/build/test/page-functions-test-case-getNodeDetails.js`,
+    `${LH_ROOT}/build/test/page-functions-test-case-getElementsInDocument.js`,
+  ];
+
+  for (const minify of [false, true]) {
+    describe(`minify: ${minify}`, () => {
+      for (const input of testCases) {
+        it(`bundle and evaluate ${path.basename(input)}`, async () => {
+          const output = minify ?
+            input.replace('.js', '-out.min.js') :
+            input.replace('.js', '-out.js');
+          await buildBundle(input, output, {minify});
+          execFileSync('node', [output]);
+        });
+      }
+    });
+  }
+});

build/test/page-functions-test-case-computeBenchmarkIndex.js

@@ -0,0 +1,31 @@
+/**
+ * @license Copyright 2023 The Lighthouse Authors. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+ */
+
+import {ExecutionContext} from '../../core/gather/driver/execution-context.js';
+import {pageFunctions} from '../../core/lib/page-functions.js';
+
+/**
+ *
+ * @param {Function} mainFn
+ * @param {any[]} args
+ * @param {any[]} deps
+ * @return {string}
+ */
+function stringify(mainFn, args, deps) {
+  const argsSerialized = ExecutionContext.serializeArguments(args);
+  const depsSerialized = ExecutionContext.prototype._serializeDeps(deps);
+  const expression = `(() => {
+    ${depsSerialized}
+    return (${mainFn})(${argsSerialized});
+  })()`;
+  return expression;
+}
+
+// Indirect eval so code is run in global scope, and won't have incidental access to the
+// esbuild keepNames function wrapper.
+const indirectEval = eval;
+const result = indirectEval(stringify(pageFunctions.computeBenchmarkIndex, [], []));
+if (typeof result !== 'number') throw new Error(`expected number, but got ${result}`);

build/test/page-functions-test-case-getElementsInDocument.js

@@ -0,0 +1,80 @@
+/**
+ * @license Copyright 2023 The Lighthouse Authors. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+*/
+
+/* eslint-disable no-undef */
+
+import {ExecutionContext} from '../../core/gather/driver/execution-context.js';
+import {pageFunctions} from '../../core/lib/page-functions.js';
+
+/**
+ *
+ * @param {Function} mainFn
+ * @param {any[]} args
+ * @param {any[]} deps
+ * @return {string}
+ */
+function stringify(mainFn, args, deps) {
+  const argsSerialized = ExecutionContext.serializeArguments(args);
+  const depsSerialized = ExecutionContext.prototype._serializeDeps(deps);
+  const expression = `(() => {
+    ${depsSerialized}
+    return (${mainFn})(${argsSerialized});
+  })()`;
+  return expression;
+}
+
+const fakeWindow = {};
+fakeWindow.Node = class FakeNode {
+  querySelectorAll() {
+    return [
+      Object.assign(new HTMLElement(), {innerText: 'interesting'}),
+      Object.assign(new HTMLElement(), {innerText: 'not so interesting'}),
+    ];
+  }
+};
+fakeWindow.Element = class FakeElement extends fakeWindow.Node {
+  matches() {
+    return true;
+  }
+};
+fakeWindow.HTMLElement = class FakeHTMLElement extends fakeWindow.Element {};
+
+// @ts-expect-error
+globalThis.window = fakeWindow;
+// @ts-expect-error
+globalThis.document = new fakeWindow.Node();
+globalThis.HTMLElement = globalThis.window.HTMLElement;
+
+/**
+ * @param {HTMLElement[]} elements
+ * @return {HTMLElement[]}
+ */
+function filterInterestingHtmlElements(elements) {
+  return elements.filter(e => e.innerText === 'interesting');
+}
+
+function mainFn() {
+  const el = Object.assign(new HTMLElement(), {
+    tagName: 'FakeHTMLElement',
+    innerText: 'contents',
+    classList: [],
+  });
+  /** @type {HTMLElement[]} */
+  // @ts-expect-error
+  const elements = getElementsInDocument(el);
+  return filterInterestingHtmlElements(elements);
+}
+
+// Indirect eval so code is run in global scope, and won't have incidental access to the
+// esbuild keepNames function wrapper.
+const indirectEval = eval;
+const result = indirectEval(stringify(mainFn, [], [
+  pageFunctions.getElementsInDocument,
+  filterInterestingHtmlElements,
+]));
+if (!result || result.length !== 1 || result[0].innerText !== 'interesting') {
+  throw new Error(`unexpected result, got ${JSON.stringify(result, null, 2)}`);
+}

build/test/page-functions-test-case-getNodeDetails.js

@@ -0,0 +1,65 @@
+/**
+ * @license Copyright 2023 The Lighthouse Authors. All Rights Reserved.
+ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
+ * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
+*/
+
+/* eslint-disable no-undef */
+
+import {ExecutionContext} from '../../core/gather/driver/execution-context.js';
+import {pageFunctions} from '../../core/lib/page-functions.js';
+
+/**
+ *
+ * @param {Function} mainFn
+ * @param {any[]} args
+ * @param {any[]} deps
+ * @return {string}
+ */
+function stringify(mainFn, args, deps) {
+  const argsSerialized = ExecutionContext.serializeArguments(args);
+  const depsSerialized = ExecutionContext.prototype._serializeDeps(deps);
+  const expression = `(() => {
+    ${depsSerialized}
+    return (${mainFn})(${argsSerialized});
+  })()`;
+  return expression;
+}
+
+const fakeWindow = {
+  HTMLElement: class FakeHTMLElement {
+    getAttribute() {
+      return '';
+    }
+
+    getBoundingClientRect() {
+      return {left: 42};
+    }
+  },
+};
+
+// @ts-expect-error
+globalThis.window = fakeWindow;
+globalThis.HTMLElement = globalThis.window.HTMLElement;
+// @ts-expect-error
+globalThis.ShadowRoot = class FakeShadowRoot {};
+// @ts-expect-error
+globalThis.Node = {DOCUMENT_FRAGMENT_NODE: 11};
+
+function mainFn() {
+  const el = Object.assign(new HTMLElement(), {
+    tagName: 'FakeHTMLElement',
+    innerText: 'contents',
+    classList: [],
+  });
+  // @ts-expect-error
+  return getNodeDetails(el);
+}
+
+// Indirect eval so code is run in global scope, and won't have incidental access to the
+// esbuild keepNames function wrapper.
+const indirectEval = eval;
+const result = indirectEval(stringify(mainFn, [], [pageFunctions.getNodeDetails]));
+if (result.lhId !== 'page-0-FakeHTMLElement' || result.boundingRect.left !== 42) {
+  throw new Error(`unexpected result, got ${JSON.stringify(result, null, 2)}`);
+}

core/gather/driver/execution-context.js

@@ -108,7 +108,7 @@ class ExecutionContext {
         ${ExecutionContext._cachedNativesPreamble};
         globalThis.__lighthouseExecutionContextUniqueIdentifier =
           ${uniqueExecutionContextIdentifier};
-        ${pageFunctions.esbuildFunctionNameStubString}
+        ${pageFunctions.esbuildFunctionWrapperString}
         return new Promise(function (resolve) {
           return Promise.resolve()
             .then(_ => ${expression})
@@ -164,13 +164,20 @@ class ExecutionContext {
    * @return {string}
    */
   _serializeDeps(deps) {
-    deps = [pageFunctions.esbuildFunctionNameStubString, ...deps || []];
+    deps = [pageFunctions.esbuildFunctionWrapperString, ...deps || []];
     return deps.map(dep => {
       if (typeof dep === 'function') {
         // esbuild will change the actual function name (ie. function actualName() {})
-        // always, despite minification settings, but preserve the real name in `actualName.name`
-        // (see esbuildFunctionNameStubString).
-        return `const ${dep.name} = ${dep}`;
+        // always, and preserve the real name in `actualName.name`.
+        // See esbuildFunctionWrapperString.
+        const output = dep.toString();
+        const runtimeName = pageFunctions.getRuntimeFunctionName(dep);
+        if (runtimeName !== dep.name) {
+          // In addition to exposing the mangled name, also expose the original as an alias.
+          return `${output}; const ${dep.name} = ${pageFunctions.getRuntimeFunctionName(dep)};`;
+        } else {
+          return output;
+        }
       } else {
         return dep;
       }

core/gather/gatherers/trace-elements.js

@@ -23,6 +23,7 @@ import {ProcessedNavigation} from '../../computed/processed-navigation.js';
 import {LighthouseError} from '../../lib/lh-error.js';
 import {Responsiveness} from '../../computed/metrics/responsiveness.js';
 import {CumulativeLayoutShift} from '../../computed/metrics/cumulative-layout-shift.js';
+import {ExecutionContext} from '../driver/execution-context.js';
 
 /** @typedef {{nodeId: number, score?: number, animations?: {name?: string, failureReasonsMask?: number, unsupportedProperties?: string[]}[], type?: string}} TraceElementData */
 
@@ -284,12 +285,15 @@ class TraceElements extends BaseGatherer {
         try {
           const objectId = await resolveNodeIdToObjectId(session, backendNodeId);
           if (!objectId) continue;
+
+          const deps = ExecutionContext.prototype._serializeDeps([
+            pageFunctions.getNodeDetails,
+            getNodeDetailsData,
+          ]);
           response = await session.sendCommand('Runtime.callFunctionOn', {
             objectId,
             functionDeclaration: `function () {
-              ${pageFunctions.esbuildFunctionNameStubString}
-              ${getNodeDetailsData.toString()};
-              ${pageFunctions.getNodeDetails};
+              ${deps}
               return getNodeDetailsData.call(this);
             }`,
             returnByValue: true,