Consider reducing the weight of comment nodes and adjacent text nodes in excessive DOM nodes warning

[rictic]

Some HTML rendering systems use comments to separate static and dynamic parts of HTML. For example, in Lit, the template:

html`<p>Hello, ${this.name}!</p>`

Will render as the HTML:

<p>
  TextNode(Hello, )
  <!--?lit$382362303$-->
  TextNode(World)
  TextNode(!)
</p>

Or in the latest proposed API for the DOM Parts standard a template system would render the HTML:

<p>Hello <!--?child-node-part?-->World<!--?/child-node-part?-->!</p>

We could walk the DOM before rendering a template and remove the comment nodes and in some cases merge adjacent text nodes in order to get a better Lighthouse score, but in benchmarking we've found that comment nodes are very cheap, and that it's generally slower to do so.

While I do appreciate that each DOM node does have a memory cost, it seems like the predominant cost is in style calculation, layout, and paint, and on those metrics comments and adjacent text nodes are far cheaper than an element.

[adamraine]

The large DOM size audit does not contribute directly to your performance score. Style calculation, layout and paint tasks which block the main thread do contribute to the performance score via the TBT metric.

That being said this idea sounds fine to me. Our audits shouldn't implicitly encourage users to remove all their comment nodes if comment nodes have negligible performance impact.

[justinfagnani]

@adamraine it would be great to get comments out of the audit altogether, or find some empirical cost-based weighting (memory?). From everything we can see, comments are nearly free compared to elements, and only show (slightly) up in tree-walk scenarios like cloning and non-cached queries.

[adamraine]

@rictic @justinfagnani can you provide an example URL where there are a lot of comment nodes?

[connorjclark]

We already are not counting comments. We only consider elements.

lighthouse/core/gather/gatherers/dobetterweb/domstats.js

Line 46 in 100c54a

let child = element.firstElementChild;

So let's update the audit docs to make this more clear.

---

In this example:

<p>
  TextNode(Hello, )
  <!--?lit$382362303$-->
  TextNode(World)
  TextNode(!)
</p>

<p> contains no elements, only Text and Comment nodes. So we never recurse into it.