updates for using lighthouse v3 #178

denar90 · 2018-05-28T17:55:52Z

Core of changes to use LH v3

updated perf config
cleaned up metrics adapter
cleaned types

Has to be merged after final release and test of v3 version.

denar90 · 2018-05-28T17:56:43Z

lib/index.ts

 const parseChromeFlags = require('lighthouse/lighthouse-cli/run').parseChromeFlags;
-const perfConfig: any = require('./lh-config');
+const perfConfig: any = require('lighthouse/lighthouse-core/config/plots-config.js');
+perfConfig.extends = 'lighthouse:default';


note: already fixed in LH master branch and has to be removed

denar90 · 2018-05-30T19:09:55Z

it will be nice if someone takes a look at it before further work on it...
thanks)

paulirish

in 3.0 you have the choice of using lantern's simulated throttling or keep using the devtools throttling. I would recommend using lantern (on by default via the default lighthouse config), but maybe a user should be able to change this.

paulirish · 2018-05-30T23:23:11Z

lib/index.ts

 const parseChromeFlags = require('lighthouse/lighthouse-cli/run').parseChromeFlags;
-const perfConfig: any = require('./lh-config');
+const perfConfig: any = require('lighthouse/lighthouse-core/config/plots-config.js');


I think you should just provide a config file here in this repo. We don't have much reason to maintain that plots-config fwiw. :)

metrics audit

while everything you need is in the new audit called metrics it doesnt come with human-readable names. Since you provide those in messages.ts maybe its fine and all you really need is metrics. I think this could work.

individual audits

If you really need all the individual audits:

module.exports = { extends: 'lighthouse:default', settings: { onlyAudits: defaultConfig.categories.performance.auditRefs .filter(a => a.group && a.group === 'metrics') .map(a => a.id) }, };

Paul, thanks. Would you like to keep all metrics we already have or reduce their number? I'm kinda confused because I faced that people just ignore FCP and rely only on FMP in perf aduits/case studies etc. As result with 3.0 and plots-config I thought maybe trends were changed...

The way we look at the problem now is:

key metrics

FCP

Speed Index

TTI

worthwhile diagnostic metrics

FMP

FCPUIdle

EIL is interesting but could use some work. it'll also change substantially in the future. And it's not on the same timescale as the others.

FirstVC should mostly be duplicated by FCP.

VizComplete is cool unless it marked the wrong thing, in which case it and Speed Index are totally useless. :)

@paulirish but VizComplete is not part of metrics.

auditRefs: [ {id: 'first-contentful-paint', weight: 3, group: 'metrics'}, {id: 'first-meaningful-paint', weight: 1, group: 'metrics'}, {id: 'speed-index', weight: 4, group: 'metrics'}, {id: 'interactive', weight: 5, group: 'metrics'}, {id: 'first-cpu-idle', weight: 2, group: 'metrics'}, {id: 'estimated-input-latency', weight: 0, group: 'metrics'},

Should it be added there?
Or, should we use pwmetrics-events and get it manually?

paulirish · 2018-05-30T23:34:03Z

lib/metrics.ts

    lighthouseVersion: res.lighthouseVersion,
-    initialUrl: res.initialUrl,
-    url: res.url
+    initialUrl: res.finalUrl,


initial is final?

i'd recommend adopting requestedUrl and finalUrl names for this metrics export.

paulirish · 2018-05-30T23:35:43Z

lib/utils/messages.ts

+    case 'estimated-input-latency':
+      return 'Estimated Input Latency';
+    case 'first-cpu-idle':
+      return 'Time to First Interactive';


the metric is renamed so it'd be best to call it "First CPU Idle" now

paulirish · 2018-05-30T23:38:31Z

lib/utils/messages.ts

@@ -44,24 +44,16 @@ const getMessage = function (messageType: string, ...args: any[]) {
      return `No matching message prefix: ${args[0]}`;
    case 'METRIC_IS_UNAVAILABLE':
      return `Sorry, ${args[0]} metric is unavailable`;
-    case 'ttfcp':
+    case 'first-meaningful-paint':


seems wrong.

in 3.0 FCP is both in 'metrics' audit and its own. i would def recommend showing both FCP and FMP in pwmetrics. (plots doesnt have FCP)

denar90 · 2018-06-08T21:00:23Z

at list tests passed ;)

denar90 · 2018-06-08T21:05:45Z

lib/metrics/metrics.ts

+// Copyright 2018 Google Inc. All Rights Reserved.
+// Licensed under the Apache License, Version 2.0. See LICENSE
+
+export default {


I see this one useful for people using expectations and stuff. What they need is just import our constants and use them for expectations config.

const METRICS = require('pwmetrics/lib/metrics-adapter/metrics'); module.exports = { url: 'http://my-awesome-site.com' flags: { expectations: true }, expectations: { [METRICS. TTFCP]: { warn: '>=1000', error: '>=2000' } } }

Other then that cleaner for our code to have map metrics ids.

denar90 · 2018-07-02T20:15:26Z

@paulirish we are good to go after solving #178 (comment)
Then I'll do manual regression and update docs.

Since LH 3.0 is done, It will be nice to announce pwmetrics updates right after LH announcement.

denar90 · 2018-08-04T20:00:53Z

@paulirish can you take a look here 👀?

denar90 · 2018-08-11T22:07:21Z

Opened questions are:

handle vizcomplete
gulp example still failes on CI :( (we can live with that for now)
code review

paulirish · 2018-08-15T00:36:24Z

package.json

    "chrome-launcher": "^0.10.2",
    "google-auth-library": "^0.10.0",
    "googleapis": "^16.0.0",
-    "lighthouse": "^2.9.4",
+    "lighthouse": "3.0.0",


you can bump it to 3.0.3 now

paulirish · 2018-08-15T00:36:39Z

package.json

    "mocha": "^3.2.0",
    "sinon": "^1.17.7",
    "sinon-chai": "^2.8.0",
-    "typescript": "^2.0.3"
+    "typescript": "2.9.1-insiders.20180516",


this can go to 3.0.0 now too :)

paulirish · 2018-08-15T00:38:01Z

package.json

    "mocha": "^3.2.0",
    "sinon": "^1.17.7",
    "sinon-chai": "^2.8.0",
-    "typescript": "^2.0.3"
+    "typescript": "2.9.1-insiders.20180516",
+    "vscode-chrome-debug-core": "3.23.8"


not sure why this is added. was it types?

I think it's fixed in LH now, so can be dropped 👍

hmm, still needed because without that tsc build failed

huh. weird. we're about to remove that dependency: GoogleChrome/lighthouse#5836

probably we will wait for PR to be merged...

dropped 💥
just waiting for next LH release

paulirish · 2018-08-15T00:38:58Z

.travis.yml

-  - npm install -g --no-progress yarn@v0.22.0
+    - lighthouse-extension/node_modules
+    - lighthouse-viewer/node_modules
+    - /home/travis/.rvm/gems/


the two lighthouse-* folders dont make sense to me here.
and i dont think you're running ruby, so you can nuke this one too.

paulirish · 2018-08-15T00:41:20Z

lib/index.ts

@@ -107,15 +98,15 @@ class PWMetrics {
    }

    if (resultHasExpectationErrors && this.flags.expectations) {
-      throw new Error(messages.getMessage('HAS_EXPECTATION_ERRORS'));
+      console.log(getMessage('HAS_EXPECTATION_ERRORS'));


you want to let things continue in this case? im fine with that, just pointing out that the behavior is changing. :)
probably should console.error() if its staying non-throwing.

paulirish · 2018-08-15T00:53:02Z

recipes/gulp/gulpfile.js

@@ -57,22 +39,19 @@ const runPwmetrics = function() {
 * @param {Object} results - Pwmetrics results obtained through Lighthouse
 */
 const handleOk = function(results) {
-  stopServer();
-  console.log(results);
+  console.log(JSON.stringify(results, null, 4));


4 -> 2. 😝

paulirish · 2018-08-15T00:54:46Z

types/types.ts

+  4: number; // PSI timing
+  5: number; // VISUALLY_COMPLETE timing
+  6: number; // TTFI timing
+  7: number; // TTCI timing


since these are changing position, i guess these sheets will be a bit broken now.

nice catch 👍

paulirish · 2018-08-15T00:57:25Z

lib/metrics/metrics.ts

+  TTFCP: 'first-contentful-paint',
+  TTFMP: 'first-meaningful-paint',
+  TTF_CPU_IDLE: 'first-cpu-idle',
+  TTCI: 'interactive',


paulirish · 2018-08-15T00:57:53Z

lib/metrics/metrics.ts

+export default {
+  TTFCP: 'first-contentful-paint',
+  TTFMP: 'first-meaningful-paint',
+  TTF_CPU_IDLE: 'first-cpu-idle',


TTFCPUIDLE ? :)

paulirish · 2018-08-15T00:58:22Z

lib/metrics/metrics.ts

+  TTFMP: 'first-meaningful-paint',
+  TTF_CPU_IDLE: 'first-cpu-idle',
+  TTCI: 'interactive',
+  PSI: 'speed-index',


PSI -> SI everywhere. we ditched perceptual speed index and now just calculate speed index.

denar90 · 2018-08-27T22:43:53Z

Finally CI passes 🎉
So waiting for LH to be updated with removing vscode-chrome-debug-core.
Other than that if we are ok to go without vizComplete metric I'll do final manual check and ready to 🚢 P.S. I still don't know what version it will be. Looks like v4...

denar90 · 2018-09-07T20:40:38Z

@paulirish we are ready, I think)

denar90 · 2018-09-12T08:53:10Z

I'm going to merge it at the and of the day today. In case of issues/comments, we can fix them in separate PR.

paulirish · 2018-09-12T08:56:05Z

Sounds good On Sep 12, 2018 1:53 AM, "Artem Denysov" <notifications@github.com> wrote: I'm going to merge it at the and of the day today. In case of issues/comments, we can fix them in separate PR. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#178 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AACZF769dOQLQ79EonDF3VpIrX_2yFHiks5uaMt4gaJpZM4UQeKE> .

denar90 · 2018-09-12T19:39:34Z

Merged 🎉

denar90 · 2018-09-12T19:48:58Z

Still wondering about version,
3.4.0 or 4.0.0 wdyt?

paulirish · 2018-09-12T20:21:53Z

i'd say stick to semver

so only bump major if you have a breaking change.

but since pwmetrics is using simulated throttling instead of devtools (right?), then the metrics numbers aren't really comparable. that'd be a breaking change. :)

so 4.0! :)

denar90 · 2018-09-12T20:36:44Z

but since pwmetrics is using simulated throttling instead of devtools (right?), then the metrics numbers aren't really comparable. that'd be a breaking change. :)

yup. thanks)

denar90 commented May 28, 2018

View reviewed changes

paulirish reviewed May 30, 2018

View reviewed changes

denar90 added 4 commits June 7, 2018 21:24

updates for using lighthouse v3

628485a

improve imports, split up logical modules, bring back perf-config

5c7c9a8

update deps

13ffe96

update wunderbar, fix output for terminals without width

013f03c

denar90 force-pushed the lh-v3-update branch from 494d63c to 013f03c Compare June 7, 2018 19:10

denar90 added 4 commits June 8, 2018 23:35

fix/drop tests

3e4ff9f

bump node support, improve travis

33e0666

bum LH to commit with _Crdp fix

187f6ca

improve travis

6e598e5

denar90 commented Jun 8, 2018

View reviewed changes

get back to old friends metrics

765d1eb

denar90 added 2 commits August 12, 2018 00:42

improve gulp example, get rid of throwing if expectation error happened

1aadbcc

update docs

46f20b0

denar90 changed the title ~~updates for using lighthouse v3, wip~~ updates for using lighthouse v3 Aug 11, 2018

denar90 mentioned this pull request Aug 13, 2018

Expectation errors should be based on median run values #181

Closed

paulirish requested changes Aug 15, 2018

View reviewed changes

denar90 added 3 commits August 15, 2018 23:44

clean travis

fae1e62

cleanup after review

c616627

improve metrics adaptation

c3dcc99

denar90 mentioned this pull request Aug 27, 2018

Add Visually Complete 100% to group: 'metrics' GoogleChrome/lighthouse#5741

Closed

denar90 added 3 commits August 28, 2018 00:15

fix tests, updates

ad9df21

marge master into lh-update-v3

7a8a4d2

try run example on CI

2a94a58

try run example on CI

bf18d46

denar90 mentioned this pull request Aug 27, 2018

Error: connect ECONNREFUSED 127.0.0.1:9222 #179

Closed

use logger for LH runner

c09cc46

denar90 mentioned this pull request Aug 29, 2018

Typo in error logging for gsheets #186

Closed

denar90 added 7 commits August 30, 2018 22:16

cleanup log

f77d47e

improve imports

7649f92

improve metrics import in example

a91e1de

drop vscode-chrome-debug-core dep

c59f1de

check expectations base on median results

9c45a26

bump LH version, small fixes

658e7c5

update spreadsheet

0de4518

denar90 merged commit ea85988 into master Sep 12, 2018

denar90 mentioned this pull request Sep 12, 2018

Update LH dep to v3 #158

Closed

denar90 deleted the lh-v3-update branch September 12, 2018 21:31

updates for using lighthouse v3 #178

updates for using lighthouse v3 #178

Conversation

denar90 commented May 28, 2018 • edited Loading

Choose a reason for hiding this comment

denar90 commented May 30, 2018

paulirish left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

metrics audit

individual audits

Choose a reason for hiding this comment

Choose a reason for hiding this comment

key metrics

worthwhile diagnostic metrics

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

denar90 commented Jun 8, 2018

Choose a reason for hiding this comment

denar90 commented Jul 2, 2018

denar90 commented Aug 4, 2018

denar90 commented Aug 11, 2018

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

denar90 commented Aug 27, 2018

denar90 commented Sep 7, 2018

denar90 commented Sep 12, 2018

paulirish commented Sep 12, 2018 via email

denar90 commented Sep 12, 2018

denar90 commented Sep 12, 2018

paulirish commented Sep 12, 2018

denar90 commented Sep 12, 2018

denar90 commented May 28, 2018 •

edited

Loading

`metrics` audit