From 112dd0599cd76aa01a7152c2ec53acbc303b6424 Mon Sep 17 00:00:00 2001 From: Steven Hargrove Date: Tue, 6 Feb 2018 23:16:10 -0500 Subject: [PATCH 1/8] added logic for improving monorepo support for no-extraneous-dependencies rule --- src/rules/no-extraneous-dependencies.js | 64 +++++++++++++++++++++---- 1 file changed, 54 insertions(+), 10 deletions(-) diff --git a/src/rules/no-extraneous-dependencies.js b/src/rules/no-extraneous-dependencies.js index bb684e448..6c186c058 100644 --- a/src/rules/no-extraneous-dependencies.js +++ b/src/rules/no-extraneous-dependencies.js @@ -7,22 +7,66 @@ import importType from '../core/importType' import isStaticRequire from '../core/staticRequire' import docsUrl from '../docsUrl' +function hasKeys(obj = {}) { + return Object.keys(obj).length > 0 +} + +function extractDepFields(pkg) { + return { + dependencies: pkg.dependencies || {}, + devDependencies: pkg.devDependencies || {}, + optionalDependencies: pkg.optionalDependencies || {}, + peerDependencies: pkg.peerDependencies || {}, + } +} + function getDependencies(context, packageDir) { try { - const packageContent = packageDir - ? JSON.parse(fs.readFileSync(path.join(packageDir, 'package.json'), 'utf8')) - : readPkgUp.sync({cwd: context.getFilename(), normalize: false}).pkg + const paths = [] + const packageContent = { + dependencies: {}, + devDependencies: {}, + optionalDependencies: {}, + peerDependencies: {}, + } - if (!packageContent) { - return null + if (Object.prototype.hasOwnProperty(context.settings, 'import/paths')) { + paths.push( + ...context.settings['import/paths'] + .map(path.resolve) + ) } - return { - dependencies: packageContent.dependencies || {}, - devDependencies: packageContent.devDependencies || {}, - optionalDependencies: packageContent.optionalDependencies || {}, - peerDependencies: packageContent.peerDependencies || {}, + if (packageDir) { + paths.unshift(packageDir) + } else { + Object.assign( + packageContent, + extractDepFields( + readPkgUp.sync({cwd: context.getFilename(), normalize: false}).pkg + ) + ) } + + if (paths.length) { + paths.forEach((dir) => { + Object.assign(packageContent, extractDepFields( + JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf8')) + )) + }) + } + + + if (![ + packageContent.dependencies, + packageContent.devDependencies, + packageContent.optionalDependencies, + packageContent.peerDependencies, + ].some(hasKeys)) { + return null + } + + return packageContent } catch (e) { if (packageDir && e.code === 'ENOENT') { context.report({ From 4ce8d72a94acdf2dd5d14445fa858995dfa6cbf4 Mon Sep 17 00:00:00 2001 From: Steven Hargrove Date: Tue, 6 Feb 2018 23:17:12 -0500 Subject: [PATCH 2/8] added tests to support new settings['import/paths'] logic for no-extraneous-dependencies rule --- tests/files/monorepo/package.json | 6 ++++ .../packages/nested-package/package.json | 6 ++++ tests/src/rules/no-extraneous-dependencies.js | 36 +++++++++++++++++++ 3 files changed, 48 insertions(+) create mode 100644 tests/files/monorepo/package.json create mode 100644 tests/files/monorepo/packages/nested-package/package.json diff --git a/tests/files/monorepo/package.json b/tests/files/monorepo/package.json new file mode 100644 index 000000000..3ed889ddf --- /dev/null +++ b/tests/files/monorepo/package.json @@ -0,0 +1,6 @@ +{ + "private": true, + "devDependencies": { + "left-pad": "^1.2.0" + } +} diff --git a/tests/files/monorepo/packages/nested-package/package.json b/tests/files/monorepo/packages/nested-package/package.json new file mode 100644 index 000000000..615c0a234 --- /dev/null +++ b/tests/files/monorepo/packages/nested-package/package.json @@ -0,0 +1,6 @@ +{ + "name": "nested-monorepo-pkg", + "dependencies": { + "react": "^16.0.0" + } +} diff --git a/tests/src/rules/no-extraneous-dependencies.js b/tests/src/rules/no-extraneous-dependencies.js index a8817931b..d80ba8760 100644 --- a/tests/src/rules/no-extraneous-dependencies.js +++ b/tests/src/rules/no-extraneous-dependencies.js @@ -15,6 +15,8 @@ const packageFileWithSyntaxErrorMessage = (() => { } })() const packageDirWithFlowTyped = path.join(__dirname, '../../files/with-flow-typed') +const packageDirMonoRepoRoot = path.join(__dirname, '../../files/monorepo') +const packageDirMonoRepoWithNested = path.join(__dirname, '../../files/monorepo/packages/nested-package') ruleTester.run('no-extraneous-dependencies', rule, { valid: [ @@ -75,8 +77,42 @@ ruleTester.run('no-extraneous-dependencies', rule, { options: [{packageDir: packageDirWithFlowTyped}], parser: 'babel-eslint', }), + test({ + code: 'import react from "react";', + options: [{packageDir: packageDirMonoRepoWithNested}], + // filename: path.join(process.cwd(), 'foo.spec.js'), + }), + test({ + code: 'import leftpad from "left-pad";', + options: [{packageDir: packageDirMonoRepoWithNested}], + settings: { 'import/paths': [packageDirMonoRepoRoot] }, + // filename: path.join(process.cwd(), 'foo.spec.js'), + }), + test({ + code: 'import leftpad from "left-pad";', + options: [{packageDir: packageDirMonoRepoRoot}], + settings: { 'import/paths': [packageDirMonoRepoRoot] }, + }), ], invalid: [ + test({ + code: 'import "not-a-dependency"', + options: [{packageDir: packageDirMonoRepoWithNested}], + settings: { 'import/paths': [packageDirMonoRepoRoot] }, + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: '\'not-a-dependency\' should be listed in the project\'s dependencies. Run \'npm i -S not-a-dependency\' to add it', + }], + }), + test({ + code: 'import "not-a-dependency"', + options: [{packageDir: packageDirMonoRepoRoot}], + settings: { 'import/paths': [packageDirMonoRepoRoot] }, + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: '\'not-a-dependency\' should be listed in the project\'s dependencies. Run \'npm i -S not-a-dependency\' to add it', + }], + }), test({ code: 'import "not-a-dependency"', errors: [{ From 815abf05741cbb7a197e1eeb67e0d5fcdf5a2b83 Mon Sep 17 00:00:00 2001 From: Steven Hargrove Date: Tue, 6 Feb 2018 23:17:47 -0500 Subject: [PATCH 3/8] added docs to describe new settings['import/paths'] behavior --- README.md | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/README.md b/README.md index ae6b92c04..5b61dfad9 100644 --- a/README.md +++ b/README.md @@ -353,6 +353,18 @@ settings: [`eslint_d`]: https://www.npmjs.com/package/eslint_d [`eslint-loader`]: https://www.npmjs.com/package/eslint-loader +#### `import/paths` + +Settings for `package.json` lookup paths in order to include their defined dependencies in addition to the next-immediate `package.json` relative to the linted file. For example, the [`no-extraneous-dependencies`] rule. + +This is useful in scenarios where your workspace includes several hierarchical `package.json` files. For example, a [lerna](https://github.com/lerna/lerna) monorepo where you've hoisted all your `devDependencies` to the `package.json` at the root of the monorepo. + +```yaml +# .eslintrc.yml +settings: + import/paths: ['.', 'packages/my-dev-utils'] +``` + ## SublimeLinter-eslint SublimeLinter-eslint introduced a change to support `.eslintignore` files From 275c6c7230bc2ca8474117ed5e7b8440151d05db Mon Sep 17 00:00:00 2001 From: Steven Hargrove Date: Tue, 6 Feb 2018 23:34:50 -0500 Subject: [PATCH 4/8] fixed hasOwnProperty conditional for import/paths setting --- src/rules/no-extraneous-dependencies.js | 4 ++-- tests/src/rules/no-extraneous-dependencies.js | 14 +++++++++++--- 2 files changed, 13 insertions(+), 5 deletions(-) diff --git a/src/rules/no-extraneous-dependencies.js b/src/rules/no-extraneous-dependencies.js index 6c186c058..05d5a6450 100644 --- a/src/rules/no-extraneous-dependencies.js +++ b/src/rules/no-extraneous-dependencies.js @@ -30,10 +30,10 @@ function getDependencies(context, packageDir) { peerDependencies: {}, } - if (Object.prototype.hasOwnProperty(context.settings, 'import/paths')) { + if (Object.prototype.hasOwnProperty.call(context.settings, 'import/paths')) { paths.push( ...context.settings['import/paths'] - .map(path.resolve) + .map((dir) => path.resolve(dir)) ) } diff --git a/tests/src/rules/no-extraneous-dependencies.js b/tests/src/rules/no-extraneous-dependencies.js index d80ba8760..96884a7d5 100644 --- a/tests/src/rules/no-extraneous-dependencies.js +++ b/tests/src/rules/no-extraneous-dependencies.js @@ -80,13 +80,11 @@ ruleTester.run('no-extraneous-dependencies', rule, { test({ code: 'import react from "react";', options: [{packageDir: packageDirMonoRepoWithNested}], - // filename: path.join(process.cwd(), 'foo.spec.js'), }), test({ code: 'import leftpad from "left-pad";', options: [{packageDir: packageDirMonoRepoWithNested}], settings: { 'import/paths': [packageDirMonoRepoRoot] }, - // filename: path.join(process.cwd(), 'foo.spec.js'), }), test({ code: 'import leftpad from "left-pad";', @@ -97,7 +95,17 @@ ruleTester.run('no-extraneous-dependencies', rule, { invalid: [ test({ code: 'import "not-a-dependency"', - options: [{packageDir: packageDirMonoRepoWithNested}], + filename: path.join(packageDirMonoRepoRoot, 'foo.js'), + settings: { 'import/paths': [packageDirMonoRepoRoot] }, + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: '\'not-a-dependency\' should be listed in the project\'s dependencies. Run \'npm i -S not-a-dependency\' to add it', + }], + }), + test({ + code: 'import "not-a-dependency"', + filename: path.join(packageDirMonoRepoWithNested, 'foo.js'), + options: [{packageDir: packageDirMonoRepoRoot}], settings: { 'import/paths': [packageDirMonoRepoRoot] }, errors: [{ ruleId: 'no-extraneous-dependencies', From 980b08736cbd99d5bf08c65cd1d7bbf73ed52857 Mon Sep 17 00:00:00 2001 From: Steven Hargrove Date: Wed, 7 Feb 2018 02:25:11 -0500 Subject: [PATCH 5/8] corrected order in which deps are located and applied --- src/rules/no-extraneous-dependencies.js | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/src/rules/no-extraneous-dependencies.js b/src/rules/no-extraneous-dependencies.js index 05d5a6450..21010caf6 100644 --- a/src/rules/no-extraneous-dependencies.js +++ b/src/rules/no-extraneous-dependencies.js @@ -38,14 +38,7 @@ function getDependencies(context, packageDir) { } if (packageDir) { - paths.unshift(packageDir) - } else { - Object.assign( - packageContent, - extractDepFields( - readPkgUp.sync({cwd: context.getFilename(), normalize: false}).pkg - ) - ) + paths.push(packageDir) } if (paths.length) { @@ -56,6 +49,14 @@ function getDependencies(context, packageDir) { }) } + if (!packageDir) { + Object.assign( + packageContent, + extractDepFields( + readPkgUp.sync({cwd: context.getFilename(), normalize: false}).pkg + ) + ) + } if (![ packageContent.dependencies, From 77d44234db6ef3ee89ded94b3795c5549583376d Mon Sep 17 00:00:00 2001 From: Steven Hargrove Date: Wed, 7 Feb 2018 12:38:43 -0500 Subject: [PATCH 6/8] replaced Object.prototype.hasOwnProperty usage with has module --- src/rules/no-extraneous-dependencies.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/rules/no-extraneous-dependencies.js b/src/rules/no-extraneous-dependencies.js index 21010caf6..3297731bf 100644 --- a/src/rules/no-extraneous-dependencies.js +++ b/src/rules/no-extraneous-dependencies.js @@ -1,5 +1,6 @@ import path from 'path' import fs from 'fs' +import has from 'has' import readPkgUp from 'read-pkg-up' import minimatch from 'minimatch' import resolve from 'eslint-module-utils/resolve' @@ -30,7 +31,7 @@ function getDependencies(context, packageDir) { peerDependencies: {}, } - if (Object.prototype.hasOwnProperty.call(context.settings, 'import/paths')) { + if (has(context.settings, 'import/paths')) { paths.push( ...context.settings['import/paths'] .map((dir) => path.resolve(dir)) From 72bb2cb4a5c389d5ec9a8fe20d8b851445b6964e Mon Sep 17 00:00:00 2001 From: Ben Mosher Date: Wed, 18 Apr 2018 07:14:31 -0400 Subject: [PATCH 7/8] merge import/paths setting into existing packageDir option --- README.md | 11 ------ docs/rules/no-extraneous-dependencies.md | 7 ++++ package.json | 1 - src/rules/no-extraneous-dependencies.js | 38 +++++++++---------- tests/files/node_modules/left-pad | 1 + tests/files/node_modules/react | 1 + tests/src/rules/no-extraneous-dependencies.js | 36 ++++++++++++++---- 7 files changed, 55 insertions(+), 40 deletions(-) create mode 120000 tests/files/node_modules/left-pad create mode 120000 tests/files/node_modules/react diff --git a/README.md b/README.md index 5b61dfad9..f86d0d161 100644 --- a/README.md +++ b/README.md @@ -353,17 +353,6 @@ settings: [`eslint_d`]: https://www.npmjs.com/package/eslint_d [`eslint-loader`]: https://www.npmjs.com/package/eslint-loader -#### `import/paths` - -Settings for `package.json` lookup paths in order to include their defined dependencies in addition to the next-immediate `package.json` relative to the linted file. For example, the [`no-extraneous-dependencies`] rule. - -This is useful in scenarios where your workspace includes several hierarchical `package.json` files. For example, a [lerna](https://github.com/lerna/lerna) monorepo where you've hoisted all your `devDependencies` to the `package.json` at the root of the monorepo. - -```yaml -# .eslintrc.yml -settings: - import/paths: ['.', 'packages/my-dev-utils'] -``` ## SublimeLinter-eslint diff --git a/docs/rules/no-extraneous-dependencies.md b/docs/rules/no-extraneous-dependencies.md index 069dab0ce..5c3542ebd 100644 --- a/docs/rules/no-extraneous-dependencies.md +++ b/docs/rules/no-extraneous-dependencies.md @@ -35,6 +35,13 @@ Also there is one more option called `packageDir`, this option is to specify the "import/no-extraneous-dependencies": ["error", {"packageDir": './some-dir/'}] ``` +It may also be an array of multiple paths, to support monorepos or other novel project +folder layouts: + +```js +"import/no-extraneous-dependencies": ["error", {"packageDir": ['./some-dir/', './root-pkg']}] +``` + ## Rule Details Given the following `package.json`: diff --git a/package.json b/package.json index 4edb2aa8a..2a68bbfa9 100644 --- a/package.json +++ b/package.json @@ -85,7 +85,6 @@ "doctrine": "1.5.0", "eslint-import-resolver-node": "^0.3.1", "eslint-module-utils": "^2.1.1", - "has": "^1.0.1", "lodash": "^4.17.4", "minimatch": "^3.0.3", "read-pkg-up": "^2.0.0" diff --git a/src/rules/no-extraneous-dependencies.js b/src/rules/no-extraneous-dependencies.js index 3297731bf..9d51018e9 100644 --- a/src/rules/no-extraneous-dependencies.js +++ b/src/rules/no-extraneous-dependencies.js @@ -1,6 +1,6 @@ import path from 'path' import fs from 'fs' -import has from 'has' +import { isArray, isEmpty } from 'lodash' import readPkgUp from 'read-pkg-up' import minimatch from 'minimatch' import resolve from 'eslint-module-utils/resolve' @@ -22,8 +22,8 @@ function extractDepFields(pkg) { } function getDependencies(context, packageDir) { + let paths = [] try { - const paths = [] const packageContent = { dependencies: {}, devDependencies: {}, @@ -31,26 +31,23 @@ function getDependencies(context, packageDir) { peerDependencies: {}, } - if (has(context.settings, 'import/paths')) { - paths.push( - ...context.settings['import/paths'] - .map((dir) => path.resolve(dir)) - ) - } - - if (packageDir) { - paths.push(packageDir) + if (!isEmpty(packageDir)) { + if (!isArray(packageDir)) { + paths = [path.resolve(packageDir)] + } else { + paths = packageDir.map(dir => path.resolve(dir)) + } } - if (paths.length) { - paths.forEach((dir) => { + if (!isEmpty(paths)) { + // use rule config to find package.json + paths.forEach(dir => { Object.assign(packageContent, extractDepFields( JSON.parse(fs.readFileSync(path.join(dir, 'package.json'), 'utf8')) )) }) - } - - if (!packageDir) { + } else { + // use closest package.json Object.assign( packageContent, extractDepFields( @@ -70,7 +67,7 @@ function getDependencies(context, packageDir) { return packageContent } catch (e) { - if (packageDir && e.code === 'ENOENT') { + if (!isEmpty(paths) && e.code === 'ENOENT') { context.report({ message: 'The package.json file could not be found.', loc: { line: 0, column: 0 }, @@ -112,9 +109,8 @@ function reportIfMissing(context, deps, depsOptions, node, name) { } const resolved = resolve(name, context) - if (!resolved) { - return - } + if (!resolved) { return } + const splitName = name.split('/') const packageName = splitName[0][0] === '@' ? splitName.slice(0, 2).join('/') @@ -170,7 +166,7 @@ module.exports = { 'devDependencies': { 'type': ['boolean', 'array'] }, 'optionalDependencies': { 'type': ['boolean', 'array'] }, 'peerDependencies': { 'type': ['boolean', 'array'] }, - 'packageDir': { 'type': 'string' }, + 'packageDir': { 'type': ['string', 'array'] }, }, 'additionalProperties': false, }, diff --git a/tests/files/node_modules/left-pad b/tests/files/node_modules/left-pad new file mode 120000 index 000000000..dbbbe75d2 --- /dev/null +++ b/tests/files/node_modules/left-pad @@ -0,0 +1 @@ +not-a-dependency \ No newline at end of file diff --git a/tests/files/node_modules/react b/tests/files/node_modules/react new file mode 120000 index 000000000..dbbbe75d2 --- /dev/null +++ b/tests/files/node_modules/react @@ -0,0 +1 @@ +not-a-dependency \ No newline at end of file diff --git a/tests/src/rules/no-extraneous-dependencies.js b/tests/src/rules/no-extraneous-dependencies.js index 96884a7d5..381b392cb 100644 --- a/tests/src/rules/no-extraneous-dependencies.js +++ b/tests/src/rules/no-extraneous-dependencies.js @@ -83,20 +83,18 @@ ruleTester.run('no-extraneous-dependencies', rule, { }), test({ code: 'import leftpad from "left-pad";', - options: [{packageDir: packageDirMonoRepoWithNested}], - settings: { 'import/paths': [packageDirMonoRepoRoot] }, + options: [{packageDir: [packageDirMonoRepoWithNested, packageDirMonoRepoRoot]}], }), test({ code: 'import leftpad from "left-pad";', options: [{packageDir: packageDirMonoRepoRoot}], - settings: { 'import/paths': [packageDirMonoRepoRoot] }, }), ], invalid: [ test({ code: 'import "not-a-dependency"', filename: path.join(packageDirMonoRepoRoot, 'foo.js'), - settings: { 'import/paths': [packageDirMonoRepoRoot] }, + options: [{packageDir: packageDirMonoRepoRoot }], errors: [{ ruleId: 'no-extraneous-dependencies', message: '\'not-a-dependency\' should be listed in the project\'s dependencies. Run \'npm i -S not-a-dependency\' to add it', @@ -106,7 +104,6 @@ ruleTester.run('no-extraneous-dependencies', rule, { code: 'import "not-a-dependency"', filename: path.join(packageDirMonoRepoWithNested, 'foo.js'), options: [{packageDir: packageDirMonoRepoRoot}], - settings: { 'import/paths': [packageDirMonoRepoRoot] }, errors: [{ ruleId: 'no-extraneous-dependencies', message: '\'not-a-dependency\' should be listed in the project\'s dependencies. Run \'npm i -S not-a-dependency\' to add it', @@ -115,7 +112,6 @@ ruleTester.run('no-extraneous-dependencies', rule, { test({ code: 'import "not-a-dependency"', options: [{packageDir: packageDirMonoRepoRoot}], - settings: { 'import/paths': [packageDirMonoRepoRoot] }, errors: [{ ruleId: 'no-extraneous-dependencies', message: '\'not-a-dependency\' should be listed in the project\'s dependencies. Run \'npm i -S not-a-dependency\' to add it', @@ -241,5 +237,31 @@ ruleTester.run('no-extraneous-dependencies', rule, { message: 'The package.json file could not be parsed: ' + packageFileWithSyntaxErrorMessage, }], }), - ], + test({ + code: 'import leftpad from "left-pad";', + filename: path.join(packageDirMonoRepoWithNested, 'foo.js'), + options: [{packageDir: packageDirMonoRepoWithNested}], + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: "'left-pad' should be listed in the project's dependencies. Run 'npm i -S left-pad' to add it", + }], + }), + test({ + code: 'import react from "react";', + filename: path.join(packageDirMonoRepoRoot, 'foo.js'), + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: "'react' should be listed in the project's dependencies. Run 'npm i -S react' to add it", + }], + }), + test({ + code: 'import react from "react";', + filename: path.join(packageDirMonoRepoWithNested, 'foo.js'), + options: [{packageDir: packageDirMonoRepoRoot}], + errors: [{ + ruleId: 'no-extraneous-dependencies', + message: "'react' should be listed in the project's dependencies. Run 'npm i -S react' to add it", + }], + }), + ] }) From 5111c797be563b4f755b8bbdb55b4e203541e7c5 Mon Sep 17 00:00:00 2001 From: Ben Mosher Date: Mon, 23 Apr 2018 10:01:49 -0400 Subject: [PATCH 8/8] revert `has` removal per @ljharb's note --- package.json | 1 + 1 file changed, 1 insertion(+) diff --git a/package.json b/package.json index f55668e25..860f822a6 100644 --- a/package.json +++ b/package.json @@ -84,6 +84,7 @@ "doctrine": "1.5.0", "eslint-import-resolver-node": "^0.3.1", "eslint-module-utils": "^2.2.0", + "has": "^1.0.1", "lodash": "^4.17.4", "minimatch": "^3.0.3", "read-pkg-up": "^2.0.0",