-
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: add largest contentful paint to lantern and default config #9905
Changes from all commits
ec75de3
183b080
03f8ab6
52a8d28
4e2e688
f1836a0
449c7c5
d821da7
9cc6b8c
211a90b
ee9f34e
24db2ed
308f0fe
33fecc9
2b87da0
c473bf5
26bb02a
b848a73
967d630
5b80688
9da66f9
49aa30a
a71ab04
7fd52f4
f351590
ff42528
e93e396
9103431
a3fade6
567efc2
459a1de
a0b31db
045b829
498df14
7bb9d86
de48435
eb3599e
6d12f11
fa7f089
42b7ad1
8b5cce0
ce0042b
bbc5030
9812701
e0a04b7
2d34e65
6a1fdb7
1579ccb
5fc5db0
98f3ea2
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,71 @@ | ||
/** | ||
* @license Copyright 2019 Google Inc. All Rights Reserved. | ||
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 | ||
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. | ||
*/ | ||
'use strict'; | ||
|
||
const Audit = require('../audit.js'); | ||
const i18n = require('../../lib/i18n/i18n.js'); | ||
const ComputedLcp = require('../../computed/metrics/largest-contentful-paint.js'); | ||
|
||
const UIStrings = { | ||
/** The name of the metric that marks the time at which the largest text or image is painted by the browser. Shown to users as the label for the numeric metric value. Ideally fits within a ~40 character limit. */ | ||
title: 'Largest Contentful Paint', | ||
/** Description of the Largest Contentful Paint (LCP) metric, which marks the time at which the largest text or image is painted by the browser. This is displayed within a tooltip when the user hovers on the metric name to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */ | ||
description: 'Largest Contentful Paint marks the time at which the largest text or image is ' + | ||
`painted. [Learn More](https://web.dev/largest-contentful-paint)`, // TODO: waiting on LH specific doc. | ||
}; | ||
|
||
const str_ = i18n.createMessageInstanceIdFn(__filename, UIStrings); | ||
|
||
class LargestContentfulPaint extends Audit { | ||
/** | ||
* @return {LH.Audit.Meta} | ||
*/ | ||
static get meta() { | ||
return { | ||
id: 'largest-contentful-paint', | ||
title: str_(UIStrings.title), | ||
description: str_(UIStrings.description), | ||
scoreDisplayMode: Audit.SCORING_MODES.NUMERIC, | ||
requiredArtifacts: ['traces', 'devtoolsLogs'], | ||
}; | ||
} | ||
|
||
/** | ||
* @return {LH.Audit.ScoreOptions} | ||
*/ | ||
static get defaultOptions() { | ||
return { | ||
// TODO: Reusing FCP's scoring curve. Set correctly once distribution of results is available. | ||
scorePODR: 2000, | ||
scoreMedian: 4000, | ||
}; | ||
} | ||
|
||
/** | ||
* @param {LH.Artifacts} artifacts | ||
* @param {LH.Audit.Context} context | ||
* @return {Promise<LH.Audit.Product>} | ||
*/ | ||
static async audit(artifacts, context) { | ||
const trace = artifacts.traces[Audit.DEFAULT_PASS]; | ||
const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS]; | ||
const metricComputationData = {trace, devtoolsLog, settings: context.settings}; | ||
const metricResult = await ComputedLcp.request(metricComputationData, context); | ||
|
||
return { | ||
score: Audit.computeLogNormalScore( | ||
metricResult.timing, | ||
context.options.scorePODR, | ||
context.options.scoreMedian | ||
), | ||
numericValue: metricResult.timing, | ||
displayValue: str_(i18n.UIStrings.seconds, {timeInMs: metricResult.timing}), | ||
}; | ||
} | ||
} | ||
|
||
module.exports = LargestContentfulPaint; | ||
module.exports.UIStrings = UIStrings; |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,81 @@ | ||
/** | ||
* @license Copyright 2019 Google Inc. All Rights Reserved. | ||
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 | ||
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. | ||
*/ | ||
'use strict'; | ||
|
||
const makeComputedArtifact = require('../computed-artifact.js'); | ||
const LanternMetric = require('./lantern-metric.js'); | ||
const LHError = require('../../lib/lh-error.js'); | ||
const LanternFirstContentfulPaint = require('./lantern-first-contentful-paint.js'); | ||
|
||
/** @typedef {import('../../lib/dependency-graph/base-node.js').Node} Node */ | ||
|
||
class LanternLargestContentfulPaint extends LanternMetric { | ||
/** | ||
* @return {LH.Gatherer.Simulation.MetricCoefficients} | ||
*/ | ||
static get COEFFICIENTS() { | ||
// TODO: Calibrate | ||
return { | ||
connorjclark marked this conversation as resolved.
Show resolved
Hide resolved
|
||
intercept: 0, | ||
optimistic: 0.5, | ||
pessimistic: 0.5, | ||
}; | ||
} | ||
|
||
/** | ||
* TODO: Validate. | ||
* @param {Node} dependencyGraph | ||
* @param {LH.Artifacts.TraceOfTab} traceOfTab | ||
* @return {Node} | ||
*/ | ||
static getOptimisticGraph(dependencyGraph, traceOfTab) { | ||
const lcp = traceOfTab.timestamps.largestContentfulPaint; | ||
if (!lcp) { | ||
throw new LHError(LHError.errors.NO_LCP); | ||
} | ||
|
||
return LanternFirstContentfulPaint.getFirstPaintBasedGraph( | ||
dependencyGraph, | ||
lcp, | ||
_ => true | ||
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. I'm still working on this. 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. Haven't made any progress on improving this. @patrickhulce any ideas? Or maybe the accuracy is good enough (would want to use the new data you're collecting from GCP before saying for sure). 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. yeah let's wait for the full dataset before we invest too much more time. @connorjclark I'll need to ping you separately for a more privileged key to run the actual collection since mine is capped at 200/day if you want to peek though :) |
||
); | ||
} | ||
|
||
/** | ||
* TODO: Validate. | ||
* @param {Node} dependencyGraph | ||
* @param {LH.Artifacts.TraceOfTab} traceOfTab | ||
* @return {Node} | ||
*/ | ||
static getPessimisticGraph(dependencyGraph, traceOfTab) { | ||
const lcp = traceOfTab.timestamps.largestContentfulPaint; | ||
if (!lcp) { | ||
throw new LHError(LHError.errors.NO_LCP); | ||
} | ||
|
||
return LanternFirstContentfulPaint.getFirstPaintBasedGraph( | ||
dependencyGraph, | ||
lcp, | ||
_ => true, | ||
// For pessimistic LCP we'll include *all* layout nodes | ||
node => node.didPerformLayout() | ||
); | ||
} | ||
|
||
/** | ||
* @param {LH.Artifacts.MetricComputationDataInput} data | ||
* @param {LH.Audit.Context} context | ||
* @return {Promise<LH.Artifacts.LanternMetric>} | ||
*/ | ||
static async compute_(data, context) { | ||
const fcpResult = await LanternFirstContentfulPaint.request(data, context); | ||
const metricResult = await this.computeMetricWithGraphs(data, context); | ||
metricResult.timing = Math.max(metricResult.timing, fcpResult.timing); | ||
return metricResult; | ||
} | ||
} | ||
|
||
module.exports = makeComputedArtifact(LanternLargestContentfulPaint); |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -5,26 +5,27 @@ | |
*/ | ||
'use strict'; | ||
|
||
const makeComputedArtifact = require('../computed-artifact.js'); | ||
const ComputedMetric = require('./metric.js'); | ||
const LHError = require('../../lib/lh-error.js'); | ||
|
||
/** | ||
* @fileoverview Computed Largest Contentful Paint (LCP), the paint time of the largest in-viewport contentful element | ||
* COMPAT: LCP's trace event was first introduced in m78. We can't surface an LCP for older Chrome versions | ||
* @see https://github.com/WICG/largest-contentful-paint | ||
* @see https://wicg.github.io/largest-contentful-paint/ | ||
* @see https://web.dev/largest-contentful-paint | ||
*/ | ||
|
||
const makeComputedArtifact = require('../computed-artifact.js'); | ||
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. just curious, any reason for moving these? 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. I think a file overview comment block should be the first thing in a file (except license / strict i guess) do you hate it 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. we're inconsistent about this already. it looks 50/50 in a brief survey. being inconsistent would suggest that we leave existing ones as they lie until we fix all of them at once. (i think that's what we've done before in similar situations... require('xx.js').. right?) that said i dont think its important this time. so lets ignore this one.. just this one time. :) |
||
const ComputedMetric = require('./metric.js'); | ||
const LHError = require('../../lib/lh-error.js'); | ||
const LanternLargestContentfulPaint = require('./lantern-largest-contentful-paint.js'); | ||
|
||
class LargestContentfulPaint extends ComputedMetric { | ||
/** | ||
* @param {LH.Artifacts.MetricComputationData} data | ||
* @param {LH.Audit.Context} context | ||
* @return {Promise<LH.Artifacts.LanternMetric>} | ||
*/ | ||
// eslint-disable-next-line no-unused-vars | ||
static computeSimulatedMetric(data, context) { | ||
throw new Error('Unimplemented'); | ||
return LanternLargestContentfulPaint.request(data, context); | ||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -178,6 +178,7 @@ const defaultConfig = { | |
'without-javascript', | ||
'metrics/first-contentful-paint', | ||
'metrics/first-meaningful-paint', | ||
'metrics/largest-contentful-paint', | ||
'load-fast-enough-for-pwa', | ||
'metrics/speed-index', | ||
'screenshot-thumbnails', | ||
|
@@ -383,6 +384,7 @@ const defaultConfig = { | |
auditRefs: [ | ||
{id: 'first-contentful-paint', weight: 3, group: 'metrics'}, | ||
{id: 'first-meaningful-paint', weight: 1, group: 'metrics'}, | ||
{id: 'largest-contentful-paint', weight: 0, group: 'metrics'}, | ||
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 do I recall correctly that we intend to give this one of the highest weights right out of the gate? 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. yup. i'll do that in a followup PR. :) |
||
{id: 'speed-index', weight: 4, group: 'metrics'}, | ||
{id: 'interactive', weight: 5, group: 'metrics'}, | ||
{id: 'first-cpu-idle', weight: 2, group: 'metrics'}, | ||
|
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.
@brendankenny is it your estimation that we won't have any data on this? can we get access to the november run that hopefully has our good chrome?