Update to lighthouse 2 and fix errors

[evenstensberg]

So I've noticed that the repo is a bit outdated. I saw the CLI options disabled as we're not running in a bash context too. There's a couple of more things that needs to be addressed before this is ready for merge:

1. How are you using the Printer module? Right now, it defaults to nothing, which makes the build throw on run. I've set a fallback to json.
2. What is the expected terminal output? I did some changes, and there's a lot of clutty json formatted code in the terminal, please help me with that 💯
3. Strip unneeded code, drag in lighthouse as a main module and remove require calls to lighthouse-core, what do you think about this?
4. Readme needs a minor brush

Cheers 🎈

[evenstensberg]

Mind checking out the branch and give feedback on the results you expect @addyosmani ? Right now I can tell that it is different than the Lighthouse CLI, don't know the expected output from when it was working

[addyosmani]

@ev1stensberg Thanks a lot for working on this PR! I've been a little delayed with reviews out on manager training. I was wondering if you had seen https://github.com/addyosmani/webpack-lighthouse-plugin/issues/5#issuecomment-313824390 from Paul Irish (commented today)

[addyosmani]

How are you using the Printer module? Right now, it defaults to nothing, which makes the build throw on run. I've set a fallback to json.

I was previously trying to emulate the output experience you would get with lighthouse-cli, exposing a report printout in the terminal when a run successfully completed.

What is the expected terminal output? I did some changes, and there's a lot of clutty json formatted code in the terminal, please help me with that 💯

This sounds like a side-effect of the upstream API churn 🙈 As mentioned, the output shouldn't be JSON in the terminal but rather a pretty-printed report as one used to get with the CLI.

Strip unneeded code, drag in lighthouse as a main module and remove require calls to lighthouse-core, what do you think about this?

That sounds pretty reasonable. I would also take a look at that thread I linked up to as Paul's suggestions should help us with some clean-up too.

Readme needs a minor brush

Suggestions welcome 👍

[evenstensberg]

Sweet. I'll get a mergable PR on the README and this one up by the end of the week hopefully!

[evenstensberg]

So the PR has landed and we can remove the manual printer logic?

[addyosmani]

Sweet. I'll get a mergable PR on the README and this one up by the end of the week hopefully!

👍 Landed the README minor cleanup.

So the PR has landed and we can remove the manual printer logic?

The current printer handling in Lighthouse CLI can be found at https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-cli/bin.ts and https://github.com/GoogleChrome/lighthouse/blob/master/lighthouse-cli/printer.ts. Is this what you were looking for?

[evenstensberg]

Was trying to not change too much code , but as lighthouse output now is prettified by itself and reachable invoking without any deep modules , we could remove the printer, yea?

[addyosmani]

but as lighthouse output now is prettified by itself and reachable invoking without any deep modules , we could remove the printer, yea?

I believe yes we should be able to.

[evenstensberg]

Sweet. Will push a new commit later this week, a bit hectic these days!

[evenstensberg]

Printer, logging etc is working now, but it won't exit and print result path etc.. Could be an issue with the promise race, or a flag that is missing, will check later this week.

[addyosmani]

Printer, logging etc is working now, but it won't exit and print result path etc.. Could be an issue with the promise race, or a flag that is missing, will check later this week.

Thanks for getting the printer and logging output working, @ev1stensberg! Appreciate any additional checking on why it isn't exiting when you get another chance.

[evenstensberg]

No probs, will finish the PR later this week, promise!

[addyosmani]

🍪 🍪 🍪 🥛

[evenstensberg]

Should be working now @addyosmani

[addyosmani]

@ev1stensberg I checked out your PR and tried running the demo locally but wasn't able to get the dependencies reconciling correctly.

Output

> webpack-lighthouse-plugin@1.0.2 demo /Users/addyo/projects/webpack-lighthouse-plugin > cd test && webpack . module.js:487 throw err; ^ Error: Cannot find module 'lighthouse/lighthouse-core/lib/log' at Function.Module._resolveFilename (module.js:485:15) at Function.Module._load (module.js:437:25) at Module.require (module.js:513:17) at require (internal/module.js:11:18) at Object. (/Users/addyo/projects/webpack-lighthouse-plugin/src/lighthouse-bin.js:27:13) at Module._compile (module.js:569:30) at Object.Module._extensions..js (module.js:580:10) at Module.load (module.js:503:32) at tryModuleLoad (module.js:466:12) at Function.Module._load (module.js:458:3) npm ERR! code ELIFECYCLE npm ERR! errno 1 npm ERR! webpack-lighthouse-plugin@1.0.2 demo: `cd test && webpack.` npm ERR! Exit status 1 npm ERR! npm ERR! Failed at the webpack-lighthouse-plugin@1.0.2 demo script. npm ERR! This is probably not a problem with npm. There is likely additional logging output above.

It looks like the version of Lighthouse that was being pulled in might have been based on a specific directory structure in mind since https://github.com/GoogleChrome/lighthouse/tree/master/lighthouse-core/lib no longer appears to contain log.

Reading through https://github.com/GoogleChrome/lighthouse/commit/c787b75420525dc00ae91ece688e29f0034d7cf4, it seems this has been replaced with lighthouse-logger which @samccone published over at https://www.npmjs.com/package/lighthouse-logger. Can we switch over to that logger?

[paulirish]

yup log moved to that module, which i think you'll need as a dep if you want to log through the same mechanism.

[evenstensberg]

Sure, lemme rebase in a sec

[evenstensberg]

@addyosmani R4R