Skip to content
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

Merged
merged 9 commits into from
Jun 25, 2019
Merged

Conversation

patrickhulce
Copy link
Collaborator

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 the third-party-web dataset. It only includes the origins we actually observed in HTTPArchive 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 😮
image

(and another 10 more got cutoff)

@@ -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:
Copy link
Collaborator

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)},
Copy link
Member

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/).',
Copy link
Member

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?

Copy link
Collaborator Author

@patrickhulce patrickhulce Jun 12, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

s/only/try to/ ?

Copy link
Member

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',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
columnMainThreadTime: 'Main Thread Time',
columnMainThreadTime: 'Main-Thread Time',

Copy link
Collaborator Author

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.

Copy link
Member

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.

🤷‍♀

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.

@brendankenny
Copy link
Member

also I wish bundlesize master queries were working. For the FYI: adds about 75kB (16kB after gzip) to the bundle

Copy link
Member

@brendankenny brendankenny left a 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 :)

lighthouse-core/audits/third-party-summary.js Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants