new_audit: add js-libraries audit, just listing detected js libs by paulirish · Pull Request #6081 · GoogleChrome/lighthouse

paulirish · 2018-09-20T19:26:16Z

output in the LHR looks like this:

Details

{
  "id": "js-libraries",
  "title": "JavaScript libraries",
  "description": "All detected front-end JavaScript libraries on the page",
  "score": 1,
  "scoreDisplayMode": "binary",
  "rawValue": true,
  "details": {
    "type": "table",
    "headings": [
      {
        "key": "name",
        "itemType": "text",
        "text": "Name"
      },
      {
        "key": "version",
        "itemType": "text",
        "text": "Version"
      }
    ],
    "items": [
      {
        "name": "jQuery",
        "version": "1.12.4",
        "npm": "jquery"
      },
      {
        "name": "jQuery UI",
        "version": "1.11.4",
        "npm": "jquery-ui"
      },
      {
        "name": "Modernizr",
        "version": "2.6.2",
        "npm": "modernizr"
      },
      {
        "name": "Underscore",
        "version": "1.8.3",
        "npm": "underscore"
      },
      {
        "name": "FlexSlider",
        "version": null,
        "npm": "flexslider"
      }
    ],
    "summary": {}
  }
}

paulirish · 2018-09-20T19:38:02Z

cc @rviscomi to review the data format

developit · 2018-09-20T19:58:30Z

Looks good! This should be relatively easy to parse from within BQ.

@rviscomi - any thoughts about whether this format makes it possible to flatten the list of frameworks down so it's more readily accessible (as columns)?

patrickhulce · 2018-09-20T20:09:03Z

we should also probably bump our version of the detector, last time I checked they were missing ~90% of react sites I tested and a few commits helped lower that significantly

wardpeet · 2018-09-20T20:15:00Z

this is awesome :)

rviscomi · 2018-09-20T20:15:15Z

any thoughts about whether this format makes it possible to flatten the list of frameworks down so it's more readily accessible (as columns)?

Yeah with a user-generated function we're able to massage this formatting however we need. This format LGTM.

paulirish · 2018-09-20T22:05:18Z

we should also probably bump our version of the detector, last time I checked they were missing ~90% of react sites I tested and a few commits helped lower that significantly

yup. already on it: johnmichel/Library-Detector-for-Chrome#118

brendankenny

LGTM! 📚📚📚📚📚📚

brendankenny · 2018-09-20T22:27:18Z

lighthouse-core/audits/dobetterweb/js-libraries.js

@@ -0,0 +1,52 @@
+/**
+ * @license Copyright 2017 Google Inc. All Rights Reserved.


brendankenny · 2018-09-20T22:29:07Z

lighthouse-core/test/audits/dobetterweb/js-libraries-test.js

@@ -0,0 +1,64 @@
+/**
+ * @license Copyright 2017 Google Inc. All Rights Reserved.


brendankenny · 2018-09-20T22:30:20Z

lighthouse-core/test/audits/dobetterweb/js-libraries-test.js

+    });
+    assert.equal(auditResult1.rawValue, true);
+
+    // duplicates. TODO: consider failing in this case


TODO: consider failing in this case

it's not that uncommon to have more than one jquery in one page. And if we merge network stuff from iframes, won't it be even more common?

brendankenny · 2018-09-20T22:30:49Z

typings/audit.d.ts

    export type DetailsItem = string | number | DetailsRendererNodeDetailsJSON |
      DetailsRendererLinkDetailsJSON | DetailsRendererCodeDetailJSON | undefined |
-      boolean | DetailsRendererUrlDetailsJSON;
+      boolean | DetailsRendererUrlDetailsJSON | null;


details make me sad

exterkamp

LGTM, but does this new Audit need i18n on it's strings?

lighthouse-core/audits/dobetterweb/js-libraries.js

exterkamp · 2018-09-21T00:33:09Z

❌ FAIL. LHR has changed. Run 'yarn update:sample-json' to rebaseline the golden LHR. error Command failed with exit code 1. info Visit https://yarnpkg.com/en/docs/cli/run for documentation about this command. The command "yarn diff:sample-json" exited with 1.

Need to update the sample-json with the new audit table?

paulirish added 2 commits September 20, 2018 12:19

new_audit: add js-libraries audit, just listing detected js libs

6ac61ba

icon can be duplicated. nvm about that

a918b6d

paulirish requested review from brendankenny and patrickhulce as code owners September 20, 2018 19:26

add test

7805f4b

developit approved these changes Sep 20, 2018

View reviewed changes

brendankenny approved these changes Sep 20, 2018

View reviewed changes

exterkamp reviewed Sep 21, 2018

View reviewed changes

lighthouse-core/audits/dobetterweb/js-libraries.js Show resolved Hide resolved

feedback

f9cc492

paulirish merged commit 417cdbe into master Sep 22, 2018

paulirish deleted the usedlibs branch September 22, 2018 00:46

		@@ -0,0 +1,52 @@
		/**
		* @license Copyright 2017 Google Inc. All Rights Reserved.

		@@ -0,0 +1,64 @@
		/**
		* @license Copyright 2017 Google Inc. All Rights Reserved.

Conversation

paulirish commented Sep 20, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

paulirish commented Sep 20, 2018

Uh oh!

developit commented Sep 20, 2018

Uh oh!

patrickhulce commented Sep 20, 2018

Uh oh!

wardpeet commented Sep 20, 2018

Uh oh!

rviscomi commented Sep 20, 2018

Uh oh!

paulirish commented Sep 20, 2018

Uh oh!

brendankenny left a comment

Choose a reason for hiding this comment

Uh oh!

brendankenny Sep 20, 2018

Choose a reason for hiding this comment

Uh oh!

brendankenny Sep 20, 2018

Choose a reason for hiding this comment

Uh oh!

brendankenny Sep 20, 2018

Choose a reason for hiding this comment

Uh oh!

brendankenny Sep 20, 2018

Choose a reason for hiding this comment

Uh oh!

exterkamp left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

exterkamp commented Sep 21, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

paulirish commented Sep 20, 2018 •

edited

Loading