Eslint-plugin-import: no-extraneous-dependencies only targets one package.json

[Robinfr]

In the Node.js environment, a call using require() will walk up the tree to find the dependency, this means that a dependency that is required can be in any node_modules folder up the tree. Currently it only looks at one folder.

Could this be adjusted?

[Robinfr]

Also related to https://github.com/benmosher/eslint-plugin-import/issues/458 which allows for a single package folder to be set. But in this case, I have multiple..

[ljharb]

It walks up the tree until it finds package.json, then it stops.

[Robinfr]

Yes, it shouldn't. It should keep walking and find all of the package.json files and merge them for this rule. This is how Node does it also.

[ljharb]

Can you provide a concrete example of what problem you need solved?

[Robinfr]

Sure, I have a Lerna monorepo which has several packages that contain tests that make use of Chai and Sinon. Since I don't want to install those in all packages, I installed it at the root level. This works perfectly well, except that this rule is complaining.

[ljharb]

That's not how a lerna monorepo is supposed to be organized tho; lerna bootstrap is supposed to install all those as dev deps via npm link. Each package in that multirepo is supposed to be able to function as if it was the root of the repo.

[Robinfr]

Hmm thats not true though. Just look at the Babel repository for example. Besides this works because of the way node works. So why shouldn't it work here?

[ljharb]

The babel repo isn't the best example for the best way to set up a lerna monorepo :-)

Regardless, does that mean you're running the linter per-package?

[Robinfr]

No I'm running the linter for all packages at once. Each package has their own .eslintrc file however.

[ljharb]

Gotcha.

While I think that organizing your repo this way is a very bad practice, I agree that since node does it, this plugin should support it - at least up to the dir in which eslint is run (traversing upwards from there could hide bugs if there's /package.json or ~/package.json)

[Robinfr]

Aside from this issue, what would be your suggested way of organizing such a repo?

[ljharb]

I'd suggest making each dir inside packages have its own package.json, have root: true in its .eslintrc, and be 100% self-containing - in other words, I should be able to cd into each package dir, and everything should function identically as if i had each package dir in a separate repo.

Then, the root package.json would contain lerna commands in "scripts" to automate common tasks across packages.

[Hypnosphi]

That's not how a lerna monorepo is supposed to be organized

Lerna docs disagree: https://github.com/lerna/lerna#common-devdependencies

[ljharb]

I'm using "supposed" here based on community best practices; if Lerna documentation deviates from that, Lerna is wrong.

[Robinfr]

I guess you can make both work by having Lerna hoist certain dev dependencies. This will make each module still self-contained, but you'll also run into this problem potentially.

[ljharb]

Yes, lerna should only hoist deps when the versions are the same; if it doesn’t work this way now, it’s a bug.

[Robinfr]

Any update on this? I would like to use this rule, but I can't until it works properly.

[evocateur]

lerna bootstrap --hoist makes it faster, but it doesn't strictly preclude _listing_ the dev deps in each package. It's mostly a performance optimization that takes advantage of the node resolution algorithm. It's not meant to be an excuse for implicit dependencies (dev or otherwise).

[hulkish]

+1

i feel like this could be solved by providing a way to configure lookup paths for a package.json in addition to the current behavior of seeking first found package.json relative to the context of the file being linted.

[penx]

@ljharb hopefully this is a concrete example of why you would want some dependencies to be at the root of a monorepo.

I have a monorepo, one of the packages is a dev tool to generate documentation that is a devDependency of a lot of other packages. I only expect to run this tool with the package checked out under the monorepo structure.

I can either:

1. add the tool as a devDependency to every package that needs it
2. add the tool as a devDependency at the root of the monorepo

Unfortunately, if I do (1), then make a code change to the tool and then run lerna publish, lerna will increment the devDepencency version of the tool in every package that uses it and then republish a new version of all these packages.

I only want to republish packages when dependencies that are relevant to users outside of the monorepo structure change, so I am considering putting devDependencies like these on the monorepo's root package.json.

Project specific issue: https://github.com/penx/govuk-react/issues/175#issuecomment-379890945

Alternatively, it would be nice for no-extraneous-dependencies config to accept an array of whitelisted dependencies so that should not error if missing.

[Hypnosphi]

it would be nice for no-extraneous-dependencies config to accept an array of whitelisted dependencies so that should not error if missing

https://github.com/benmosher/eslint-plugin-import#importcore-modules

[ljharb]

@penx tbh that sounds like a flaw (not the only one) in lerna publish :-/

[evocateur]

@penx Just turn off brittle import/no-extraneous-dependencies for test files, and move all the devDependencies to the root. If you're using lerna, you might as well recognize that you don't _have_ to pretend your leaf packages are self-sufficient (why bother with a multi-package repository otherwise?).

---
  # repo-root .eslintrc.yaml
  root: true
  plugins:
    # or just extend airbnb ¯\_(ツ)_/¯
    - import
  rules:
    import/no-extraneous-dependencies: error

  # ↓ this is the important part
  overrides:
    - files: "**/*{-,.}{test,stories}.js"
      rules:
        # devDependencies are all in the root
        import/no-extraneous-dependencies: off

import/no-unresolved is still able to ensure you actually _have_ those dev dependencies installed in the root, and is more than sufficient for the task.

Each package in that [multi-package repository] is supposed to be able to function as if it was the root of the repo.

@ljharb Citation needed?

[ljharb]

The reasons to "bother" with a multi-package repo aren't because you don't want self-sufficient leaf packages, they're because you can make related changes in multiple packages in the same PR, and more easily run tests against multiple packages at once.

[evocateur]

Self-sufficient leaf packages aren’t at all necessary to accomplish those two goals, they’re just a stylistic preference. Repeating the same devDependencies and scripts and configuration &c in every single leaf package is a mind-numbing maintenance burden.

[penx]

I think having a config option for no-extraneous-dependencies to support multiple parent directories with package.json files would be useful, though perhaps disabled by default.

I can certainly see the argument for making a package in a monorepo self sufficient so that a developer who only wants to work on one package can just cd to that package and yarn/npm install. For devDependencies that install a CLI tool it is useful to have this on the path when running scripts in a sub project (I'm not sure if .bin from a parent node_modules folder is included in the npm script resolution path, I don't think it is).

However I agree with @evocateur that this is a stylistic preference. I also find the maintenance of repeated devDependencies a pain, and can see the benefit of certain devDependencies (that are only used by files that assume they are being run as part of the monorepo) to be in the root package.json.

[hulkish]

@ljharb I view this as a debate of correctness vs efficiency/what's practical.

I believe I have a very valid use case for needing to hoist dev dependencies to monorepo root. I don't think being "correct" is worth the trade of a poorer dev environment. Not to.mention, the difference also has performance implications - Using leaf packages for dev dependencies being far less work for installs.

I also think it's inappropriate to suggest that it's not a valid concern all because you personally find it ugly.