Skip to content

Commit

Permalink
report: extract independent report-generator types (#12940)
Browse files Browse the repository at this point in the history
  • Loading branch information
brendankenny authored Aug 19, 2021
1 parent 21eb8d3 commit e75ab32
Show file tree
Hide file tree
Showing 33 changed files with 88 additions and 60 deletions.
2 changes: 1 addition & 1 deletion build/build-bundle.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ async function browserifyFile(entryPath, distPath) {
// 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/report-assets.js'));
bundle.ignore(require.resolve('../report/generator/report-assets.js'));
}

// Expose the audits, gatherers, and computed artifacts so they can be dynamically loaded.
Expand Down
6 changes: 3 additions & 3 deletions build/build-dt-report-resources.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,8 @@ const {LH_ROOT} = require('../root.js');

const distDir = path.join(LH_ROOT, 'dist', 'dt-report-resources');
const bundleOutFile = `${distDir}/report-generator.js`;
const generatorFilename = `./report/report-generator.js`;
const htmlReportAssets = require('../report/report-assets.js');
const generatorFilename = `./report/generator/report-generator.js`;
const htmlReportAssets = require('../report/generator/report-assets.js');

/**
* Used to save cached resources (Runtime.cachedResources).
Expand All @@ -38,7 +38,7 @@ writeFile('report-generator.d.ts', 'export {}');

const pathToReportAssets = require.resolve('../clients/devtools-report-assets.js');
browserify(generatorFilename, {standalone: 'Lighthouse.ReportGenerator'})
// Shims './report/report-assets.js' to resolve to devtools-report-assets.js
// Shims './report/generator/report-assets.js' to resolve to devtools-report-assets.js
.require(pathToReportAssets, {expose: './report-assets.js'})
.bundle((err, src) => {
if (err) throw err;
Expand Down
4 changes: 2 additions & 2 deletions build/build-lightrider-bundles.js
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ const distDir = path.join(LH_ROOT, 'dist', 'lightrider');
const sourceDir = path.join(LH_ROOT, 'clients', 'lightrider');

const bundleOutFile = `${distDir}/report-generator-bundle.js`;
const generatorFilename = `./report/report-generator.js`;
const generatorFilename = `./report/generator/report-generator.js`;

const entrySourceName = 'lightrider-entry.js';
const entryDistName = 'lighthouse-lr-bundle.js';
Expand All @@ -39,7 +39,7 @@ function buildEntryPoint() {
function buildReportGenerator() {
browserify(generatorFilename, {standalone: 'ReportGenerator'})
// Flow report is not used in LR, so don't include flow assets.
.ignore(require.resolve('../report/flow-report-assets.js'))
.ignore(require.resolve('../report/generator/flow-report-assets.js'))
// Transform the fs.readFile etc into inline strings.
.transform('@wardpeet/brfs', {
readFileTransform: minifyFileTransform,
Expand Down
4 changes: 2 additions & 2 deletions build/build-sample-reports.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,12 +11,12 @@
const fs = require('fs');
const path = require('path');
const swapLocale = require('../lighthouse-core/lib/i18n/swap-locale.js');
const ReportGenerator = require('../lighthouse-core/../report/report-generator.js');
const ReportGenerator = require('../report/generator/report-generator.js');
const {defaultSettings} = require('../lighthouse-core/config/constants.js');
const lighthouse = require('../lighthouse-core/index.js');
const lhr = /** @type {LH.Result} */ (require('../lighthouse-core/test/results/sample_v2.json'));
const {LH_ROOT} = require('../root.js');
const htmlReportAssets = require('../lighthouse-core/../report/report-assets.js');
const htmlReportAssets = require('../report/generator/report-assets.js');

/** @type {LH.FlowResult} */
const flowResult = JSON.parse(
Expand Down
6 changes: 3 additions & 3 deletions build/build-viewer.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,18 +8,18 @@
const browserify = require('browserify');
const GhPagesApp = require('./gh-pages-app.js');
const {minifyFileTransform} = require('./build-utils.js');
const htmlReportAssets = require('../report/report-assets.js');
const htmlReportAssets = require('../report/generator/report-assets.js');
const {LH_ROOT} = require('../root.js');

/**
* Build viewer, optionally deploying to gh-pages if `--deploy` flag was set.
*/
async function run() {
// JS bundle from browserified ReportGenerator.
const generatorFilename = `${LH_ROOT}/report/report-generator.js`;
const generatorFilename = `${LH_ROOT}/report/generator/report-generator.js`;
const generatorBrowserify = browserify(generatorFilename, {standalone: 'ReportGenerator'})
// Flow report is not used in report viewer, so don't include flow assets.
.ignore(require.resolve('../report/flow-report-assets.js'))
.ignore(require.resolve('../report/generator/flow-report-assets.js'))
.transform('@wardpeet/brfs', {
readFileTransform: minifyFileTransform,
});
Expand Down
2 changes: 1 addition & 1 deletion clients/devtools-report-assets.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
/**
* @fileoverview Instead of loading report assets form the filesystem, in Devtools we must load
* them via Runtime.cachedResources. We use this module to shim
* report/report-assets.js in Devtools.
* report/generator/report-assets.js in Devtools.
*/

/* global globalThis */
Expand Down
2 changes: 1 addition & 1 deletion docs/hacking-tips.md
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ node generate_report.js > temp.report.html; open temp.report.html
// generate_report.js
'use strict';

const ReportGenerator = require('./report/report-generator.js');
const ReportGenerator = require('./report/generator/report-generator.js');
const results = require('./temp.report.json');
const html = ReportGenerator.generateReportHtml(results);

Expand Down
2 changes: 1 addition & 1 deletion lighthouse-cli/run.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ const ChromeLauncher = require('chrome-launcher');
const yargsParser = require('yargs-parser');
const lighthouse = require('../lighthouse-core/index.js');
const log = require('lighthouse-logger');
const getFilenamePrefix = require('../lighthouse-core/lib/file-namer.js').getFilenamePrefix;
const getFilenamePrefix = require('../report/generator/file-namer.js').getFilenamePrefix;
const assetSaver = require('../lighthouse-core/lib/asset-saver.js');
const URL = require('../lighthouse-core/lib/url-shim.js');

Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const fs = require('fs');
const path = require('path');
const URL = require('./lib/url-shim.js');
const Sentry = require('./lib/sentry.js');
const generateReport = require('../report/report-generator.js').generateReport;
const generateReport = require('../report/generator/report-generator.js').generateReport;
const LHError = require('./lib/lh-error.js');

/** @typedef {import('./gather/connections/connection.js')} Connection */
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/scripts/build-test-flow-report.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ const open = require('open');
const {execFileSync} = require('child_process');

execFileSync(`yarn`, ['build-report']);
const reportGenerator = require('../../report/report-generator.js');
const reportGenerator = require('../../report/generator/report-generator.js');

const flow = JSON.parse(fs.readFileSync(
`${__dirname}/../test/fixtures/fraggle-rock/reports/sample-lhrs.json`,
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/scripts/update-flow-fixtures.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const fs = require('fs');
const open = require('open');
const puppeteer = require('puppeteer');
const lighthouse = require('../fraggle-rock/api.js');
const reportGenerator = require('../../report/report-generator.js');
const reportGenerator = require('../../report/generator/report-generator.js');

(async () => {
const browser = await puppeteer.launch({headless: false, slowMo: 50});
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-viewer/app/src/github-api.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@

import idbKeyval from 'idb-keyval';
import {FirebaseAuth} from './firebase-auth.js';
import {getFilenamePrefix} from '../../../report/renderer/file-namer.js';
import {getFilenamePrefix} from '../../../report/generator/file-namer.js';

/**
* Wrapper around the GitHub API for reading/writing gists.
Expand Down
3 changes: 1 addition & 2 deletions lighthouse-viewer/types/viewer.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,13 +4,12 @@
* 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 _ReportGenerator = require('../../report/report-generator.js');
import _ReportGenerator = require('../../report/generator/report-generator.js');
import {DOM as _DOM} from '../../report/renderer/dom.js';
import {ReportRenderer as _ReportRenderer} from '../../report/renderer/report-renderer.js';
import {ReportUIFeatures as _ReportUIFeatures} from '../../report/renderer/report-ui-features.js';
import {Logger as _Logger} from '../../report/renderer/logger.js';
import {TextEncoding as _TextEncoding} from '../../report/renderer/text-encoding.js';
import {getFilenamePrefix as _getFilenamePrefix} from '../../report/renderer/file-namer.js';
import {LighthouseReportViewer as _LighthouseReportViewer} from '../app/src/lighthouse-report-viewer.js';
import 'google.analytics';
import {FirebaseNamespace} from '@firebase/app-types';
Expand Down
4 changes: 2 additions & 2 deletions report/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,8 +8,8 @@ Example standalone HTML report, delivered by the CLI: [`dbwtest-3.0.3.html`](htt

### Report Renderer components

1. [`report/report-generator.js`](https://github.com/GoogleChrome/lighthouse/blob/master/report/report-generator.js) is the entry point for generating the HTML from Node. It compiles together the HTML string with everything required within it.
- It runs natively in Node.js but can run in the browser after a compile step is applied during our bundling pipeline. That compile step uses a browserify transform that takes any `fs.readFileSync()` calls and replaces them with the stringified file content.
1. [`report/generator/report-generator.js`](https://github.com/GoogleChrome/lighthouse/blob/master/report/generator/report-generator.js) is the entry point for generating the HTML from Node. It compiles together the HTML string with everything required within it.
- It runs natively in Node.js but can run in the browser after a compile step is applied during our bundling pipeline. That compile step uses `brfs`, which takes any `fs.readFileSync()` calls and replaces them with the stringified file content.
1. [`report/renderer`](https://github.com/GoogleChrome/lighthouse/tree/master/report/renderer) are all client-side JS files. They transform an LHR object into a report DOM tree. They are all executed within the browser.
1. [`report/assets/standalone-template.html`](https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-core/report/html/report-template.html) is where the client-side build of the DOM report is typically kicked off ([with these four lines](https://github.com/GoogleChrome/lighthouse/blob/eda3a3e2e271249f261655f9504fd542d6acf0f8/lighthouse-core/report/html/report-template.html#L29-L33)) However, see _Current Uses.._ below for more possibilities.

Expand Down
9 changes: 9 additions & 0 deletions report/generator/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
# Lighthouse Report Generator

## Overview

Lighthouse's report generator is the entry point for creating reports from an **LHR** (Lighthouse Result object). It returns results as HTML, JSON, and CSV.

It runs natively in Node.js but can run in the browser after a compile step is applied during our bundling pipeline. That compile step uses `brfs`, which takes any `fs.readFileSync()` calls and replaces them with the stringified file content.

Because it's shared between core and the report, dependencies (both code and types) should be kept minimal.
File renamed without changes.
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,10 @@
'use strict';

const fs = require('fs');
const {LH_ROOT} = require('../root.js');

/* eslint-disable max-len */
const FLOW_REPORT_TEMPLATE = fs.readFileSync(`${LH_ROOT}/flow-report/assets/standalone-flow-template.html`, 'utf8');
const FLOW_REPORT_JAVASCRIPT = fs.readFileSync(`${LH_ROOT}/dist/report/flow.js`, 'utf8');
const FLOW_REPORT_TEMPLATE = fs.readFileSync(`${__dirname}/../../flow-report/assets/standalone-flow-template.html`, 'utf8');
const FLOW_REPORT_JAVASCRIPT = fs.readFileSync(`${__dirname}/../../dist/report/flow.js`, 'utf8');
/* eslint-enable max-len */

module.exports = {
Expand Down
4 changes: 4 additions & 0 deletions report/generator/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "commonjs",
"//": "Preserve commonjs in this directory. Temporary file until converted to type: module"
}
7 changes: 4 additions & 3 deletions report/report-assets.js → report/generator/report-assets.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,10 @@
const fs = require('fs');
const flowReportAssets = require('./flow-report-assets.js');

const REPORT_TEMPLATE = fs.readFileSync(__dirname + '/assets/standalone-template.html', 'utf8');
const REPORT_JAVASCRIPT = fs.readFileSync(__dirname + '/../dist/report/standalone.js', 'utf8');
const REPORT_CSS = fs.readFileSync(__dirname + '/assets/styles.css', 'utf8');
const REPORT_TEMPLATE = fs.readFileSync(__dirname + '/../assets/standalone-template.html',
'utf8');
const REPORT_JAVASCRIPT = fs.readFileSync(__dirname + '/../../dist/report/standalone.js', 'utf8');
const REPORT_CSS = fs.readFileSync(__dirname + '/../assets/styles.css', 'utf8');

// Changes to this export interface should be reflected in build/build-dt-report-resources.js
// and clients/devtools-report-assets.js
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,9 @@

const htmlReportAssets = require('./report-assets.js');

/** @typedef {import('../../types/lhr/lhr').default} LHResult */
/** @typedef {import('../../types/lhr/flow').default} FlowResult */

class ReportGenerator {
/**
* Replaces all the specified strings in source without serial replacements.
Expand Down Expand Up @@ -40,7 +43,7 @@ class ReportGenerator {

/**
* Returns the standalone report HTML as a string with the report JSON and renderer JS inlined.
* @param {LH.Result} lhr
* @param {LHResult} lhr
* @return {string}
*/
static generateReportHtml(lhr) {
Expand All @@ -58,7 +61,7 @@ class ReportGenerator {

/**
* Returns the standalone flow report HTML as a string with the report JSON and renderer JS inlined.
* @param {LH.FlowResult} flow
* @param {FlowResult} flow
* @return {string}
*/
static generateFlowReportHtml(flow) {
Expand All @@ -81,7 +84,7 @@ class ReportGenerator {
* - the score type that is used for the audit
* - the score value of the audit
*
* @param {LH.Result} lhr
* @param {LHResult} lhr
* @return {string}
*/
static generateReportCSV(lhr) {
Expand Down Expand Up @@ -118,8 +121,8 @@ class ReportGenerator {

/**
* Creates the results output in a format based on the `mode`.
* @param {LH.Result} lhr
* @param {LH.Config.Settings['output']} outputModes
* @param {LHResult} lhr
* @param {LHResult['configSettings']['output']} outputModes
* @return {string|string[]}
*/
static generateReport(lhr, outputModes) {
Expand Down
28 changes: 28 additions & 0 deletions report/generator/tsconfig.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
{
"compilerOptions": {
"composite": true,
"outDir": "../../.tmp/tsbuildinfo/report/generator",
"emitDeclarationOnly": true,
"declarationMap": true,

"target": "ES2020",
"module": "commonjs",
"moduleResolution": "node",

"allowJs": true,
"checkJs": true,
"strict": true,
// TODO: remove the next line to be fully `strict`.
"useUnknownInCatchVariables": false,

// "listFiles": true,
// "noErrorTruncation": true,
"extendedDiagnostics": true,
},
"references": [
{"path": "../../types/lhr/"},
],
"include": [
"**/*.js",
],
}
File renamed without changes.
8 changes: 0 additions & 8 deletions report/renderer/file-namer.js

This file was deleted.

4 changes: 0 additions & 4 deletions report/renderer/package.json

This file was deleted.

2 changes: 1 addition & 1 deletion report/renderer/report-ui-features.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@

/** @typedef {import('./dom').DOM} DOM */

import {getFilenamePrefix} from './file-namer.js';
import {getFilenamePrefix} from '../../report/generator/file-namer.js';
import {ElementScreenshotRenderer} from './element-screenshot-renderer.js';
import {TextEncoding} from './text-encoding.js';
import {Util} from './util.js';
Expand Down
4 changes: 0 additions & 4 deletions report/test/clients/package.json

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
'use strict';

const assert = require('assert').strict;
const getFilenamePrefix = require('../../lib/file-namer.js').getFilenamePrefix;
const getFilenamePrefix = require('../../generator/file-namer.js').getFilenamePrefix;

/* eslint-env jest */
describe('file-namer helper', () => {
Expand Down
4 changes: 4 additions & 0 deletions report/test/generator/package.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
{
"type": "commonjs",
"//": "Preserve commonjs in this directory. Temporary file until converted to type: module"
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,8 @@

const assert = require('assert').strict;
const fs = require('fs');
const ReportGenerator = require('../report-generator.js');
const sampleResults = require('../../lighthouse-core/test/results/sample_v2.json');
const ReportGenerator = require('../../generator/report-generator.js');
const sampleResults = require('../../../lighthouse-core/test/results/sample_v2.json');
const csvValidator = require('csv-validator');

/* eslint-env jest */
Expand Down
4 changes: 0 additions & 4 deletions report/test/renderer/package.json

This file was deleted.

2 changes: 1 addition & 1 deletion report/test/renderer/report-ui-features-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

import {strict as assert} from 'assert';
import jsdom from 'jsdom';
import reportAssets from '../../report-assets.js';
import reportAssets from '../../generator/report-assets.js';
import {Util} from '../../renderer/util.js';
import {DOM} from '../../renderer/dom.js';
import {DetailsRenderer} from '../../renderer/details-renderer.js';
Expand Down
3 changes: 2 additions & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,8 @@
"lighthouse-core/lib/large-javascript-libraries/bundlephobia-database.json",
],
"references": [
{"path": "./types/lhr/"}
{"path": "./types/lhr/"},
{"path": "./report/generator/"}
],
"exclude": [
"lighthouse-core/test/audits/**/*.js",
Expand Down
2 changes: 1 addition & 1 deletion types/lhr/tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
// "noErrorTruncation": true,

"composite": true,
"outDir": "../../.tmp/tsbuildinfo/lhr",
"outDir": "../../.tmp/tsbuildinfo/types/lhr",
},
"include": [
"*.d.ts",
Expand Down

0 comments on commit e75ab32

Please sign in to comment.