Open rosesyrett opened 6 days ago
@philipwalton would know more, but I think we only added the attributes we needed by the library rather than looking for this to be comprehensive definition of the type.
lastInputTime
isn't used by the library hence it was not needed to add this.
Ideally all these types would be added to the standard lib.dom.ts so we didn't even need to define the fields we used, but that tends to go slower, or wait for them to leave experimental status for that to happen.
So all in all I don't think you can depend on this libraries DOM types being complete.
@philipwalton would know more, but I think we only added the attributes we needed by the library rather than looking for this to be comprehensive definition of the type.
lastInputTime
isn't used by the library hence it was not needed to add this.
This is exactly right. @rosesyrett if you want to open a PR to add lastInputTime
to the LayoutShift
type, we'd be happy to accept!
Ideally all these types would be added to the standard lib.dom.ts so we didn't even need to define the fields we used, but that tends to go slower, or wait for them to leave experimental status for that to happen.
Exactly, the tsc policy is to add to the DOM types when implemented in two different browser engines (usually happens automatically when that criteria is met according to https://github.com/mdn/browser-compat-data), so it won't happen for this until Firefox or Safari add CLS.
Thanks for the prompt responses - that makes sense. I guess for now I could just update the "additionalProperties" field to be "true" in my generated schemas where I notice there are validation issues like this one, and have some tests that run which will fail if this ever changes in future. Because I'm interested in collecting all the attribution metrics your repo offers, I'm going to write some tests on my end and check for which cases these validation checks fail. I might then open a PR to address these e.g. to add fields in, that would be helpful for us in future!
I recognise this is still potentially a bit fragile; e.g. if the DOM types ever actually changed then the ones here would be inaccurate. But I suppose that's a risk you face even now (e.g. with existing fields). Is there a PR/issue somewhere I could track for something like adding these to the standard lib.dom.ts?
Description
Hello,
I am trying to integrate this npm package into my react (ts)/django website to track slow webpages. So far the documentation has been really clear and easy to follow, so thanks for everyone who contributed to that!
For my use case, it is helpful to:
I have worked out how to use another npm package,
ts-json-schema-generator
, to generate json schemas from e.g.CLSMetricWithAttribution
. So, I decided to start with this as a test case and see what would happen if, upon generating this schema, I tried to validate it with a metric produced by this npm package. I expected, of course, that the metric would be validated just fine, and so I could incorporate this into my backend serialization. However, I came across a hiccup. My metric looked something like this (click to expand):CLS metric
{ "name": "CLS", "value": 1.0004260050639588, "rating": "poor", "delta": 1.0004260050639588, "entries": [ { "name": "", "entryType": "layout-shift", "startTime": 1824, "duration": 0, "value": 1, "hadRecentInput": false, "lastInputTime": 0, "sources": [ { "previousRect": { "x": 8, "y": 8, "width": 1551, "height": 1320, "top": 8, "right": 1559, "bottom": 1328, "left": 8 }, "currentRect": { "x": 0, "y": 0, "width": 1559, "height": 1328, "top": 0, "right": 1559, "bottom": 1328, "left": 0 } } ] } ], "id": "...", "navigationType": "reload", "attribution": { "largestShiftTarget": "...", "largestShiftTime": 1824, "largestShiftValue": 1, "largestShiftSource": { "previousRect": { "x": 8, "y": 8, "width": 1551, "height": 1320, "top": 8, "right": 1559, "bottom": 1328, "left": 8 }, "currentRect": { "x": 0, "y": 0, "width": 1559, "height": 1328, "top": 0, "right": 1559, "bottom": 1328, "left": 0 } }, "largestShiftEntry": { "name": "", "entryType": "layout-shift", "startTime": 1824, "duration": 0, "value": 1, "hadRecentInput": false, "lastInputTime": 0, "sources": [ { "previousRect": { "x": 8, "y": 8, "width": 1551, "height": 1320, "top": 8, "right": 1559, "bottom": 1328, "left": 8 }, "currentRect": { "x": 0, "y": 0, "width": 1559, "height": 1328, "top": 0, "right": 1559, "bottom": 1328, "left": 0 } } ] }, "loadState": "dom-content-loaded" } }
Importantly, note the presence of
lastInputTime
in the "entries" field. Looking through the type definitions forCLSMetricWithAttribution
we can see that it is defined like so:Ctrl+clicking on
LayoutShift
in my editor (I'm using VScode) leads me to this bit of code, that definesLayoutShift
: https://github.com/GoogleChrome/web-vitals/blob/9b932519b16f72328c6d8e9814b811f1bc1a0bb5/src/types.ts#L76-L80 This is apparently a modification to the built-in layout-shift to be more accurate (as per the comment), however it does not include thelastInputTime
field that I am seeing in my data when usingonCLS(console.log)
locally in my website browser.So my question is, why is there this difference between
LayoutShift
definitions? My best guess is that, looking at the docs forlastInputTime
it seems this is an experimental property which was recently added. In this case, does this library need to constantly update the typings whenever the MDN interfaces change as a result? Or am I missing something here? Thanks in advanceDetails on how to reproduce
There's an easy way to reproduce the issue I'm having. First, in any existing node workspace, just include some
types.ts
file or similar that looks like:... with web-vitals installed. Then install ts-json-schema-generator with
You will need to be in a node workspace with a
tsconfig.json
for this to work. Ensure "esModuleInterop" is set to "true" in the "compilerOptions". Then, simply:this will generate the schema in the
out.json
(it is quite big! There are ways to reduce this.. but I've been experimenting with the full version to try and get validation to work, see below)Then, you can copy directly my example from above and in a python shell, do the following:
You should see the following error output: