-
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
Move networkRecords to a computed artifact #2062
Changes from all commits
7b034ef
bd58729
a206d3f
d26b466
bb36757
2ab1285
7d6e5c4
0e66f17
06410ab
f853e50
e7d39a1
fa43ff9
a85c4ce
9506235
60e6223
fac3c3d
aaabf3f
8712532
9802ad0
633c84f
fc03fc1
fc2b9fd
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 |
---|---|---|
|
@@ -41,7 +41,7 @@ class OffscreenImages extends ByteEfficiencyAudit { | |
informative: true, | ||
helpText: 'Images that are not above the fold should be lazily loaded after the page is ' + | ||
'interactive. Consider using the [IntersectionObserver](https://developers.google.com/web/updates/2016/04/intersectionobserver) API.', | ||
requiredArtifacts: ['ImageUsage', 'ViewportDimensions', 'traces', 'networkRecords'] | ||
requiredArtifacts: ['ImageUsage', 'ViewportDimensions', 'traces', 'devtoolsLogs'] | ||
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. this (and the old 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. used by the shared audit they extend 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. oh, I see. |
||
}; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -46,7 +46,7 @@ class UsesOptimizedImages extends ByteEfficiencyAudit { | |
'The following images could have smaller file sizes when compressed with ' + | ||
'[WebP](https://developers.google.com/speed/webp/) or JPEG at 80 quality. ' + | ||
'[Learn more about image optimization](https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/image-optimization).', | ||
requiredArtifacts: ['OptimizedImages', 'networkRecords'] | ||
requiredArtifacts: ['OptimizedImages', 'devtoolsLogs'] | ||
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. also unused? 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. used by the shared audit they extend |
||
}; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -40,7 +40,7 @@ class ResponsesAreCompressed extends ByteEfficiencyAudit { | |
helpText: 'Text-based responses should be served with compression (gzip, deflate or brotli)' + | ||
' to minimize total network bytes.' + | ||
' [Learn more](https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/optimize-encoding-and-transfer).', | ||
requiredArtifacts: ['ResponseCompression', 'networkRecords'] | ||
requiredArtifacts: ['ResponseCompression', 'devtoolsLogs'] | ||
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. unused? 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. used by the shared audit they extend |
||
}; | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -44,7 +44,7 @@ class UsesResponsiveImages extends ByteEfficiencyAudit { | |
'Image sizes served should be based on the device display size to save network bytes. ' + | ||
'Learn more about [responsive images](https://developers.google.com/web/fundamentals/design-and-ui/media/images) ' + | ||
'and [client hints](https://developers.google.com/web/updates/2015/09/automating-resource-selection-with-client-hints).', | ||
requiredArtifacts: ['ImageUsage', 'ViewportDimensions', 'networkRecords'] | ||
requiredArtifacts: ['ImageUsage', 'ViewportDimensions', 'devtoolsLogs'] | ||
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. unused? 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. used by the shared audit they extend |
||
}; | ||
} | ||
|
||
|
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.
since computed artifacts can call each other now, do you want to make
requestNetworkThroughput
takedevtoolsLogs
directly now and have it create thenetworkRecords
internally?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.
i do, yes! totally makes sense. in fact, i mentioned this at the end of the issue description :) ..
i'd do it in this PR but it'd add a lot more to the diff for no immediate benefit. I am suggesting doing that cleanup in a followup.
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.
got it, didn't realize that's what that was referring to
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.
although, would it? It would remove a lot of the lines just due to nesting added to this PR. Is the hard part
network-throughput-test.js
?