treosh / lighthouse-ci-action

Audit URLs using Lighthouse and test performance with Lighthouse CI.
MIT License
1.16k stars 82 forks source link

Lighthouse Plugin doesn't work without hacks #16

Closed roelfjan closed 4 years ago

roelfjan commented 4 years ago

I created a sample project with a Lighthouse Plugin for my blogpost: https://engineering.q42.nl/making-a-lighthouse-plugin-work-with-lighthouse-ci/.

I couldn't get it to work on GitHub Actions without adding the lines:

- run: npm install lighthouse
- run: mv node_modules/lighthouse-plugin-social-sharing

Otherwise it gave te error messages: Runtime error encountered: Cannot find module 'lighthouse' Runtime error encountered: Unable to locate plugin: lighthouse-plugin-field-social-sharing

My sample repo is: https://github.com/Q42/lighthouse-plugin-sample-project

paulirish commented 4 years ago

Thanks. Should be sorted with this https://github.com/GoogleChrome/lighthouse/pull/9997

paulirish commented 4 years ago

@roelfjan can you take a look at the changes and tell me if you think that'd resolve this issue?

(also technically this issue probably belongs in the LH repo but no big deal.. i appreciate the report)

paulirish commented 4 years ago

(also technically this issue probably belongs in the LH repo but no big deal.. i appreciate the report)

Ah I see that this is indeed a report for the action, my bad.

+@connorjclark We also have some path resolution issues that affect plugin developers. My current guess is the path resolution fix you used for lighthouse-plugin-social-sharing ended up breaking things for the plugin user.

Anyway it's mostly our (lighthouse-core) bug. :/

connorjclark commented 4 years ago

I forked your repo - thanks for setting that up and making it real easy to dig into.

https://github.com/connorjclark/lighthouse-plugin-sample-project

few things here:

  1. You must run npm install to get the packages the you declare in your package.json
  2. This treosh action bundles its own version of Lighthouse (and a bunch of other stuff in its node_modules). When your project's action does uses: treosh/lighthouse-ci-action@v2, the copy of Lighthouse that runs is in a totally different directory (-run: ls -a helped me understand that). Node's resolution strategy won't have access to it, so...
  3. Copying the plugin module you install to this treosh action's bundled node_modules seems like the least hacky option.

I believe we can avoid this hairy issue pretty simply. This treosh action should accept a parameter extra_modules, which it will call npm install for each. Those modules will then live alongside the bundled modules, and will be in the right spot. @exterkamp, sg?

roelfjan commented 4 years ago

Great work, thanks for the fix! I also updated my blogpost.

connorjclark commented 4 years ago

Great! BTW - I don't see the updated content. Still in draft?

roelfjan commented 4 years ago

Great! BTW - I don't see the updated content. Still in draft?

'Works on my machine'? Also checked it a on another machine. It should contain Update: ... in italic, some strikethroughs and an updated example.

connorjclark commented 4 years ago

I missed that update banner. I saw some things that looked like the old post but it looks great now. Thanks for the bug report!