-
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(lantern): move lantern metrics to lib/lantern #15875
Conversation
0529ace
to
6df3f36
Compare
6df3f36
to
f1c36ac
Compare
f1c36ac
to
9836f5f
Compare
9836f5f
to
a033ae0
Compare
core/test/lib/lantern/metrics/__snapshots__/first-meaningful-paint-test.js.snap
Outdated
Show resolved
Hide resolved
* @param {{trace: LH.Trace, devtoolsLog: LH.DevtoolsLog, settings?: LH.Audit.Context['settings'], URL?: LH.Artifacts.URL}} opts | ||
*/ | ||
function getComputationDataFromFixture({trace, devtoolsLog, settings, URL}) { | ||
settings = settings || {}; |
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.
settings
would always be filled out in a real scenario right? Maybe the fallback should be a copy of the default settings?
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.
settings and devtoolslog and URL are not real input to the planned lantern library, but they are used here as that's what Lighthouse was using. this will be addressed in a followup pr (see above TODO)
speed index is only test that is passing a settings object
ref #15841
This moves the meat of all the computed lantern metrics to
lib/lantern/metrics
, leaving behind just a ComputedArtifact wrapper and a conversion from Lighthouse data to Lantern data.A change to note is that the Lantern metrics now do not directly call what they depend on, but instead are expected to be called with any dependent metric values they require. This change was made to remove caching of results (the purpose of Lighthouse's ComputedArtifact) from the library.