-
Notifications
You must be signed in to change notification settings - Fork 9.4k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
core(cumulative-layout-shift): experiment with new shared trace engine #15702
Changes from 3 commits
96e1a0e
6a4f4ba
30b7e6b
6b3dd22
d3d9f48
3f369c4
ab8f0b9
69c03c4
4c19cd5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,111 @@ | ||
// @ts-nocheck | ||
/* eslint-disable */ | ||
|
||
function polyfillDOMRect() { | ||
// devtools assumes clientside :( | ||
|
||
// Everything else in here is the DOMRect polyfill | ||
// https://raw.githubusercontent.com/JakeChampion/polyfill-library/master/polyfills/DOMRect/polyfill.js | ||
|
||
(function (global) { | ||
function number(v) { | ||
return v === undefined ? 0 : Number(v); | ||
} | ||
|
||
function different(u, v) { | ||
return u !== v && !(isNaN(u) && isNaN(v)); | ||
} | ||
|
||
function DOMRect(xArg, yArg, wArg, hArg) { | ||
let x, y, width, height, left, right, top, bottom; | ||
|
||
x = number(xArg); | ||
y = number(yArg); | ||
width = number(wArg); | ||
height = number(hArg); | ||
|
||
Object.defineProperties(this, { | ||
x: { | ||
get: function () { return x; }, | ||
set: function (newX) { | ||
if (different(x, newX)) { | ||
x = newX; | ||
left = right = undefined; | ||
} | ||
}, | ||
enumerable: true | ||
}, | ||
y: { | ||
get: function () { return y; }, | ||
set: function (newY) { | ||
if (different(y, newY)) { | ||
y = newY; | ||
top = bottom = undefined; | ||
} | ||
}, | ||
enumerable: true | ||
}, | ||
width: { | ||
get: function () { return width; }, | ||
set: function (newWidth) { | ||
if (different(width, newWidth)) { | ||
width = newWidth; | ||
left = right = undefined; | ||
} | ||
}, | ||
enumerable: true | ||
}, | ||
height: { | ||
get: function () { return height; }, | ||
set: function (newHeight) { | ||
if (different(height, newHeight)) { | ||
height = newHeight; | ||
top = bottom = undefined; | ||
} | ||
}, | ||
enumerable: true | ||
}, | ||
left: { | ||
get: function () { | ||
if (left === undefined) { | ||
left = x + Math.min(0, width); | ||
} | ||
return left; | ||
}, | ||
enumerable: true | ||
}, | ||
right: { | ||
get: function () { | ||
if (right === undefined) { | ||
right = x + Math.max(0, width); | ||
} | ||
return right; | ||
}, | ||
enumerable: true | ||
}, | ||
top: { | ||
get: function () { | ||
if (top === undefined) { | ||
top = y + Math.min(0, height); | ||
} | ||
return top; | ||
}, | ||
enumerable: true | ||
}, | ||
bottom: { | ||
get: function () { | ||
if (bottom === undefined) { | ||
bottom = y + Math.max(0, height); | ||
} | ||
return bottom; | ||
}, | ||
enumerable: true | ||
} | ||
}); | ||
} | ||
|
||
globalThis.DOMRect = DOMRect; | ||
})(globalThis); | ||
} | ||
|
||
export {polyfillDOMRect}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,16 @@ | ||
// @ts-expect-error missing types | ||
import * as TraceEngine from '@paulirish/trace_engine'; | ||
|
||
import {polyfillDOMRect} from './polyfill-dom-rect.js'; | ||
|
||
polyfillDOMRect(); | ||
|
||
const TraceProcessor = TraceEngine.Processor.TraceProcessor; | ||
const TraceHandlers = TraceEngine.Handlers.ModelHandlers; | ||
const RootCauses = TraceEngine.RootCauses.RootCauses.RootCauses; | ||
|
||
export { | ||
TraceProcessor, | ||
TraceHandlers, | ||
RootCauses, | ||
}; |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -180,6 +180,7 @@ | |
"webtreemap-cdt": "^3.2.1" | ||
}, | ||
"dependencies": { | ||
"@paulirish/trace_engine": "^0.0.7", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Doesn't really matter regardless so just out of curiosity, what's the bundle size impact? 😬 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. @paulirish can this be revived? https://lh-build-tracker.herokuapp.com/ the devtools bundle: 1948329 -> 2069598 (+118KB), pre-compression |
||
"@sentry/node": "^6.17.4", | ||
"axe-core": "^4.8.1", | ||
"chrome-launcher": "^1.1.0", | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I always get mixed up on this so just correct me if I'm wrong, but isn't it less lenient?
Some events may have
had_recent_input: true
so wouldn't be counted by Chrome/DT toward the CLS total, but Lighthouse includes those events (potentially making CLS worse) because it wasn't really input, it was the emulation viewport change?edit: I guess unless it's saying it's more lenient from the perspective of the events getting filtered out, haha
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
yeah, the filter itself is more lenient in what it allows through. It's a lot of summarize so I just have the comment defer to "the comment in
getLayoutShiftEvents
"