core(tsc): add type checking to dbw audits #5069
Conversation
| lib.detectedLib.url = lib.pkgLink; | ||
| lib.detectedLib.type = 'link'; | ||
| /** @type {Array<{highestSeverity: string, vulnCount: number, detectedLib: LH.Audit.DetailsRendererLinkDetailsJSON}>} */ | ||
| const vulnerableResults = []; |
There was a problem hiding this comment.
vulnerabilities? vulnerabilityResults?
There was a problem hiding this comment.
went with vulnerabilityResults :)
| })); | ||
| /** @type {Array<{node: LH.Audit.DetailsRendererNodeDetailsJSON}>} */ | ||
| const items = []; | ||
| passwordInputsWithPreventedPaste.forEach(input => { |
There was a problem hiding this comment.
😢 TS really can't deal with a map + assertion?
There was a problem hiding this comment.
it loses the type 'node' (it widens to type string) :( The heuristics are wonky on widening like that, but people do want very different things at times (even we would probably usually want string in this case if it weren't for wanting to tell the difference between details types).
It's basically this or put a cast in there in addition to the declared type above, I believe, so this seemed better.
| @@ -7,7 +7,7 @@ | |||
| declare global { | |||
| module LH.Audit { | |||
| export interface Context { | |||
| options: Object; // audit options | |||
| options: Record<string, any>; // audit options | |||
There was a problem hiding this comment.
what's the difference between Record<K, V> and Object<K, V>
There was a problem hiding this comment.
what's the difference between Record<K, V> and Object<K, V>
Object<K, V> only works in jsdoc. It's not parameterized in typescript code
There was a problem hiding this comment.
...we should probably come up with some sort of style rule for this vs {[p: string]: any} vs Object<> on the jsdoc side
typings/audit.d.ts
Outdated
| @@ -47,7 +47,7 @@ declare global { | |||
| debugString?: string; | |||
| } | |||
|
|
|||
| // TODO: placeholder typedefs until Details are typed | |||
| // TODO: placeholders typedefs until Details are typed | |||
| summary?: DetailsRendererDetailsSummary; | ||
| } | ||
|
|
||
| export type DetailsItem = string | number | DetailsRendererNodeDetailsJSON | |
There was a problem hiding this comment.
DetailsItemMemberChildInfo
There was a problem hiding this comment.
DetailsItemMemberChildInfo
...Summary
| @@ -3941,26 +3904,8 @@ | |||
| ], | |||
| "items": [ | |||
| { | |||
| "name": "jQuery", | |||
There was a problem hiding this comment.
lgtm on nuking this data. it's pretty low-value anyway
the only major cleanups are in the
no-vulnerable-libraries.js.js(lots of mutations) andevent-helpers.js(simplified so that the type checker could follow along).The golden lhr changes are eliminating the duplicate vulnerability listings (because the original library objects are no longer mutated) and taking out the
labelentry on the event listeners since it appears to only be set, not read, and it's just a duplicate of the line/col info anyways.