CLSMetricWithAttribution appears to have the wrong types

[rosesyrett]

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:

1. Convert the metrics I want to collect to json schemas,
2. Use those json schemas in my backend as serialization,
3. Also use those json schemas to define events for my analytics pipeline.

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 for CLSMetricWithAttribution we can see that it is defined like so:

export interface CLSMetricWithAttribution extends CLSMetric {
    attribution: CLSAttribution;
}

export interface CLSMetric extends Metric {
    name: 'CLS';
    entries: LayoutShift[];
}

interface LayoutShift extends PerformanceEntry {
	value: number;
	sources: LayoutShiftAttribution[];
	hadRecentInput: boolean;
}

Ctrl+clicking on LayoutShift in my editor (I'm using VScode) leads me to this bit of code, that defines LayoutShift:

web-vitals/src/types.ts

Lines 76 to 80 in 9b93251

	interface LayoutShift extends PerformanceEntry {
	value: number;
	sources: LayoutShiftAttribution[];
	hadRecentInput: boolean;
	}

This is apparently a modification to the built-in layout-shift to be more accurate (as per the comment), however it does not include the lastInputTime field that I am seeing in my data when using onCLS(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 for lastInputTime 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 advance

Details 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:

export { CLSMetricWithAttribution } from 'web-vitals';

... with web-vitals installed. Then install ts-json-schema-generator with

npm install ts-json-schema-generator

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:

npx ts-json-schema-generator -f tsconfig.json --path 'types.ts' --type 'CLSMetricWithAttribution' > out.json

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:

import jsonschema
import json

with open('out.json', 'r') as f:
    schema = json.load(f)

data = """
# paste the data that I have above here, i.e. the actual CLS metric in the json
"""

jsonschema.validate(json.loads(data), schema)

You should see the following error output:

Traceback (most recent call last):
  File "<stdin>", line 1, in <module>
  File "/.../.venv/lib/python3.11/site-packages/jsonschema/validators.py", line 1332, in validate
    raise error
jsonschema.exceptions.ValidationError: Additional properties are not allowed ('lastInputTime' was unexpected)

Failed validating 'additionalProperties' in schema['properties']['entries']['items']:
    ...

[tunetheweb]

@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.

[brendankenny]

@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.

[rosesyrett]

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?