Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[New] no-extraneous-dependencies: Add considerInParents option #2481

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange
- [`prefer-default-export`]: add "target" option ([#2602], thanks [@azyzz228])
- [`no-absolute-path`]: add fixer ([#2613], thanks [@adipascu])
- [`no-duplicates`]: support inline type import with `inlineTypeImport` option ([#2475], thanks [@snewcomer])
- [`no-extraneous-dependencies`]: Add `considerInParents` option to support package.json files in parent folders ([#2481], thanks [@luxaritas])

### Fixed
- [`order`]: move nested imports closer to main import entry ([#2396], thanks [@pri1311])
Expand Down Expand Up @@ -1091,6 +1092,7 @@ for info on changes for earlier releases.
[#2506]: https://github.com/import-js/eslint-plugin-import/pull/2506
[#2503]: https://github.com/import-js/eslint-plugin-import/pull/2503
[#2490]: https://github.com/import-js/eslint-plugin-import/pull/2490
[#2481]: https://github.com/import-js/eslint-plugin-import/pull/2481
[#2475]: https://github.com/import-js/eslint-plugin-import/pull/2475
[#2473]: https://github.com/import-js/eslint-plugin-import/pull/2473
[#2466]: https://github.com/import-js/eslint-plugin-import/pull/2466
Expand Down Expand Up @@ -1757,6 +1759,7 @@ for info on changes for earlier releases.
[@ludofischer]: https://github.com/ludofischer
[@Lukas-Kullmann]: https://github.com/Lukas-Kullmann
[@lukeapage]: https://github.com/lukeapage
[@luxaritas]: https://github.com/luxaritas
[@lydell]: https://github.com/lydell
[@magarcia]: https://github.com/magarcia
[@Mairu]: https://github.com/Mairu
Expand Down
12 changes: 11 additions & 1 deletion docs/rules/no-extraneous-dependencies.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,17 @@ There are 2 boolean options to opt into checking extra imports that are normally
"import/no-extraneous-dependencies": ["error", {"includeInternal": true, "includeTypes": true}]
```

Also there is one more option called `packageDir`, this option is to specify the path to the folder containing package.json.
To take `package.json` files in parent directories into account, use the `considerInParents` option.
This takes an array of strings, which can include `"prod"`, `"dev"`, `"peer"`, `"optional"`, and/or `"bundled"`.
The default is an empty array (only the nearest `package.json` is taken into account).

For example, the following would allow imports when the relevant packages are found in either `dependencies`
or `devDependencies` in either the closest `package.json` or any `package.json` found in parent directories:
```js
"import/no-extraneous-dependencies": ["error", { "considerInParents": ["prod", "dev"] }]
```

To specify the path to the folder containing package.json, use the `packageDir` option.

If provided as a relative path string, will be computed relative to the current working directory at linter execution time. If this is not ideal (does not work with some editor integrations), consider using `__dirname` to provide a path relative to your configuration.

Expand Down
81 changes: 62 additions & 19 deletions src/rules/no-extraneous-dependencies.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,5 @@
import path from 'path';
import fs from 'fs';
import pkgUp from 'eslint-module-utils/pkgUp';
import minimatch from 'minimatch';
import resolve from 'eslint-module-utils/resolve';
import moduleVisitor from 'eslint-module-utils/moduleVisitor';
Expand Down Expand Up @@ -40,16 +39,43 @@ function extractDepFields(pkg) {
};
}

function getPackageDepFields(packageJsonPath, throwAtRead) {
if (!depFieldCache.has(packageJsonPath)) {
const depFields = extractDepFields(readJSON(packageJsonPath, throwAtRead));
depFieldCache.set(packageJsonPath, depFields);
function getPackageDepFields(packageDir, considerInParents, requireInDir) {
const cacheKey = JSON.stringify({ packageDir, considerInParents: [...considerInParents] });

if (!depFieldCache.has(cacheKey)) {
// try in the current directory, erroring if the user explicitly specified this directory
// and reading fails
const parsedPackage = readJSON(path.join(packageDir, 'package.json'), requireInDir);
const depFields = extractDepFields(parsedPackage || {});

// If readJSON returned nothing, we want to keep searching since the current directory didn't
// have a package.json. Also keep searching if we're merging in some set of parents dependencies.
// However, if we're already at the root, stop.
if ((!parsedPackage || considerInParents.size > 0) && packageDir !== path.parse(packageDir).root) {
const parentDepFields = getPackageDepFields(path.dirname(packageDir), considerInParents, false);

Object.keys(depFields).forEach(depsKey => {
if (
(depsKey === 'dependencies' && considerInParents.has('prod')) ||
(depsKey === 'devDependencies' && considerInParents.has('dev')) ||
(depsKey === 'peerDependencies' && considerInParents.has('peer')) ||
(depsKey === 'optionalDependencies' && considerInParents.has('optional'))
) {
Object.assign(depFields[depsKey], parentDepFields[depsKey]);
}
if (depsKey === 'bundledDependencies' && considerInParents.has('bundled')) {
depFields[depsKey] = depFields[depsKey].concat(parentDepFields[depsKey]);
}
});
}

depFieldCache.set(cacheKey, depFields);
}

return depFieldCache.get(packageJsonPath);
return depFieldCache.get(cacheKey);
}

function getDependencies(context, packageDir) {
function getDependencies(context, packageDir, considerInParents) {
let paths = [];
try {
const packageContent = {
Expand All @@ -71,22 +97,24 @@ function getDependencies(context, packageDir) {
if (paths.length > 0) {
// use rule config to find package.json
paths.forEach(dir => {
const packageJsonPath = path.join(dir, 'package.json');
const _packageContent = getPackageDepFields(packageJsonPath, true);
Object.keys(packageContent).forEach(depsKey =>
Object.assign(packageContent[depsKey], _packageContent[depsKey]),
);
const _packageContent = getPackageDepFields(dir, considerInParents, true);
Object.keys(packageContent).forEach(depsKey => {
if (depsKey === 'bundledDependencies') {
packageContent[depsKey] = packageContent[depsKey].concat(_packageContent[depsKey]);
} else {
Object.assign(packageContent[depsKey], _packageContent[depsKey]);
}
});
});
} else {
const packageJsonPath = pkgUp({
cwd: context.getPhysicalFilename ? context.getPhysicalFilename() : context.getFilename(),
normalize: false,
});

// use closest package.json
Object.assign(
packageContent,
getPackageDepFields(packageJsonPath, false),
getPackageDepFields(
path.dirname(context.getPhysicalFilename ? context.getPhysicalFilename() : context.getFilename()),
considerInParents,
false,
),
);
}

Expand Down Expand Up @@ -275,6 +303,21 @@ module.exports = {
'packageDir': { 'type': ['string', 'array'] },
'includeInternal': { 'type': ['boolean'] },
'includeTypes': { 'type': ['boolean'] },
'considerInParents': {
'type': 'array',
'uniqueItems': true,
'additionalItems': false,
'items': {
'type': 'string',
'enum': [
'prod',
'dev',
'peer',
'bundled',
'optional',
],
},
},
},
'additionalProperties': false,
},
Expand All @@ -284,7 +327,7 @@ module.exports = {
create(context) {
const options = context.options[0] || {};
const filename = context.getPhysicalFilename ? context.getPhysicalFilename() : context.getFilename();
const deps = getDependencies(context, options.packageDir) || extractDepFields({});
const deps = getDependencies(context, options.packageDir, new Set(options.considerInParents || [])) || extractDepFields({});

const depsOptions = {
allowDevDeps: testConfig(options.devDependencies, filename) !== false,
Expand Down
9 changes: 8 additions & 1 deletion tests/files/monorepo/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -5,5 +5,12 @@
},
"devDependencies": {
"left-pad": "^1.2.0"
}
},
"peerDependencies": {
"lodash": "4.17.21"
},
"optionalDependencies": {
"chalk": "5.2.0"
},
"bundledDependencies": ["commander"]
}
66 changes: 66 additions & 0 deletions tests/src/rules/no-extraneous-dependencies.js
Original file line number Diff line number Diff line change
Expand Up @@ -116,6 +116,30 @@ ruleTester.run('no-extraneous-dependencies', rule, {
code: 'import rightpad from "right-pad";',
options: [{ packageDir: [packageDirMonoRepoRoot, packageDirMonoRepoWithNested] }],
}),
test({
code: 'import leftpad from "left-pad";',
options: [{ packageDir: packageDirMonoRepoWithNested, considerInParents: ['prod', 'dev'] }],
}),
test({
code: 'import rightpad from "right-pad";',
options: [{ packageDir: packageDirMonoRepoWithNested, considerInParents: ['prod', 'dev'] }],
}),
test({
code: 'import leftpad from "left-pad";',
options: [{ packageDir: packageDirMonoRepoWithNested, considerInParents: ['dev'] }],
}),
test({
code: 'import lodash from "lodash";',
options: [{ packageDir: packageDirMonoRepoWithNested, considerInParents: ['peer'] }],
}),
test({
code: 'import chalk from "chalk";',
options: [{ packageDir: packageDirMonoRepoWithNested, considerInParents: ['optional'] }],
}),
test({
code: 'import commander from "commander";',
options: [{ packageDir: packageDirMonoRepoWithNested, considerInParents: ['bundled'] }],
}),
test({ code: 'import foo from "@generated/foo"' }),
test({
code: 'import foo from "@generated/foo"',
Expand Down Expand Up @@ -319,6 +343,48 @@ ruleTester.run('no-extraneous-dependencies', rule, {
message: "'left-pad' should be listed in the project's dependencies. Run 'npm i -S left-pad' to add it",
}],
}),
test({
code: 'import rightpad from "right-pad";',
options: [{ packageDir: packageDirMonoRepoWithNested, considerInParents: [] }],
errors: [{
message: "'right-pad' should be listed in the project's dependencies. Run 'npm i -S right-pad' to add it",
}],
}),
test({
code: 'import rightpad from "right-pad";',
options: [{ packageDir: packageDirMonoRepoWithNested, considerInParents: ['dev'] }],
errors: [{
message: "'right-pad' should be listed in the project's dependencies. Run 'npm i -S right-pad' to add it",
}],
}),
test({
code: 'import leftpad from "left-pad";',
options: [{ packageDir: packageDirMonoRepoWithNested, considerInParents: ['prod'] }],
errors: [{
message: "'left-pad' should be listed in the project's dependencies. Run 'npm i -S left-pad' to add it",
}],
}),
test({
code: 'import lodash from "lodash";',
options: [{ packageDir: packageDirMonoRepoWithNested, considerInParents: ['prod'] }],
errors: [{
message: "'lodash' should be listed in the project's dependencies. Run 'npm i -S lodash' to add it",
}],
}),
test({
code: 'import chalk from "chalk";',
options: [{ packageDir: packageDirMonoRepoWithNested, considerInParents: ['prod'] }],
errors: [{
message: "'chalk' should be listed in the project's dependencies. Run 'npm i -S chalk' to add it",
}],
}),
test({
code: 'import commander from "commander";',
options: [{ packageDir: packageDirMonoRepoWithNested, considerInParents: ['prod'] }],
errors: [{
message: "'commander' should be listed in the project's dependencies. Run 'npm i -S commander' to add it",
}],
}),
test({
code: 'import react from "react";',
filename: path.join(packageDirMonoRepoRoot, 'foo.js'),
Expand Down