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: update eslint to latest #12333

Merged
merged 2 commits into from
Apr 5, 2021
Merged

deps: update eslint to latest #12333

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
1b83aec
deps: update eslint to latest
brendankenny Apr 2, 2021
7018a25
@typescript-eslint/parser, too
brendankenny Apr 5, 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
22 changes: 12 additions & 10 deletions lighthouse-core/gather/driver.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -336,22 +336,24 @@ class Driver {
sendCommandToSession(method, sessionId, ...params) {
const timeout = this._nextProtocolTimeout;
this._nextProtocolTimeout = DEFAULT_PROTOCOL_TIMEOUT;
return new Promise(async (resolve, reject) => {
const asyncTimeout = setTimeout((() => {

Copy link
Member Author

Choose a reason for hiding this comment

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

this change is from no-async-promise-executor. Usually reasonable, but to be fair the reasoning "if an async executor function throws an error, the error will be lost" is exactly why we have the try/catch in there.

Rather than ignoring the error, though, this new form is how we already handle adding a timeout in most of the rest of the code base, so it seems reasonable to use the same style and now pass the lint check.

/** @type {NodeJS.Timer|undefined} */
let asyncTimeout;
const timeoutPromise = new Promise((resolve, reject) => {
asyncTimeout = setTimeout((() => {
const err = new LHError(
LHError.errors.PROTOCOL_TIMEOUT,
{protocolMethod: method}
);
reject(err);
}), timeout);
try {
const result = await this._innerSendCommand(method, sessionId, ...params);
resolve(result);
} catch (err) {
reject(err);
} finally {
clearTimeout(asyncTimeout);
}
});

return Promise.race([
this._innerSendCommand(method, sessionId, ...params),
timeoutPromise,
]).finally(() => {
asyncTimeout && clearTimeout(asyncTimeout);
});
}

Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/gather/gatherers/dobetterweb/tags-blocking-first-paint.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,7 @@ function installMediaListener() {
// @ts-expect-error - `___linkMediaChanges` created above.
window.___linkMediaChanges.push(mediaChange);

return this.setAttribute('media', val);
this.setAttribute('media', val);
Copy link
Member Author

@brendankenny brendankenny Apr 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

I think the intention here was to be double sure we were matching the behavior of the replaced property setter...but eslint correctly points out that even if you return a value in a setter the value still goes nowhere, so the return is misleading (and setAttribute() itself returns undefined anyways)

},
});
}
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/gather/gatherers/seo/robots-txt.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

const FRGatherer = require('../../../fraggle-rock/gather/base-gatherer.js');

/* global fetch, URL, location */
/* global fetch, location */

/** @return {Promise<LH.Artifacts['RobotsTxt']>} */
/* c8 ignore start */
Expand Down
2 changes: 0 additions & 2 deletions lighthouse-core/lib/file-namer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,6 @@
*/
'use strict';

/* global URL */

/**
* @fileoverview
* @suppress {reportUnknownTypes}
Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/lib/page-functions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -258,8 +258,8 @@ function computeBenchmarkIndex() {
let iterations = 0;

while (Date.now() - start < 500) {
let s = ''; // eslint-disable-line no-unused-vars
for (let j = 0; j < 10000; j++) s += 'a';
let s = '';
for (let j = 0; j < 10000; j++) s += 'a'; // eslint-disable-line no-unused-vars

iterations++;
}
Expand Down
2 changes: 0 additions & 2 deletions lighthouse-core/lib/url-shim.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,6 @@
* URL shim so we keep our code DRY
*/

/* global URL */

const Util = require('../report/html/renderer/util.js');

/** @typedef {import('./network-request.js')} NetworkRequest */
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/report/html/renderer/details-renderer.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/
'use strict';

/* globals self CriticalRequestChainRenderer SnippetRenderer ElementScreenshotRenderer Util URL */
/* globals self CriticalRequestChainRenderer SnippetRenderer ElementScreenshotRenderer Util */

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

Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/report/html/renderer/dom.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/
'use strict';

/* globals URL self Util */
/* globals self Util */

/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementByTagName */
/** @template {string} T @typedef {import('typed-query-selector/parser').ParseSelector<T, Element>} ParseSelector */
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/report/html/renderer/i18n.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@
*/
'use strict';

/* globals self, URL */
/* globals self */

// Not named `NBSP` because that creates a duplicate identifier (util.js).
const NBSP2 = '\xa0';
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/report/html/renderer/util.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@
*/
'use strict';

/* globals self, URL */
/* globals self */

/** @typedef {import('./i18n')} I18n */

Expand Down
12 changes: 6 additions & 6 deletions lighthouse-core/scripts/generate-bundlephobia-database.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -64,12 +64,12 @@ function hasBeenRecentlyScraped(library) {
* @return {library is BundlePhobiaLibrary}
*/
function validateLibraryObject(library) {
return library.hasOwnProperty('name') &&
library.hasOwnProperty('size') &&
library.hasOwnProperty('gzip') &&
library.hasOwnProperty('description') &&
library.hasOwnProperty('repository') &&
library.hasOwnProperty('version') &&
return typeof library.name === 'string' &&
typeof library.size === 'number' &&
typeof library.gzip === 'number' &&
typeof library.description === 'string' &&
typeof library.repository === 'string' &&
typeof library.version === 'string' &&
Copy link
Member Author

@brendankenny brendankenny Apr 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

this lint rule is for preventing risks due to prototype pollution that has no bearing on us here, but I'm also not sure what the intention of using hasOwnProperty was. Checking typeof seems more straightforward and is stricter to boot.

!library.version.match(/^([0-9]+) packages$/);
}

Expand Down
4 changes: 2 additions & 2 deletions lighthouse-core/test/audits/dobetterweb/no-vulnerable-libraries-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -107,9 +107,9 @@ describe('Snyk database', () => {

// For every subset ...
for (const subset of range.set) {
// Upperbound exists if...
// Upperbound exists if...

// < or <= is in one of the subset's clauses (= gets normalized to >= and <).
// < or <= is in one of the subset's clauses (= gets normalized to >= and <).
if (subset.some(comparator => comparator.operator && comparator.operator.match(/^</))) {
continue;
}
Expand Down
41 changes: 12 additions & 29 deletions lighthouse-core/test/gather/gatherers/dobetterweb/response-compression-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ const traceData = {
networkRecords: [
{
url: 'http://google.com/index.js',
_statusCode: 200,
statusCode: 200,
mimeType: 'text/javascript',
requestId: 0,
resourceSize: 9,
Expand All @@ -33,7 +33,7 @@ const traceData = {
},
{
url: 'http://google.com/index.css',
_statusCode: 200,
statusCode: 200,
mimeType: 'text/css',
requestId: 1,
resourceSize: 6,
Expand All @@ -45,7 +45,7 @@ const traceData = {
},
{
url: 'http://google.com/index.json',
_statusCode: 200,
statusCode: 200,
mimeType: 'application/json',
requestId: 2,
resourceSize: 7,
Expand All @@ -57,7 +57,7 @@ const traceData = {
},
{
url: 'http://google.com/index.json',
_statusCode: 200,
statusCode: 200,
mimeType: 'application/json',
requestId: 27,
resourceSize: 7,
Expand All @@ -70,7 +70,7 @@ const traceData = {
},
{
url: 'http://google.com/index.json',
_statusCode: 304, // ignore for being a cache not modified response
statusCode: 304, // ignore for being a cache not modified response
mimeType: 'application/json',
requestId: 22,
resourceSize: 7,
Expand All @@ -82,7 +82,7 @@ const traceData = {
},
{
url: 'http://google.com/other.json',
_statusCode: 200,
statusCode: 200,
mimeType: 'application/json',
requestId: 23,
resourceSize: 7,
Expand All @@ -94,7 +94,7 @@ const traceData = {
},
{
url: 'http://google.com/index.jpg',
_statusCode: 200,
statusCode: 200,
mimeType: 'image/jpg',
requestId: 3,
resourceSize: 10,
Expand All @@ -106,7 +106,7 @@ const traceData = {
},
{
url: 'http://google.com/helloworld.mp4',
_statusCode: 200,
statusCode: 200,
mimeType: 'video/mp4',
requestId: 4,
resourceSize: 100,
Expand Down Expand Up @@ -136,7 +136,7 @@ describe('Optimized responses', () => {
});

it('returns only text and non encoded responses', () => {
return responseCompression.afterPass(options, createNetworkRequests(traceData))
return responseCompression.afterPass(options, traceData)
.then(artifact => {
assert.equal(artifact.length, 2);
assert.ok(/index\.css$/.test(artifact[0].url));
Expand All @@ -145,7 +145,7 @@ describe('Optimized responses', () => {
});

it('computes sizes', () => {
return responseCompression.afterPass(options, createNetworkRequests(traceData))
return responseCompression.afterPass(options, traceData)
.then(artifact => {
assert.equal(artifact.length, 2);
assert.equal(artifact[0].resourceSize, 6);
Expand All @@ -155,7 +155,7 @@ describe('Optimized responses', () => {

it('recovers from driver errors', () => {
options.driver.getRequestContent = () => Promise.reject(new Error('Failed'));
return responseCompression.afterPass(options, createNetworkRequests(traceData))
return responseCompression.afterPass(options, traceData)
.then(artifact => {
assert.equal(artifact.length, 2);
assert.equal(artifact[0].resourceSize, 6);
Expand Down Expand Up @@ -191,27 +191,10 @@ describe('Optimized responses', () => {
],
};

return responseCompression.afterPass(options, createNetworkRequests(traceData))
return responseCompression.afterPass(options, traceData)
.then(artifact => {
assert.equal(artifact.length, 1);
assert.equal(artifact[0].resourceSize, 123);
});
});

// Change into SDK.networkRequest when examples are ready
function createNetworkRequests(traceData) {
traceData.networkRecords = traceData.networkRecords.map(record => {
record.url = record.url;
record.statusCode = record._statusCode;
record.mimeType = record.mimeType;
record.resourceSize = record.resourceSize;
record.transferSize = record.transferSize;
record.responseHeaders = record.responseHeaders;
record.requestId = record.requestId;

return record;
});
Comment on lines -203 to -213
Copy link
Member Author

@brendankenny brendankenny Apr 5, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

this is handling suuuuper legacy stuff, from back when we used SDK.networkRequest for network request handling and had a split between leading-underscored internal properties and no-underscored external interface, so this was doubling up the values. Over time we migrated off using the underscores directly (and these dropped their underscores one by one until only _statusCode was left) and then stopped using SDK.networkRequest altogether (almost three years ago - #5451), but we never got rid of this thing doing 85% nothing and 15% useless work :)

Anyways, _statusCode isn't a thing anymore except in this file, so 👋


return traceData;
}
});
1 change: 0 additions & 1 deletion lighthouse-core/test/report/html/renderer/util-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ const I18n = require('../../../../report/html/renderer/i18n.js');
const sampleResult = require('../../../results/sample_v2.json');

/* eslint-env jest */
/* global URL */

describe('util helpers', () => {
beforeEach(() => {
Expand Down
4 changes: 2 additions & 2 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,7 @@
"@types/ws": "^4.0.1",
"@types/yargs": "^15.0.11",
"@types/yargs-parser": "^15.0.0",
"@typescript-eslint/parser": "4.16.1",
"@typescript-eslint/parser": "^4.21.0",
"@wardpeet/brfs": "2.1.0",
"angular": "^1.7.4",
"archiver": "^3.0.0",
Expand All @@ -134,7 +134,7 @@
"cross-env": "^7.0.2",
"csv-validator": "^0.0.3",
"devtools-protocol": "0.0.805376",
"eslint": "^4.19.1",
"eslint": "^7.23.0",
"eslint-config-google": "^0.9.1",
"eslint-plugin-local-rules": "0.1.0",
"exorcist": "^1.0.1",
Expand Down
Loading