core(simulator): improved timing typedef #5347
Conversation
patrickhulce
force-pushed
the
simulator_timing_typdef
branch
from
7d0376f
to
45b92b5
Compare
| @@ -405,9 +431,20 @@ class Simulator { | |||
|
|
|||
| return { | |||
| timeInMs: totalElapsedTime, | |||
| nodeTimings: this._nodeTimings, | |||
| nodeTimings: this._computeFinalNodeTimings(), | |||
There was a problem hiding this comment.
since we're now emitting a diff shape at the end should we use diff property names to keep totally clear? nodeTimeSpans vs nodeTimings? eh
There was a problem hiding this comment.
I would prefer to rename the internal since that's the one that carries more than just timings data
There was a problem hiding this comment.
Which name would you prefer since you were the original ✍️ of nodeTiming ;) #3187 (comment)
|
wfm
|
| @@ -112,7 +112,6 @@ class UsesRelPreloadAudit extends Audit { | |||
| const originalNode = originalNodesByRecord.get(node.record); | |||
| const timingAfter = simulationAfterChanges.nodeTimings.get(node); | |||
| const timingBefore = simulationBeforeChanges.nodeTimings.get(originalNode); | |||
| // @ts-ignore TODO(phulce): fix timing typedef | |||
| const wastedMs = Math.round(timingBefore.endTime - timingAfter.endTime); | |||
There was a problem hiding this comment.
ha, looks like the @ts-ignore was also hiding the possibly undefined (to tsc) response from nodeTimings.get(). I'm good with casting or a runtime check. It would be nice to avoid @ts-ignore since it masks everything in the line, but it also wouldn't be the end of the world
There was a problem hiding this comment.
whoops yeah, added a check for it! 👍
cleans up a lot of the TODOs left based on the internal/external timing discrepancy