addyosmani / webpack-lighthouse-plugin

A Webpack plugin for Lighthouse
Apache License 2.0
290 stars 12 forks source link

Error: Cannot find module 'lighthouse/lighthouse-core/lib/environment.js' #5

Closed skloiber-sz closed 7 years ago

skloiber-sz commented 7 years ago

Hi Addy,

when requiring the plugin in the webpack.config file and executing webpack I get this error message:

Error: Cannot find module 'lighthouse/lighthouse-core/lib/environment.js'

It seems that the node compatibility check was removed from lighthouse core.

Thanks, Stefan

addyosmani commented 7 years ago

Thanks for flagging this, @skloiber-sz. I'll check what we've changed upstream in Lighthouse and get this patched up in the module.

RichAyotte commented 7 years ago

'lighthouse/lighthouse-core/lib/environment.js' doesn't exist in lighthouse anymore. https://github.com/GoogleChrome/lighthouse/tree/master/lighthouse-core/lib

Is there a quick workaround before I dig too deep into this?

screendriver commented 7 years ago

Same issue here. Any workaround at the moment?

carlkenne commented 7 years ago

any news on this?

paulirish commented 7 years ago

This module is broken at the moment.

It will need some work to fix up.

Most of https://github.com/addyosmani/webpack-lighthouse-plugin/blob/master/src/lighthouse-bin.js can be nuked and replaced with https://github.com/GoogleChrome/lighthouse/tree/b354890076f2c077c5460b2fa56ded546cca72ee/docs#using-programmatically effectively.

I know addy is pretty busy right now.. Anyone wanna give this a shot?

addyosmani commented 7 years ago

I know addy is pretty busy right now.. Anyone wanna give this a shot?

My fault. Sorry for dropping the ball on this folks!

It will need some work to fix up.

If someone would like to help with a PR to update the plugin to use https://github.com/GoogleChrome/lighthouse/tree/b354890076f2c077c5460b2fa56ded546cca72ee/docs#using-programmatically directly that would be mucho awesome.

If not, I'll try to make some progress later in the week on rewriting.

carlkenne commented 7 years ago

I'd love to help out, so I had a look at the code with the intention of fixing it. However, if we use that example from the docs and nuke the rest from https://github.com/addyosmani/webpack-lighthouse-plugin/blob/master/src/lighthouse-bin.js we will loose some lighthouse functionality. The problem is that some functionality resides in lighthouse-cli (like saveAssets, saveArtifacts, html generation, error handling, various config params...). Using the example (which goes straight to lighthouse-core) would bypass that.

Ideally I think that functionality should be extracted from lighthouse-cli/bin.ts to lighthouse-core (in the lighthouse repo). But to get this plugin working again I would instead need to copy that functionality from lighthouse-cli/bin.ts to this plugin before using the example code from the docs (similar to what Addy has done already, so code would just slightly change). I could send a PR to show you what I mean. Unless I'm missing something here?

addyosmani commented 7 years ago

saveAssets, saveArtifacts, html generation, error handling, various config params

Unless I'm mistaken I believe you're right. cc'ing in @samccone in case we're missing something obvious.

But to get this plugin working again I would instead need to copy that functionality from lighthouse-cli/bin.ts to this plugin before using the example code from the docs

Let's wait to hear from @paulirish and/or @samccone before doing this. On the Lighthouse side they've supportive of folks not needing to copy large pieces of functionality (a problem in the past) but I'm unsure short-term how much room there would be for them to add-in the other features we're using above.

Thanks for any time you're able to commit to helping here @carlkenne. With some luck we'll get a little clarity on next steps soon.

paulirish commented 7 years ago

Ah I see. looks like you're saving results to disk. Gotcha. Assuming that's something we want to continue to do here... i have an alternate proposal:

You could use runLighthouse from lighthouse-cli which kinda does all this work itself. Check it out: https://github.com/GoogleChrome/lighthouse/blob/5add27db28459f0c4f57b873bc7cb168af77bf75/lighthouse-cli/bin.ts#L174-L199

Based on what's currently needed from this plugin, that actually seems totally compatible. If you wanted to process the JSON results yourself, you'd need to pick and choose these methods a bit more.

carlkenne commented 7 years ago

Alright, I tried it and it works. As a user I would assume that the plugin worked like the cli tool, so copying 20 or so lines from the beginning of bin.ts file would be needed (flags / config setup). Will work short term until lighthouse makes some changes to that part.

paulirish commented 7 years ago

@carlkenne that seems good.

in the future we'd like to improve consuming the module, so we'll use the implementation you do here as guidance for what public API we should expose.

carlkenne commented 7 years ago

@paulirish I tried it out in a real project today and failed. It appears that when I require('lighthouse-cli/bin') the line with const cliFlags = getFlags(); is called and throws an error because no argv was provided from the command line (throw new Error('Please provide a url'); from cli-flags.ts).

It makes sense since I'm not using the command line and this code expects you to. As I see it, the only way to go forward now is to copy the entire bin.ts and remove the const cliFlags = getFlags(); line (not a good long term solution). Or if someone could split this file in 2 in the lighthouse repo...

paulirish commented 7 years ago

cc @ebidel

paulirish commented 7 years ago

with https://github.com/GoogleChrome/lighthouse/pull/2602 complete, this should be much more straightforward.

something like this:

const runLighthouse = require('lighthouse/lighthouse-cli/run.js').runLighthouse;

const flags = { 
  port: 0,
  chromeFlags: '',
  output: 'html'
};

runLighthouse(url, flags).then(results => {
  results....
});

The above requires this PR to land, but that should happen by the time you read this. :)

It will get the JSON passed back so you can evaluated it, but it'll also save the HTML report to disk.

benjaminhoffman commented 7 years ago

got this error this morning after giving lighthouse a shot in my webpack config. any thoughts on when this may be fixed?

evenstensberg commented 7 years ago

I'll get a fix to this by tomorrow @benjaminhoffman , sorry for the delay

addyosmani commented 7 years ago

Thanks Even! Sorry for the delays in replies folks. Traveling for work at.

On Sep 24, 2017 2:23 AM, "Even Stensberg" notifications@github.com wrote:

I'll get a fix to this by tomorrow @benjaminhoffman https://github.com/benjaminhoffman , sorry for the delay

— You are receiving this because you were assigned. Reply to this email directly, view it on GitHub https://github.com/addyosmani/webpack-lighthouse-plugin/issues/5#issuecomment-331654640, or mute the thread https://github.com/notifications/unsubscribe-auth/AAGxaTtGZTXnjvN6idWNUnXk9PxTYOpoks5slT6qgaJpZM4L-fbb .

evenstensberg commented 7 years ago

7 Done