Failed loading plugins in current working directory

[koh110]

I'm having trouble lhci collect with plugins is failed.

https://github.com/GoogleChrome/lighthouse#plugins
https://github.com/treosh/lighthouse-plugin-field-performance

$ which lhci
/usr/local/bin/lhci
$ lhci --version
0.6.1
$ pwd
/home/koh110/dev/tmp/lighthouse-test
$ cat package.json 
{
  "private": true,
  "dependencies": {
    "lighthouse-plugin-field-performance": "^2.2.1"
  }
}
$ ls -la
total 112
drwxrwxr-x   3 koh110 koh110  4096 11月 15 17:11 ./
drwxrwxr-x  17 koh110 koh110  4096 11月 14 19:34 ../
-rw-rw-r--   1 koh110 koh110   137 11月 14 17:18 lighthouse.js
drwxrwxr-x 224 koh110 koh110 12288 11月 14 19:28 node_modules/
-rw-rw-r--   1 koh110 koh110   127 11月 14 19:28 package.json
-rw-rw-r--   1 koh110 koh110 74807 11月 14 19:28 package-lock.json

$ lhci collect --url=https://google.com --config=./lighthouse.js 
Running Lighthouse 3 time(s) on https://google.com
Run GoogleChrome/lighthouse-ci#1...failed!
Error: Lighthouse failed with exit code 1
    at ChildProcess.<anonymous> (/usr/local/lib/node_modules/@lhci/cli/src/collect/node-runner.js:119:21)
    at ChildProcess.emit (events.js:315:20)
    at Process.ChildProcess._handle.onexit (internal/child_process.js:277:12)

Runtime error encountered: Unable to locate plugin: `lighthouse-plugin-field-performance`.
     Tried to require() from these locations:
       /usr/local/lib/node_modules/@lhci/cli/node_modules/lighthouse/lighthouse-core/config
       /home/koh110/dev/tmp/lighthouse-test/lighthouse-plugin-field-performance
       /usr/local/lib/node_modules/@lhci/cli/node_modules/lighthouse/lighthouse-core/config/lighthouse-plugin-field-performance
Error: Unable to locate plugin: `lighthouse-plugin-field-performance`.
     Tried to require() from these locations:
       /usr/local/lib/node_modules/@lhci/cli/node_modules/lighthouse/lighthouse-core/config
       /home/koh110/dev/tmp/lighthouse-test/lighthouse-plugin-field-performance
       /usr/local/lib/node_modules/@lhci/cli/node_modules/lighthouse/lighthouse-core/config/lighthouse-plugin-field-performance
    at resolveModule (/usr/local/lib/node_modules/@lhci/cli/node_modules/lighthouse/lighthouse-core/config/config-helpers.js:213:9)
    at Function.mergePlugins (/usr/local/lib/node_modules/@lhci/cli/node_modules/lighthouse/lighthouse-core/config/config.js:444:9)
    at new Config (/usr/local/lib/node_modules/@lhci/cli/node_modules/lighthouse/lighthouse-core/config/config.js:335:25)
    at generateConfig (/usr/local/lib/node_modules/@lhci/cli/node_modules/lighthouse/lighthouse-core/index.js:60:10)
    at lighthouse (/usr/local/lib/node_modules/@lhci/cli/node_modules/lighthouse/lighthouse-core/index.js:43:18)
    at runLighthouse (/usr/local/lib/node_modules/@lhci/cli/node_modules/lighthouse/lighthouse-cli/run.js:224:32)
    at processTicksAndRejections (internal/process/task_queues.js:93:5)

Cause

This seems to be due to the resolveModule in lighthouse-core.

lighthouse/lighthouse-core/config/config-helpers.js

Line 178 in 378a31f

function resolveModule(moduleIdentifier, configDir, category) {

In my case, I would expect the plugin to be loaded under SECOND condition.

lhci is installed in global and plugin exists in current working directory.

Solution

I think resolveModule should be coded like this.
https://nodejs.org/api/modules.html#modules_require_resolve_request_options

   // then the config is an object and so has no path.
-   const cwdPath = path.resolve(process.cwd(), moduleIdentifier);
   try {
-    return require.resolve(cwdPath);
+    return require.resolve(moduleIdentifier, { paths: [process.cwd()]});
   } catch (e) {}

If I'm right, this is not lhci problem. I think I will send a PR to lighthouse-core, is this correct?

I followed the code and it appears since 2016.
#679

So I couldn't determine this was a bug or a specification.

[patrickhulce]

Thanks for filing @koh110! You're right this is a lighthouse-core problem with resolution of plugins when lighthouse is globally installed (like in this example). I'll go ahead and transfer this issue :)

[koh110]

@patrickhulce Thanks for transferring! I checked lighthouse command. And It has same problem.

$ lighthouse https://www.google.com --config-path=./config.js
Runtime error encountered: Unable to locate plugin: `lighthouse-plugin-field-performance`.
     Tried to require() from these locations:
       /usr/local/lib/node_modules/lighthouse/lighthouse-core/config
       /home/koh110/dev/tmp/lighthouse-test/lighthouse-plugin-field-performance
       /home/koh110/dev/tmp/lighthouse-test/lighthouse-plugin-field-performance
Error: Unable to locate plugin: `lighthouse-plugin-field-performance`.
     Tried to require() from these locations:

When I tried to fix lighthouse-core, I think that this code is not expecting to load plugins installed by npm. It might be expecting to load plugins written in local.

If this idea is correct, I would have to modify this code.

  // First ...
  try {
-    return require.resolve(moduleIdentifier);
+    return require.resolve(moduleIdentifier, { paths: [process.cwd()] });
  } catch (e) {}

  // Second
  const cwdPath = path.resolve(process.cwd(), moduleIdentifier);
  try {
    return require.resolve(cwdPath);
  } catch (e) {}

  const errorString = 'Unable to locate ' + (category ? `${category}: ` : '') +
    `\`${moduleIdentifier}\`.
     Tried to require() from these locations:
       ${__dirname}
       ${cwdPath}`;

  if (!configDir) {
    throw new Error(errorString);
  }

  // Finally, ...
  const relativePath = path.resolve(configDir, moduleIdentifier);
  try {
-    return require.resolve(relativePath);
+    return require.resolve(moduleIdentifier,  { paths: [configDir] });
  } catch (requireError) {}

  throw new Error(errorString + `
       ${relativePath}`);

How do you feel about this code?

[patrickhulce]

IIUC, paths will replace the default resolution path, so we might need to augment with both?

I think it might help if we outline all the cases we need to deal with...

• Paths to a file
  • relative to the config file ✅
  • relative to the current working directory ✅
  • relative to the config.js file in lighthouse ✅
• node modules
  • LH globally installed, local node module 👎
  • LH locally installed, global node module ?
  • both globally installed ✅
  • both locally installed ✅

I think that's our support today. Any cases I'm missing?

[brendankenny]

Good call on fixing this!

• LH locally installed, global node module ?

This should work from the first require.resolve() (require will walk up the path then check the global installs).

I don't want to accidentally break relative paths for some existing case, so maybe the simplest thing is we can leave everything as-is and add a new step that does require.resolve(moduleIdentifier, { paths: [process.cwd()] }); specifically for the global lighthouse/local plugin case?

We could probably simplify more, but all the global/local/relative permutations make it difficult to get great test coverage on all these cases.

Separate from that change, it would also be good to annotate which cases each check is covering (maybe with examples), as it's hard to remember what's doing what :) I think the first is trying to do all the modules-based stuff and the last two are meant mostly for paths, but it would be good to make that crystal clear.

[patrickhulce]

add a new step that does require.resolve(moduleIdentifier, { paths: [process.cwd()] }); specifically for the global lighthouse/local plugin case?

SGTM 👍

[koh110]

Thanks for reviews!
Adding a new step seems good because it's so simple and doesn't break compatibility.

I will try to add a new step and test.
I agree that it's difficult to create path-related test, so I'd like to write a minimal amount of tests and comments.