refactor: makes non-finished network records available #2197
Conversation
Can you clarify "internally"? I don't see the significance |
| @@ -61,7 +61,7 @@ class TotalByteWeight extends ByteEfficiencyAudit { | |||
| let results = []; | |||
| networkRecords.forEach(record => { | |||
| // exclude data URIs since their size is reflected in other resources | |||
| if (record.scheme === 'data') return; | |||
| if (record.scheme === 'data' || !record.finished) return; | |||
| @@ -56,7 +56,7 @@ class LoadFastEnough4Pwa extends Audit { | |||
| // Ignore requests that don't have timing data or resources that have | |||
| // previously been requested and are coming from the cache. | |||
| const fromCache = record._fromDiskCache || record._fromMemoryCache; | |||
| if (!record._timing || fromCache) { | |||
| if (!record._timing || fromCache || !record.finished) { | |||
There was a problem hiding this comment.
update comment (and it seems like it may make more sense to put !record.finished first)
| @@ -73,7 +73,7 @@ class OptimizedImages extends Gatherer { | |||
| static filterImageRequests(pageUrl, networkRecords) { | |||
| const seenUrls = new Set(); | |||
| return networkRecords.reduce((prev, record) => { | |||
| if (seenUrls.has(record._url)) { | |||
| if (seenUrls.has(record._url) || !record.finished) { | |||
There was a problem hiding this comment.
nit: does it make more sense to reverse conditions?
|
travis is super slow today |
|
comments updated
I sort of disagree, the fact that this is the most obvious and requires the least thought makes it ok to put it in the afterthought position and devote the most attention/focus to the domain-specific exclusions which come first |
patrickhulce
changed the title
it was a minor point signifying that we still filter out unfinished records almost everywhere so it's mostly an internal change |
Well that's what I mean. Really they could be filtered out before entering into these loops (a la critical request chains) or removed with their own conditional check, but people around here don't seem to like multiple single-line conditionals in a row :) Anyway, it's good as is. |
patrickhulce
force-pushed
the
move_network_records
branch
from
f315009
to
3c65eaa
Compare
split from #2023 (comment)