Bug: Multiple packageDirs in no-extraneous-dependencies

[aravindet]

According to the docs, packageDirs accepts an array of paths, and (presumably) the package.json's found at all the paths have some effect.

In the current implementation, only the last element in the array is used.

This is because Object.assign() is used to merge the dependencies found in each file. As Object.assign() performs a shallow merge, and the extractDepFields function adds default empty objects for fields that are not defined in package.json, every field ends up being overridden in each iteration.

I suggest that a deep merge be performed here instead of Object.assign.

[ljharb]

You're right that this logic is incorrect. It'd be great to get a failing test :-)

[aravindet]

I'm not familiar enough with this repo to send a PR, but I can suggest a minimal test:

Directory structure:

root
+- package.json
+- .eslintrc
+- bar
   +- package.json
   +- index.js

root/.eslintrc

{
  "rules": {
    "import/no-extraneous-dependencies": ["error", {
      "packageDir": [".", ".."]
    }]
  }
}

root/package.json

{
  "dependencies": { "foodep": "x" }
}

root/bar/package.json

{
  "dependencies": { "bardep": "x" }
}

root/bar/index.js

require('foodep');
require('bardep');
require('bazdep');

Assertion

eslint bar/index.js in all the scenarios should show exactly one error: on the require('bazdep') line.

Other scenarios

In other tests test, packageDir can be:

• ["..", "."] (order swapped)
• ["/absolute_root", "/absolute_root/bar"]
• ["/absolute_root/bar", "/absolute_root"]

(Only allowing relative paths is extremely limiting, and it is not clear why this limitation exists in the documentation while path.resolve() accepts absolute paths. I would suggest that absolute paths should also be supported.)

[pzhine]

@aravindet @ljharb Already fixed this in a PR, with a test that fails for the current version.
@benmosher Can you review #1176 and merge if it looks good?