core(metrics): support CLS for all frames#11713
Conversation
| */ | ||
| static async compute_(trace) { | ||
| const cumulativeShift = trace.traceEvents | ||
| .filter(e => e.name === 'LayoutShift' && e.args && e.args.data && e.args.data.score) |
There was a problem hiding this comment.
You can narrow the type by annotating the filter function to be a type guard. here's an example: https://github.com/GoogleChrome/lighthouse/pull/7817/files#diff-b66c6ac9b4b8c0726c736eb7479a9e4ff22c7b81bfa1b2b259c2cdfa220ea1f8R19
There was a problem hiding this comment.
oh nvm the type here is kinda complicated, a ts ignore on L19 is probably better
There was a problem hiding this comment.
if this weren't on trace events I would suggest the map first and then a filter which is much easier in jsdoc type land :)
on the functionality itself though, we should normally be excluding layout shifts with had_recent_input=true, I don't expect it to have much effect but for comparison to the existing definition we should match it for now. also see fun stuff like #10960 :)
There was a problem hiding this comment.
totes forgot about the recent input thing. that was a rough bug
| */ | ||
| static async compute_(trace) { | ||
| const cumulativeShift = trace.traceEvents | ||
| .filter(e => e.name === 'LayoutShift' && e.args && e.args.data && e.args.data.score) |
There was a problem hiding this comment.
if this weren't on trace events I would suggest the map first and then a filter which is much easier in jsdoc type land :)
on the functionality itself though, we should normally be excluding layout shifts with had_recent_input=true, I don't expect it to have much effect but for comparison to the existing definition we should match it for now. also see fun stuff like #10960 :)
|
Part 1a (step 2?) of the Framehole implementation plan for CLS.
The
cumulative_scorearg inLayoutShifttrace events is computed per-frame. We could find the latestLayoutShiftevent for each frame but it seemed simpler to compute our own cumulative score from every layout shift event.Leaving this as a draft for now, will change the merge into branch to
masteronce #11701 lands.