Skip to content
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

Missing types for category.supportedModes and node.explanation #15799

Closed
2 tasks done
hilja opened this issue Feb 2, 2024 · 9 comments · Fixed by #16006
Closed
2 tasks done

Missing types for category.supportedModes and node.explanation #15799

hilja opened this issue Feb 2, 2024 · 9 comments · Fixed by #16006

Comments

@hilja
Copy link

hilja commented Feb 2, 2024

FAQ

URL

https://clubmate.fi/about

What happened?

Not a big deal, but type defs for category.supportedModes and node.explanation fields seem to be missing. Also for www.theverge.com the node.explanation is not there (under the label-content-name-mismatch item). Apparently these are more rare fields.

Is this a mistake in the types or the output?

What did you expect?

Complete typing.

What have you tried?

I saved the output to a .ts file and added a type, then looked for the red squiglies:

import type Result from 'lighthouse/types/lhr/lhr.js'

const foo: Result = { /* LH output */ }
Screenshot 2024-02-02 at 19 21 55 Screenshot 2024-02-02 at 20 02 14

How were you running Lighthouse?

node

Lighthouse Version

11.5.0

Chrome Version

No response

Node Version

v20.11.0

OS

Mac OS

Relevant log output

No response

@adamraine
Copy link
Member

The Lighthouse Config type allows supportedModes on each category but the Result type does not. TS is accurately reporting that the field is incorrect.

The explanation seems like an actual issues since we set it but it's not on the types:
https://github.com/GoogleChrome/lighthouse/blob/main/core/audits/accessibility/axe-audit.js#L79

@hilja
Copy link
Author

hilja commented Feb 3, 2024

Okay interesting. Could you elaborate on the Config a bit? Or point me to docs. I don't fully understand how to use that, or why am I seeing supportedModes in the results?

If I use Config.Category:

type CategoryConfig = Record<string, Config.Category>

Then it’s missing other properties like id and score.

I can extend the types and it seems to work:

type CategoryConfig = Record<string, Config.Category & Result.Category>

Is this correct usage?

Thanks!

@adamraine
Copy link
Member

Okay interesting. Could you elaborate on the Config a bit? Or point me to docs. I don't fully understand how to use that, or why am I seeing supportedModes in the results?

Yeah I took a look at the generated results, and looks like we are emitting supportedModes on the result category even though it's not a part of the type. So I guess we should either remove that from the output or add supportedModes to the types.

@An-Yay
Copy link

An-Yay commented Feb 26, 2024

I'd like to have a take on this issue. I believe adding supportedModes would work better

@adamraine adamraine assigned An-Yay and unassigned brendankenny Mar 5, 2024
@adamraine
Copy link
Member

Feel free to submit a PR @An-Yay

@hilja
Copy link
Author

hilja commented Mar 6, 2024

Please do it :) I just patched Lighthouse and forgot about it.

@parshva-b
Copy link

Hey @adamraine,

I can pick the issue if it is still pending!

@adamraine
Copy link
Member

Feel free to submit a PR @parshva-b

@angad-sethi
Copy link
Contributor

G'day folks, I've raised a PR here for this fix. Please let me know if I've got it wrong. Cheers!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

6 participants