Skip to content

Commit da8d584

Browse files
jeromehljharb
jeromeh
authored andcommitted
[Fix] no-extraneous-dependencies: fix package name algorithm
- resolve nested package.json problems (a/b/c import will check a, a/b and a/b/c) - resolve renamed dependencies: checks the import name and the resolve package name Fixes #2066. Fixes #2065. Fixes #2058. Fixes #2078.
1 parent 81b9d24 commit da8d584

File tree

8 files changed

+84
-23
lines changed

8 files changed

+84
-23
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
1414
- [`no-cycle`]: fix false negative when file imports a type after importing a value in Flow ([#2083], thanks [@cherryblossom000])
1515
- [`order`]: restore default behavior unless `type` is in groups ([#2087], thanks [@grit96])
1616
- [`no-import-module-exports`]: Don't crash if packages have no entrypoint ([#2099], thanks [@eps1lon])
17+
- [`no-extraneous-dependencies`]: fix package name algorithm ([#2097], thanks [@paztis])
1718

1819
### Changed
1920
- [Docs] Add `no-relative-packages` to list of to the list of rules ([#2075], thanks [@arvigeus])
@@ -796,6 +797,7 @@ for info on changes for earlier releases.
796797
[`memo-parser`]: ./memo-parser/README.md
797798

798799
[#2099]: https://github.com/benmosher/eslint-plugin-import/pull/2099
800+
[#2097]: https://github.com/benmosher/eslint-plugin-import/pull/2097
799801
[#2090]: https://github.com/benmosher/eslint-plugin-import/pull/2090
800802
[#2087]: https://github.com/benmosher/eslint-plugin-import/pull/2087
801803
[#2083]: https://github.com/benmosher/eslint-plugin-import/pull/2083

src/rules/no-extraneous-dependencies.js

+63-21
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,36 @@ function getModuleRealName(resolved) {
126126
return getFilePackageName(resolved);
127127
}
128128

129+
function checkDependencyDeclaration(deps, packageName) {
130+
// in case of sub package.json inside a module
131+
// check the dependencies on all hierarchy
132+
const packageHierarchy = [];
133+
const packageNameParts = packageName.split('/');
134+
packageNameParts.forEach((namePart, index) => {
135+
if (!namePart.startsWith('@')) {
136+
const ancestor = packageNameParts.slice(0, index + 1).join('/');
137+
packageHierarchy.push(ancestor);
138+
}
139+
});
140+
141+
return packageHierarchy.reduce((result, ancestorName) => {
142+
return {
143+
isInDeps: result.isInDeps || deps.dependencies[ancestorName] !== undefined,
144+
isInDevDeps: result.isInDevDeps || deps.devDependencies[ancestorName] !== undefined,
145+
isInOptDeps: result.isInOptDeps || deps.optionalDependencies[ancestorName] !== undefined,
146+
isInPeerDeps: result.isInPeerDeps || deps.peerDependencies[ancestorName] !== undefined,
147+
isInBundledDeps:
148+
result.isInBundledDeps || deps.bundledDependencies.indexOf(ancestorName) !== -1,
149+
};
150+
}, {
151+
isInDeps: false,
152+
isInDevDeps: false,
153+
isInOptDeps: false,
154+
isInPeerDeps: false,
155+
isInBundledDeps: false,
156+
});
157+
}
158+
129159
function reportIfMissing(context, deps, depsOptions, node, name) {
130160
// Do not report when importing types
131161
if (node.importKind === 'type' || (node.parent && node.parent.importKind === 'type') || node.importKind === 'typeof') {
@@ -139,37 +169,49 @@ function reportIfMissing(context, deps, depsOptions, node, name) {
139169
const resolved = resolve(name, context);
140170
if (!resolved) { return; }
141171

142-
// get the real name from the resolved package.json
143-
// if not aliased imports (alias/react for example) will not be correctly interpreted
144-
// fallback on original name in case no package.json found
145-
const packageName = getModuleRealName(resolved) || getModuleOriginalName(name);
146-
147-
const isInDeps = deps.dependencies[packageName] !== undefined;
148-
const isInDevDeps = deps.devDependencies[packageName] !== undefined;
149-
const isInOptDeps = deps.optionalDependencies[packageName] !== undefined;
150-
const isInPeerDeps = deps.peerDependencies[packageName] !== undefined;
151-
const isInBundledDeps = deps.bundledDependencies.indexOf(packageName) !== -1;
152-
153-
if (isInDeps ||
154-
(depsOptions.allowDevDeps && isInDevDeps) ||
155-
(depsOptions.allowPeerDeps && isInPeerDeps) ||
156-
(depsOptions.allowOptDeps && isInOptDeps) ||
157-
(depsOptions.allowBundledDeps && isInBundledDeps)
172+
const importPackageName = getModuleOriginalName(name);
173+
const importPackageNameDeclaration = checkDependencyDeclaration(deps, importPackageName);
174+
175+
if (importPackageNameDeclaration.isInDeps ||
176+
(depsOptions.allowDevDeps && importPackageNameDeclaration.isInDevDeps) ||
177+
(depsOptions.allowPeerDeps && importPackageNameDeclaration.isInPeerDeps) ||
178+
(depsOptions.allowOptDeps && importPackageNameDeclaration.isInOptDeps) ||
179+
(depsOptions.allowBundledDeps && importPackageNameDeclaration.isInBundledDeps)
180+
) {
181+
return;
182+
}
183+
184+
// test the real name from the resolved package.json
185+
// if not aliased imports (alias/react for example), importPackageName can be misinterpreted
186+
const realPackageName = getModuleRealName(resolved);
187+
const realPackageNameDeclaration = checkDependencyDeclaration(deps, realPackageName);
188+
189+
if (realPackageNameDeclaration.isInDeps ||
190+
(depsOptions.allowDevDeps && realPackageNameDeclaration.isInDevDeps) ||
191+
(depsOptions.allowPeerDeps && realPackageNameDeclaration.isInPeerDeps) ||
192+
(depsOptions.allowOptDeps && realPackageNameDeclaration.isInOptDeps) ||
193+
(depsOptions.allowBundledDeps && realPackageNameDeclaration.isInBundledDeps)
158194
) {
159195
return;
160196
}
161197

162-
if (isInDevDeps && !depsOptions.allowDevDeps) {
163-
context.report(node, devDepErrorMessage(packageName));
198+
if ((
199+
importPackageNameDeclaration.isInDevDeps ||
200+
realPackageNameDeclaration.isInDevDeps
201+
) && !depsOptions.allowDevDeps) {
202+
context.report(node, devDepErrorMessage(realPackageName));
164203
return;
165204
}
166205

167-
if (isInOptDeps && !depsOptions.allowOptDeps) {
168-
context.report(node, optDepErrorMessage(packageName));
206+
if ((
207+
importPackageNameDeclaration.isInOptDeps ||
208+
realPackageNameDeclaration.isInOptDeps
209+
) && !depsOptions.allowOptDeps) {
210+
context.report(node, optDepErrorMessage(realPackageName));
169211
return;
170212
}
171213

172-
context.report(node, missingErrorMessage(packageName));
214+
context.report(node, missingErrorMessage(realPackageName));
173215
}
174216

175217
function testConfig(config, filename) {

tests/files/node_modules/rxjs/index.js

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/files/node_modules/rxjs/operators/index.js

+1
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/files/node_modules/rxjs/operators/package.json

+5
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/files/node_modules/rxjs/package.json

+5
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

tests/files/package.json

+2-1
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@
1111
"@org/package": "^1.0.0",
1212
"jquery": "^3.1.0",
1313
"lodash.cond": "^4.3.0",
14-
"pkg-up": "^1.0.0"
14+
"pkg-up": "^1.0.0",
15+
"rxjs": "^1.0.0"
1516
},
1617
"optionalDependencies": {
1718
"lodash.isarray": "^4.0.0"

tests/src/rules/no-extraneous-dependencies.js

+5-1
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ ruleTester.run('no-extraneous-dependencies', rule, {
8888
}),
8989
test({
9090
code: `
91-
// @flow
91+
// @flow
9292
import typeof TypeScriptModule from 'typescript';
9393
`,
9494
options: [{ packageDir: packageDirWithFlowTyped }],
@@ -150,6 +150,10 @@ ruleTester.run('no-extraneous-dependencies', rule, {
150150
code: 'import "@generated/bar/and/sub/path"',
151151
settings: { 'import/core-modules': ['@generated/bar'] },
152152
}),
153+
// check if "rxjs" dependency declaration fix the "rxjs/operators subpackage
154+
test({
155+
code: 'import "rxjs/operators"',
156+
}),
153157
],
154158
invalid: [
155159
test({

0 commit comments

Comments
 (0)