no-extraneous-dependencies

[benmosher]

Report node_module imports that are not declared in the sibling package.json.

Not sure what to name this. no-missing-dependencies maybe? or just dependencies? (edit: thanks @grabbou for a great suggestion)

Needs an option to check in dependencies, devDependencies, optional, peer, etc. Just dependencies by default.

i.e. given package.json:

{ "dependencies": { "x": "^1.0.1" } }

then report for

import x from 'x' // not reported
import y from 'y' // reported: module not in dependencies.

inspired by https://github.com/mysticatea/eslint-plugin-node/blob/master/docs/rules/no-missing-import.md

[benmosher]

a la #158, need to allow/disallow core modules based on a flag, probably.

[benmosher]

Given the recent Twitter attention, this is what I was imagining as an implementation:

• resolve import path
• if resolved path is not in a node_modules folder, ignore
• if no package.json is found for the module being linted, do not report (by default, at least)
• if it is in a node_modules, verify that the relevant package.json for the module being linted (i.e. path.join(findRoot(context.getFilename()), 'package.json')) references the module

Options:

• valid-types: Array of strings, default is ['dependencies']. The keys of package.json where the module must appear.
  I would envision setting this to ['dependencies', 'devDependencies'] for my test/ directory, but using the (default) ['dependencies'] for src//lib/ paths.
• same object as no-unresolved that allows enabling/disablng commonjs (for require(x)) and/or amd (for define/require).

BTW: valid-types is a terrible name, IMO. Would be happy with almost anything else.

I can also imagine this being a separate plugin (eslint-plugin-npm?) and leveraging the recently-published eslint-module-utils. That guy lets you leverage all the resolve goodness of this plugin without having to be an explicit part. It knows everything this plugin knows about traversing imports/requires, resolving with Webpack/Node/whatever, etc.

Actually... I think that might be the best option. See eslint-plugin-git for an example of how to integrate with the module utils package.

[jfmengels]

I'm (as I'm writing) implementing it (and close to done).
I actually have a "similar" repo eslint-plugin-import-order where I don't mind adding it. The name would be a bit misleading, but it would fit IMO.
(Though I was thinking of proposing to add the import-order rule here, so that would be a step in the wrong direction for that, cc @sindresorhus)

As for the options I was thinking more of something like a single options object, containing the following key:

• devDependencies : Boolean, default is true. Allow/disallow devDependencies

I was thinking of only checking whether the module is registered in package.json, not actually resolving the name. Not sure I see the point of resolving, except maybe for dependencies with reference to submodules, like lodash/find

[grabbou]

Not sure what to name this. no-missing-dependencies maybe? or just dependencies?

Maybe just no-extraneous-dependencies or something related, to follow warnings that npm usually prints after installing when it finds non-package defined modules?

[benmosher]

Regarding sort order: I just referenced your plugin at the existing issue for sort order (#162). I've always been more interested in the static analysis and less about style, but I'm cool to add style rules if folks are interested in them. Meant to reach out to you earlier about it but hadn't gotten around to it, yet. 😅

Regarding rule option: devDependencies boolean sounds better than what I said, for sure. 😄 I was over-engineering it. Though I still think it should be false by default, but mention in the report message that the module was found in devDependencies instead of dependencies when the option is false. (edit: I forgot peerDependencies, but I still like the map-of-bool-flags. probably ought to default peerDependencies to true)

devDependencies are tough because the CI tests will always pass if the deps are in the wrong group (since all of them get installed when building/testing, generally), but this is an opportunity to warn if they are referenced from "production" source.

Regarding resolution: I'm concerned about imports of aliased modules, if the import is not resolved.

For example, I often do something akin to NODE_PATH=./src so I can make src references without doing a full relative path, i.e. jsx/MyCoolComponent instead of ../../jsx/MyCoolComponent:

// NODE_PATH=./src
import MCC from 'jsx/MyCoolComponent' // would it report that 'jsx' is not a dependency?

It's not clear to me how you could disambiguate such an aliased import/require vs. one that does target node_modules via npm without resolving the actual dependency.

Thus the suggestion of a new eslint-plugin-npm that basically just does this one thing.

Also: it would be pretty sweet if eslint --fix actually wrote the dependency into the package.json...

[jfmengels]

Meant to reach out to you earlier about it but hadn't gotten around to it, yet.

Same here then :p

"devDependencies" ... I still think it should be false by default

I lint my test files (obviously), and they require devDependencies. I don't see an easy way to tell ESLint that "this file is a production file, and that is a development file", as you can't define a config for a glob (yet). Since I don't know how to do that, I don't see how it is practical to have the default be false. (note: I like to put my tests next to the files they test. In some cases, adding a second .eslintrc in the test folder would work)

I'm concerned about imports of aliased modules, if the import is not resolved

I didn't actually know about NODE_PATH=./src. I knew Webpack had a plugin to do that, but didn't now it was this "easy" to add it to a "normal" project.
But you're right, this is tricky... I'll take a look at eslint-module-utils for this I guess. But you'd need to call eslint with that same NODE_PATH too right?

[sindresorhus]

I don't see an easy way to tell ESLint that "this file is a production file, and that is a development file"

You can do it automatically for packages that use the files field in package.json as you would know exactly what files are included in the package.

For example, I often do something akin to NODE_PATH=./src

Ugh, that is such an anti-pattern... I don't think that should be a blocker for anything. Could add an option for people needing to work around it though.

Also: it would be pretty sweet if eslint --fix actually wrote the dependency into the package.json...

👍 👍

[jfmengels]

You can do it automatically for packages that use the files field in package.json as you would know exactly what files are included in the package.

True, but you do that for packages, not for apps (afaik?). But that would be one possible way, you're right, though pretty complex to implement (done that in an eslint-plugin-ava rule, it's far from being builtin in ESLint).

Also: it would be pretty sweet if eslint --fix actually wrote the dependency into the package.json...

In my implementation, in the error message I've added a tip to run npm i -S <name> that you can copy paste in your terminal to fix the problem. Not as nice, but that might help a bit :)

[jamestalmage]

You can do it automatically for packages that use the files field in package.json ...

That won't work for anybody who transpiles the contents of their src folder to a dist folder. You would put dist in package.json, but you would want src linted.

[sindresorhus]

That won't work for anybody who transpiles the contents of their src folder to a dist folder. You would put dist in package.json, but you would want src linted.

I admit it's not foolproof, but it would work for many use-cases. For the rest, there could be an option to specify additional file paths.