report: correctly display CLS in budget table

[Beytoven]

Fixes #11177

[connorjclark]

Nice! So to be clear, the issue was that the granularity was hiding the over-budget value for CLS?

[Beytoven]

Nice! So to be clear, the issue was that the granularity was hiding the over-budget value for CLS?

That's part of it. The bigger issue was that the column was defined as 'ms' in the header and so any value in the column gets treated as such. This includes the number being rounded to the nearest 10 by default.

[brendankenny]

There's a simpler way of doing this without special-casing budgets, and I apologize it's not documented better. I think this audit is meant to use it for the SpeedIndex budget based on this comment, but it doesn't appear to actually be doing it.

Table headings set the rendered types for the items in their column (e.g. measurement and overBudget set itemType: ms here), but each item can override that rendered type individually by using the object form of an item value instead of the primitive form. An example of this is renderCode(), which can be invoked for a table value:

• with just plain text if the column is set to itemType: code or
• by an item that's an object with type: code, so the heading is ignored and the object is rendered as a code detail.

So basically the CLS item needs to be something like {type: 'numeric', value, granularity: 0.01}. Unfortunately, this object form of numeric isn't something detailsRenderer knows how to render yet, so you'll need to

• add a NumericValue type in audit-details.d.ts (alongside the similar CodeValue, LinkValue, etc)
• add the this._renderNumeric() call to _renderTableValue() here

[connorjclark]

Won't this be a breaking change?

[brendankenny]

It'll be a new type, so it'll fall back to the less good unknown details type in old renderers, but it's a new feature, not a breaking change. It would be the same for a BudgetValue, too.

[connorjclark]

I meant for either the current change or just doing what you suggested: programmatic consumers of budgets likely aren't expecting overBudget to be an object. Does this meet our threshold of a breaking change?

[patrickhulce]

Does this meet our threshold of a breaking change?

Given that it's not adding a new details renderer type, I would say probably not requiring a major version rev, but it definitely could break some integrations (the way Lighthouse CI processes resource budgets would break if we did this there) so worth giving a heads up to our big programmatic consumers. If it were close enough to 7.0 I would probably say it's worth waiting :)

I think we once threw around the idea of creating a locked issue that interested consumers could subscribe to for notable changes like this?

[brendankenny]

Given that it's not adding a new details renderer type, I would say probably not requiring a major version rev, but it definitely could break some integrations

We say details aren't generally part of our breaking changes, but there probably are exceptions for specific audits (budgets, metrics, etc) that people might reasonably rely on raw access staying stable.

OTOH, this is arguably a bug fix in line with the published details types, and people relying on the buggy behavior will have to adapt :)

Waiting if we think it's going to break someone or just relying on a strong notification of the fix both SGTM.

[connorjclark]

<3

[connorjclark]

It's unfortunate how complex this has made this fn + the sort at the end.

wb ~L167, where tableItems is called, we filter the results and replace the CLS measurement value instead ?