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.

Sign up for GitHub

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

Jump to bottom

deps: remove details-element-polyfill and rimraf #12369

Merged
merged 4 commits into from
Apr 23, 2021
Merged

deps: remove details-element-polyfill and rimraf #12369

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
8ab38cf
deps: remove details-element-polyfill
brendankenny Apr 15, 2021
17d5484
deps: remove rimraf
brendankenny Apr 15, 2021
6c1f0b2
squash jest excess
brendankenny Apr 15, 2021
435676e
asset-saver fix and test
brendankenny Apr 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion build/build-bundle.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ async function browserifyFile(entryPath, distPath) {
.ignore('intl')
.ignore('intl-pluralrules')
.ignore('raven')
.ignore('rimraf')
.ignore('pako/lib/zlib/inflate.js');

// Don't include the desktop protocol connection.
Expand Down
3 changes: 1 addition & 2 deletions build/build-dt-report-resources.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@
const browserify = require('browserify');
const fs = require('fs');
const path = require('path');
const rimraf = require('rimraf');
const assert = require('assert').strict;

const distDir = path.join(__dirname, '..', 'dist', 'dt-report-resources');
Expand All @@ -26,7 +25,7 @@ function writeFile(name, content) {
fs.writeFileSync(`${distDir}/${name}`, content);
}

rimraf.sync(distDir);
fs.rmdirSync(distDir, {recursive: true});
fs.mkdirSync(distDir);

writeFile('report.js', htmlReportAssets.REPORT_JAVASCRIPT);
Expand Down
3 changes: 1 addition & 2 deletions build/gh-pages-app.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@ const cpy = require('cpy');
const ghPages = require('gh-pages');
const glob = require('glob');
const lighthousePackage = require('../package.json');
const rimraf = require('rimraf');
const terser = require('terser');

const ghPagesDistDir = `${__dirname}/../dist/gh-pages`;
Expand Down Expand Up @@ -81,7 +80,7 @@ class GhPagesApp {
}

async build() {
rimraf.sync(this.distDir);
fs.rmdirSync(this.distDir, {recursive: true});

const html = this._compileHtml();
safeWriteFile(`${this.distDir}/index.html`, html);
Expand Down
5 changes: 2 additions & 3 deletions lighthouse-cli/test/smokehouse/lighthouse-runners/cli.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ const {promisify} = require('util');
const execFileAsync = promisify(require('child_process').execFile);

const log = require('lighthouse-logger');
const rimraf = promisify(require('rimraf'));

const assetSaver = require('../../../../lighthouse-core/lib/asset-saver.js');
const LocalConsole = require('../lib/local-console.js');
Expand All @@ -36,8 +35,8 @@ async function runLighthouse(url, configJson, testRunnerOptions = {}) {

const {isDebug} = testRunnerOptions;
return internalRun(url, tmpPath, configJson, isDebug)
// Wait for internalRun() before rimraffing scratch directory.
.finally(() => !isDebug && rimraf(tmpPath));
// Wait for internalRun() before rmdiring scratch directory.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
// Wait for internalRun() before rmdiring scratch directory.
// Wait for internalRun() before removing scratch directory.

.finally(() => !isDebug && fs.rmdir(tmpPath, {recursive: true}));
}

/**
Expand Down
5 changes: 2 additions & 3 deletions lighthouse-core/lib/asset-saver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@ const {promisify} = require('util');
const Simulator = require('./dependency-graph/simulator/simulator.js');
const lanternTraceSaver = require('./lantern-trace-saver.js');
const Metrics = require('./traces/pwmetrics-events.js');
const rimraf = require('rimraf');
const NetworkAnalysisComputed = require('../computed/network-analysis.js');
const LoadSimulatorComputed = require('../computed/load-simulator.js');
const LHError = require('../lib/lh-error.js');
Expand Down Expand Up @@ -102,8 +101,8 @@ async function saveArtifacts(artifacts, basePath) {
const status = {msg: 'Saving artifacts', id: 'lh:assetSaver:saveArtifacts'};
log.time(status);
fs.mkdirSync(basePath, {recursive: true});
rimraf.sync(`${basePath}/*${traceSuffix}`);
rimraf.sync(`${basePath}/${artifactsFilename}`);
fs.rmdirSync(`${basePath}/*${traceSuffix}`, {recursive: true});
Copy link
Collaborator

Choose a reason for hiding this comment

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

glob works?

Copy link
Member Author

Choose a reason for hiding this comment

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

glob works?

no, and in fact doing this with fs.rmdir also makes no sense. Thanks for catching :)

Copy link
Member Author

Choose a reason for hiding this comment

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

in fact doing this with fs.rmdir also makes no sense

fs.rm() will let us handle files and directories (with and without recursive: true, respectively), but not until node 14, unfortunately

Copy link
Member Author

Choose a reason for hiding this comment

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

fixed and added a test to catch this next time

fs.rmdirSync(`${basePath}/${artifactsFilename}`, {recursive: true});

const {traces, devtoolsLogs, ...restArtifacts} = artifacts;

Expand Down
3 changes: 0 additions & 3 deletions lighthouse-core/report/html/html-report-assets.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,6 @@ const REPORT_TEMPLATE = fs.readFileSync(__dirname + '/report-template.html', 'ut
const REPORT_JAVASCRIPT = [
fs.readFileSync(__dirname + '/renderer/util.js', 'utf8'),
fs.readFileSync(__dirname + '/renderer/dom.js', 'utf8'),
// COMPAT: Remove when Microsoft Edge supports <details>/<summary>
// https://developer.microsoft.com/en-us/microsoft-edge/platform/status/detailssummary/?q=details
fs.readFileSync(require.resolve('details-element-polyfill'), 'utf8'),
fs.readFileSync(__dirname + '/renderer/details-renderer.js', 'utf8'),
fs.readFileSync(__dirname + '/renderer/crc-details-renderer.js', 'utf8'),
fs.readFileSync(__dirname + '/renderer/snippet-renderer.js', 'utf8'),
Expand Down
4 changes: 0 additions & 4 deletions lighthouse-core/report/html/readme.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,3 @@ The renderer was designed to be portable across various environments.
1. _LH CLI_: It [creates the HTML as the runner finishes up](https://github.com/GoogleChrome/lighthouse/blob/440155cdda377c458c0efce006bc3a69ce2a351c/lighthouse-core/runner.js#L137-L138) and [saves it to disk](https://github.com/GoogleChrome/lighthouse/blob/440155cdda377c458c0efce006bc3a69ce2a351c/lighthouse-cli/printer.js#L71-L92).
1. _Chrome DevTools Audits Panel_: The `renderer` files are rolled into the Chromium repo, and they execute within the DevTools context. The audits panel [receives the LHR object from a WebWorker](https://github.com/ChromeDevTools/devtools-frontend/blob/aa1532c2f8bdc37c9886255644ed90ad01c61c77/front_end/audits/AuditsProtocolService.js#L27-L35), through a `postMessage` and then runs [the renderer within DevTools UI](https://github.com/ChromeDevTools/devtools-frontend/blob/aa1532c2f8bdc37c9886255644ed90ad01c61c77/front_end/audits/AuditsPanel.js#L123-L157), making a few [simplifications](https://github.com/ChromeDevTools/devtools-frontend/blob/master/front_end/audits/AuditsReportRenderer.js).
1. _Hosted [Lighthouse Viewer](https://googlechrome.github.io/lighthouse/viewer/)_: It's a webapp that has the renderer (along with some [additional features](https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-core/report/html/renderer/report-ui-features.js)) all compiled into a [`viewer.js`](https://googlechrome.github.io/lighthouse/viewer/src/viewer.js) file. Same [basic approach](https://github.com/GoogleChrome/lighthouse/blob/440155cdda377c458c0efce006bc3a69ce2a351c/lighthouse-viewer/app/src/lighthouse-report-viewer.js#L116-L117) there.

### Polyfills

The [`details-element-polyfill`](https://www.npmjs.com/package/details-element-polyfill/v/2.1.1) is pulled in to provide [support](https://caniuse.com/#feat=details) for Microsoft Edge.
5 changes: 0 additions & 5 deletions lighthouse-core/report/html/report-styles.css
View file
Open in desktop

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

3 changes: 1 addition & 2 deletions lighthouse-core/scripts/build-report-for-autodeployment.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
/* eslint-disable no-console */
const fs = require('fs');
const path = require('path');
const rimraf = require('rimraf').sync;
const swapLocale = require('../lib/i18n/swap-locale.js');

const ReportGenerator = require('../../lighthouse-core/report/report-generator.js');
Expand Down Expand Up @@ -125,6 +124,6 @@ async function generateErrorLHR() {
appleTouchIconAudit.scoreDisplayMode = 'binary';
appleTouchIconAudit.score = 1;

rimraf(TMP);
fs.rmdirSync(TMP, {recursive: true});
return errorLhr;
}
3 changes: 1 addition & 2 deletions lighthouse-core/scripts/gcp-collection/fleet-create-directories.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,14 @@

const fs = require('fs');
const path = require('path');
const rimraf = require('rimraf');

const LH_ROOT = path.join(__dirname, '../../../');
const TMP_DIR = path.join(LH_ROOT, '.tmp/gcp-instances');
const URLS_LIST = process.argv[2]
? path.resolve(process.cwd(), process.argv[2])
: path.join(__dirname, 'urls.txt');

rimraf.sync(TMP_DIR);
fs.rmdirSync(TMP_DIR, {recursive: true});
fs.mkdirSync(TMP_DIR);

const MACHINE_BASE_INDEX = 0;
Expand Down
3 changes: 1 addition & 2 deletions lighthouse-core/scripts/lantern/collect/golden.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
/** @typedef {import('../run-on-all-assets.js').Golden} Golden */

const fs = require('fs');
const rimraf = require('rimraf');
const common = require('./common.js');

/**
Expand Down Expand Up @@ -110,7 +109,7 @@ async function main() {
/** @type {Golden} */
const golden = {sites: goldenSites};

rimraf.sync(common.goldenFolder);
fs.rmdirSync(common.goldenFolder, {recursive: true});
fs.mkdirSync(common.goldenFolder);
saveGoldenData('site-index-plus-golden-expectations.json', JSON.stringify(golden, null, 2));
for (const result of goldenSites) {
Expand Down
3 changes: 1 addition & 2 deletions lighthouse-core/test/lib/asset-saver-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ const assetSaver = require('../../lib/asset-saver.js');
const Metrics = require('../../lib/traces/pwmetrics-events.js');
const assert = require('assert').strict;
const fs = require('fs');
const rimraf = require('rimraf');
const LHError = require('../../lib/lh-error.js');

const traceEvents = require('../fixtures/traces/progressive-app.json');
Expand Down Expand Up @@ -222,7 +221,7 @@ describe('asset-saver helper', () => {
const outputPath = __dirname + '/json-serialization-test-data/';

afterEach(() => {
rimraf.sync(outputPath);
fs.rmdirSync(outputPath, {recursive: true});
});

it('round trips saved artifacts', async () => {
Expand Down
5 changes: 2 additions & 3 deletions lighthouse-core/test/runner-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@ const assetSaver = require('../lib/asset-saver.js');
const fs = require('fs');
const assert = require('assert').strict;
const path = require('path');
const rimraf = require('rimraf');
const LHError = require('../lib/lh-error.js');
const i18n = require('../lib/i18n/i18n.js');

Expand Down Expand Up @@ -77,7 +76,7 @@ describe('Runner', () => {
const resolvedPath = path.resolve(process.cwd(), artifactsPath);

afterAll(() => {
rimraf.sync(resolvedPath);
fs.rmdirSync(resolvedPath, {recursive: true});
});

it('-G gathers, quits, and doesn\'t run audits', () => {
Expand Down Expand Up @@ -435,7 +434,7 @@ describe('Runner', () => {
assert.strictEqual(auditResult.scoreDisplayMode, 'error');
assert.ok(auditResult.errorMessage.includes(errorMessage));

rimraf.sync(resolvedPath);
fs.rmdirSync(resolvedPath, {recursive: true});
});

it('only passes the requested artifacts to the audit (no optional artifacts)', async () => {
Expand Down
5 changes: 1 addition & 4 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@
"build-viewer": "node ./build/build-viewer.js",
"reset-link": "(yarn unlink || true) && yarn link && yarn link lighthouse",
"c8": "bash lighthouse-core/scripts/c8.sh",
"clean": "rimraf dist proto/scripts/*.json proto/scripts/*_pb2.* proto/scripts/*_pb.* proto/scripts/__pycache__ proto/scripts/*.pyc *.report.html *.report.dom.html *.report.json *.devtoolslog.json *.trace.json lighthouse-core/lib/i18n/locales/*.ctc.json || true",
"clean": "rm -r dist proto/scripts/*.json proto/scripts/*_pb2.* proto/scripts/*_pb.* proto/scripts/__pycache__ proto/scripts/*.pyc *.report.html *.report.dom.html *.report.json *.devtoolslog.json *.trace.json lighthouse-core/lib/i18n/locales/*.ctc.json || true",
Copy link
Collaborator

Choose a reason for hiding this comment

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

breaks for windows, doesn't it?

Copy link
Collaborator

Choose a reason for hiding this comment

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

dont really care if it does. seems reasonable to support only bash-in-windows and not the windows command line.

Copy link
Member Author

Choose a reason for hiding this comment

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

dont really care if it does. seems reasonable to support only bash-in-windows and not the windows command line.

Yeah, I also don't think it matters. I honestly never run this because it also deletes *.report.html *.report.json *.devtoolslog.json *.trace.json and I usually have at least one of those lying around I don't want deleted right now, but I'm sure someone out there likes a tidy checkout.

"lint": "[ \"$CI\" = true ] && eslint --quiet -f codeframe . || eslint .",
"smoke": "node lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js",
"debug": "node --inspect-brk ./lighthouse-cli/index.js",
Expand Down Expand Up @@ -109,7 +109,6 @@
"@types/puppeteer": "1.19.x",
"@types/raven": "^2.5.1",
"@types/resize-observer-browser": "^0.1.1",
"@types/rimraf": "^2.0.2",
"@types/semver": "^5.5.0",
"@types/update-notifier": "^4.1.0",
"@types/ws": "^4.0.1",
Expand Down Expand Up @@ -159,7 +158,6 @@
"configstore": "^5.0.1",
"csp_evaluator": "^1.0.1",
"cssstyle": "1.2.1",
"details-element-polyfill": "^2.4.0",
"enquirer": "^2.3.6",
"http-link-header": "^0.8.0",
"intl": "^1.2.5",
Expand All @@ -180,7 +178,6 @@
"parse-cache-control": "1.0.1",
"ps-list": "^7.2.0",
"raven": "^2.2.1",
"rimraf": "^2.6.1",
"robots-parser": "^2.0.1",
"semver": "^5.3.0",
"speedline-core": "^1.4.3",
Expand Down