-
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
new_audit: add third party entity summary #9067
Conversation
@@ -8063,6 +8063,11 @@ text-table@~0.2.0: | |||
resolved "https://registry.yarnpkg.com/text-table/-/text-table-0.2.0.tgz#7f5ee823ae805207c00af2df4a84ec3fcfa570b4" | |||
integrity sha1-f17oI66AUgfACvLfSoTsP8+lcLQ= | |||
|
|||
third-party-web@^0.8.2: |
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.
<3
|
||
/** @type {LH.Audit.Details.Table['headings']} */ | ||
const headings = [ | ||
{key: 'entity', itemType: 'link', text: str_(i18n.UIStrings.columnURL)}, |
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.
"URL" doesnt make sense as the column header.. perhaps "Third party" ?
/** Description of a Lighthouse audit that identifies the code on the page that the user doesn't control. This is displayed after a user expands the section to see more. No character length limits. 'Learn More' becomes link text to additional documentation. */ | ||
description: 'Third-party code can significantly impact load performance. ' + | ||
'Limit the number of redundant third-party providers and only load third-party code after ' + | ||
'your page has primarily finished loading. [Learn more](https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/loading-third-party-javascript/).', |
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.
only load third-party code after your page has primarily finished loading
seems kind of strongly worded?
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.
s/only/try 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.
s/only/try to/ ?
sg
'Limit the number of redundant third-party providers and only load third-party code after ' + | ||
'your page has primarily finished loading. [Learn more](https://developers.google.com/web/fundamentals/performance/optimizing-content-efficiency/loading-third-party-javascript/).', | ||
/** Label for a table column that displays how much time each row spent executing on the main thread, entries will be the number of milliseconds spent. */ | ||
columnMainThreadTime: 'Main Thread Time', |
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.
columnMainThreadTime: 'Main Thread Time', | |
columnMainThreadTime: 'Main-Thread Time', |
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.
source? it's mixed at best in our repo too but in https://github.com/WICG/main-thread-scheduling the only thing that has Main-thread
is the kebabcase slug and one of the headers while main thread
appears 7 times.
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.
source? it's mixed at best in our repo too but in https://github.com/WICG/main-thread-scheduling the only thing that has Main-thread is the kebabcase slug and one of the headers while main thread appears 7 times.
I mean, I didn't think about it very hard, but it's a phrasal adjective so generally the rule is hyphenate. If it's a known phrase then that's also fine.
🤷♀
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.
@patrickhulce The hyphen is needed to disambiguate that the word main is part of main thread and not a general adjective that describes thread time. For comparison, in main soccer game, main describes soccer game.
also I wish bundlesize master queries were working. For the FYI: adds about 75kB (16kB after gzip) to the bundle |
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.
sorry if my comments made this look like it was blocked on me. It LedGTM and still does :)
Summary
Shines the light on third party impact!! Starting small with just a diagnostic audit that says "here are the ones you're using." I'd love to get more aggressive and in the users' face about alternatives/rankings/stats etc at some point, but this seems pretty decent start.
I'm using the
httparchive-no-stats
subset of thethird-party-web
dataset. It only includes the origins we actually observed inHTTPArchive
and deletes the stats to cut down on bundle size impact (weighs in at 65KB compared to 202KB).This is what it looks like for The Verge 😮
(and another 10 more got cutoff)