From 62bfc208a8f0eed53d942cda5df5c2d406db10b5 Mon Sep 17 00:00:00 2001 From: Jonathan Romano Date: Thu, 20 Oct 2022 16:00:31 -0400 Subject: [PATCH] [New] `no-extraneous-dependencies`: Add `considerInParents` option --- CHANGELOG.md | 3 + docs/rules/no-extraneous-dependencies.md | 12 ++- src/rules/no-extraneous-dependencies.js | 81 ++++++++++++++----- tests/files/monorepo/package.json | 9 ++- tests/src/rules/no-extraneous-dependencies.js | 66 +++++++++++++++ 5 files changed, 150 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index fe7f6771b8..6db8c07fbd 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -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]) @@ -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 @@ -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 diff --git a/docs/rules/no-extraneous-dependencies.md b/docs/rules/no-extraneous-dependencies.md index 660875d1da..4b22068042 100644 --- a/docs/rules/no-extraneous-dependencies.md +++ b/docs/rules/no-extraneous-dependencies.md @@ -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. diff --git a/src/rules/no-extraneous-dependencies.js b/src/rules/no-extraneous-dependencies.js index a149ca6599..43c28a366c 100644 --- a/src/rules/no-extraneous-dependencies.js +++ b/src/rules/no-extraneous-dependencies.js @@ -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'; @@ -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 = { @@ -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, + ), ); } @@ -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, }, @@ -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, diff --git a/tests/files/monorepo/package.json b/tests/files/monorepo/package.json index cf0b87ffa6..3c81aa46c6 100644 --- a/tests/files/monorepo/package.json +++ b/tests/files/monorepo/package.json @@ -5,5 +5,12 @@ }, "devDependencies": { "left-pad": "^1.2.0" - } + }, + "peerDependencies": { + "lodash": "4.17.21" + }, + "optionalDependencies": { + "chalk": "5.2.0" + }, + "bundledDependencies": ["commander"] } diff --git a/tests/src/rules/no-extraneous-dependencies.js b/tests/src/rules/no-extraneous-dependencies.js index 84aa8bb35d..3f593aaade 100644 --- a/tests/src/rules/no-extraneous-dependencies.js +++ b/tests/src/rules/no-extraneous-dependencies.js @@ -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"', @@ -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'),