deps: update eslint to latest #12333
Conversation
| @@ -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((() => { | |||
|
|
|||
There was a problem hiding this comment.
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.
| @@ -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); | |||
There was a problem hiding this comment.
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)
| typeof library.gzip === 'number' && | ||
| typeof library.description === 'string' && | ||
| typeof library.repository === 'string' && | ||
| typeof library.version === 'string' && |
There was a problem hiding this comment.
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.
| 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; | ||
| }); |
There was a problem hiding this comment.
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 👋
Gets rid of the
warning on every
yarn. It's been working correctly in spite of the peer dep requirement, but oureslint@^4.19.1is three years old, the upgrade was pretty painless, and the minor changes needed were reasonable.