core(tsc): tighten traceOfTab timing types #5887
Conversation
| @@ -91,6 +92,9 @@ class RenderBlockingResources extends Audit { | |||
| const metricComputationData = {trace, devtoolsLog, simulator, settings: metricSettings}; | |||
| // @ts-ignore - TODO(bckenny): allow optional `throttling` settings | |||
| const fcpSimulation = await artifacts.requestFirstContentfulPaint(metricComputationData); | |||
| if (!traceOfTab.timestamps.firstContentfulPaint) { | |||
There was a problem hiding this comment.
@patrickhulce this one I was least sure about because it could have been an extra guard vs needed for correct behavior.
This arguably "works" if fcpTsInMs ends up as NaN as no resources will be ignored. I could change it to make that explicit instead of throwing instead.
| @@ -90,6 +90,9 @@ class Interactive extends MetricArtifact { | |||
| * @return {{cpuQuietPeriod: TimePeriod, networkQuietPeriod: TimePeriod, cpuQuietPeriods: Array<TimePeriod>, networkQuietPeriods: Array<TimePeriod>}} | |||
| */ | |||
| static findOverlappingQuietPeriods(longTasks, networkRecords, traceOfTab) { | |||
| if (!traceOfTab.timestamps.firstContentfulPaint) { | |||
| throw new LHError(LHError.errors.NO_FCP); | |||
| } | |||
There was a problem hiding this comment.
this was already checked down in computeObservedMetric(), but this seemed better than passing in the known-defined value as another parameter?
| traceEnd: (timestamps.traceEnd - navigationStart.ts) / 1000, | ||
| load: getTiming(timestamps.load), | ||
| domContentLoaded: getTiming(timestamps.domContentLoaded), | ||
| }; |
There was a problem hiding this comment.
happy to code golf on timestamps/timings creation :)
brendankenny
force-pushed
the
traceoftab
branch
from
8297e8d to
d7d5b34
Compare
SG and @paulirish seemed 👍 Updated. |
typings/artifacts.d.ts
Outdated
| /** The trace event marking firstPaint, if it was found. */ | ||
| firstPaintEvt?: TraceEvent; | ||
| /** The trace event marking firstContentfulPaint, if it was found. */ | ||
| firstContentfulPaintEvt?: TraceEvent; |
There was a problem hiding this comment.
this one can be no ? :)
#5873 needed fixes to
traceOfTab'stimingsandtimestampscreation, which is a good opportunity to take care of long-standing TODOs to fix the types for those objects. Basically most of their properties may not have been found in the trace but we were pretending they were all there to make dealing with them simpler.We now throw on
NO_FCPin so many places that maybe we should just call it required in a trace and throw intraceOfTablike we do for nonavigationStart?