misc: use typed-query-selector for native querySelector #11990
Conversation
brendankenny
requested review from
connorjclark
and removed request for
adamraine
| .filter(/** @return {tag is HTMLLinkElement} */ tag => { | ||
| if (tag.tagName !== 'LINK') return false; | ||
|
|
||
| .filter(linkTag => { |
There was a problem hiding this comment.
removing the lines in this file is real nice
| @@ -184,7 +184,7 @@ class CategoryRenderer { | |||
| this.dom.find('.lh-category-header__description', tmpl).appendChild(descEl); | |||
| } | |||
|
|
|||
| return /** @type {Element} */ (tmpl.firstElementChild); | |||
| return tmpl; | |||
There was a problem hiding this comment.
not a querySelector simplification, but you can appendChild a DocumentFragment. Not sure why we bothered with passing back the first child here since this is equivalent.
| */ | ||
| _getNextSelectableNode(allNodes, startNode) { | ||
| const nodes = allNodes.filter((node) => { | ||
| const nodes = allNodes.filter(/** @return {node is HTMLElement} */ (node) => { |
There was a problem hiding this comment.
also not querySelector related, but this was already checking instanceof HTMLElement, so it seems good
| * @param {ParentNode} context | ||
| * @return {HTMLElement} | ||
| */ | ||
| function find(query, context) { |
There was a problem hiding this comment.
we apparently have four copies of find() :)
There was a problem hiding this comment.
soon to be 5 with #11832
I often copy Dom in personal projects because the interface is so nice.
| @@ -283,7 +282,7 @@ class LighthouseReportViewer { | |||
| return new Promise((resolve, reject) => { | |||
| const reader = new FileReader(); | |||
| reader.onload = function(e) { | |||
| const readerTarget = /** @type {?FileReader} */ (e.target); | |||
| const readerTarget = e.target; | |||
There was a problem hiding this comment.
want to just remove the variable?
There was a problem hiding this comment.
want to just remove the variable?
oh good call
| * @param {ParentNode} context | ||
| * @return {HTMLElement} | ||
| */ | ||
| function find(query, context) { |
There was a problem hiding this comment.
soon to be 5 with #11832
I often copy Dom in personal projects because the interface is so nice.
| @@ -170,7 +170,7 @@ class CategoryRenderer { | |||
| /** | |||
| * @param {LH.ReportResult.Category} category | |||
| * @param {Record<string, LH.Result.ReportGroup>} groupDefinitions | |||
| * @return {Element} | |||
| * @return {DocumentFragment} | |||
There was a problem hiding this comment.
Can this just remain Element? I don't see benefit in making the return type more specific.
There was a problem hiding this comment.
Can this just remain Element? I don't see benefit in making the return type more specific.
It would be a Node, not an Element, so it had to change either way
Follow up to #11526, expands the use of
typed-query-selectorto uses ofquerySelector/querySelectorAlloutside of uses ofdom.find/dom.findAll. Allows removal of a few more type casts.