-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
No relative parent imports rule (#1093)
* No relative parent imports rule * Fix test * Add tests for require() * Add support for dynamic imports * Typo * Removes `src/core/staticImport` and adds support for dynamic imports to `utils/moduleVisitor` instead. * Make the error messages more actionable. * Add a lot more detail around how to fix errors in the docs. * docs grammar * docs grammar
- Loading branch information
1 parent
8f668c7
commit e6f5c13
Showing
7 changed files
with
244 additions
and
1 deletion.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,120 @@ | ||
# no-relative-parent-imports | ||
|
||
Use this rule to prevent imports to folders in relative parent paths. | ||
|
||
This rule is useful for enforcing tree-like folder structures instead of complex graph-like folder structures. While this restriction might be a departure from Node's default resolution style, it can lead large, complex codebases to be easier to maintain. If you've ever had debates over "where to put files" this rule is for you. | ||
|
||
To fix violations of this rule there are three general strategies. Given this example: | ||
|
||
``` | ||
numbers | ||
└── three.js | ||
add.js | ||
``` | ||
|
||
```js | ||
// ./add.js | ||
export default function (numbers) { | ||
return numbers.reduce((sum, n) => sum + n, 0); | ||
} | ||
|
||
// ./numbers/three.js | ||
import add from '../add'; // violates import/no-relative-parent-imports | ||
|
||
export default function three() { | ||
return add([1, 2]); | ||
} | ||
``` | ||
|
||
You can, | ||
|
||
1. Move the file to be in a sibling folder (or higher) of the dependency. | ||
|
||
`three.js` could be be in the same folder as `add.js`: | ||
|
||
``` | ||
three.js | ||
add.js | ||
``` | ||
|
||
or since `add` doesn't have any imports, it could be in it's own directory (namespace): | ||
|
||
``` | ||
math | ||
└── add.js | ||
three.js | ||
``` | ||
|
||
2. Pass the dependency as an argument at runtime (dependency injection) | ||
|
||
```js | ||
// three.js | ||
export default function three(add) { | ||
return add([1, 2]); | ||
} | ||
|
||
// somewhere else when you use `three.js`: | ||
import add from './add'; | ||
import three from './numbers/three'; | ||
console.log(three(add)); | ||
``` | ||
|
||
3. Make the dependency a package so it's globally available to all files in your project: | ||
|
||
```js | ||
import add from 'add'; // from https://www.npmjs.com/package/add | ||
export default function three() { | ||
return add([1,2]); | ||
} | ||
``` | ||
|
||
These are (respectively) static, dynamic & global solutions to graph-like dependency resolution. | ||
|
||
### Examples | ||
|
||
Given the following folder structure: | ||
|
||
``` | ||
my-project | ||
├── lib | ||
│ ├── a.js | ||
│ └── b.js | ||
└── main.js | ||
``` | ||
|
||
And the .eslintrc file: | ||
``` | ||
{ | ||
... | ||
"rules": { | ||
"import/no-relative-parent-imports": "error" | ||
} | ||
} | ||
``` | ||
|
||
The following patterns are considered problems: | ||
|
||
```js | ||
/** | ||
* in my-project/lib/a.js | ||
*/ | ||
|
||
import bar from '../main'; // Import parent file using a relative path | ||
``` | ||
|
||
The following patterns are NOT considered problems: | ||
|
||
```js | ||
/** | ||
* in my-project/main.js | ||
*/ | ||
|
||
import foo from 'foo'; // Import package using module path | ||
import a from './lib/a'; // Import child file using relative path | ||
|
||
/** | ||
* in my-project/lib/a.js | ||
*/ | ||
|
||
import b from './b'; // Import sibling file using relative path | ||
``` |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,34 @@ | ||
import moduleVisitor, { makeOptionsSchema } from 'eslint-module-utils/moduleVisitor' | ||
import docsUrl from '../docsUrl' | ||
import { basename } from 'path' | ||
|
||
import importType from '../core/importType' | ||
|
||
module.exports = { | ||
meta: { | ||
docs: { | ||
url: docsUrl('no-relative-parent-imports'), | ||
}, | ||
schema: [makeOptionsSchema()], | ||
}, | ||
|
||
create: function noRelativePackages(context) { | ||
const myPath = context.getFilename() | ||
if (myPath === '<text>') return {} // can't cycle-check a non-file | ||
|
||
function checkSourceValue(sourceNode) { | ||
const depPath = sourceNode.value | ||
if (importType(depPath, context) === 'parent') { | ||
context.report({ | ||
node: sourceNode, | ||
message: 'Relative imports from parent directories are not allowed. ' + | ||
`Please either pass what you're importing through at runtime ` + | ||
`(dependency injection), move \`${basename(myPath)}\` to same ` + | ||
`directory as \`${depPath}\` or consider making \`${depPath}\` a package.`, | ||
}) | ||
} | ||
} | ||
|
||
return moduleVisitor(checkSourceValue, context.options[0]) | ||
}, | ||
} |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,73 @@ | ||
import { RuleTester } from 'eslint' | ||
import rule from 'rules/no-relative-parent-imports' | ||
import { test as _test, testFilePath } from '../utils' | ||
|
||
const test = def => _test(Object.assign(def, { | ||
filename: testFilePath('./internal-modules/plugins/plugin2/index.js'), | ||
parser: 'babel-eslint', | ||
})) | ||
|
||
const ruleTester = new RuleTester() | ||
|
||
ruleTester.run('no-relative-parent-imports', rule, { | ||
valid: [ | ||
test({ | ||
code: 'import foo from "./internal.js"', | ||
}), | ||
test({ | ||
code: 'import foo from "./app/index.js"', | ||
}), | ||
test({ | ||
code: 'import foo from "package"', | ||
}), | ||
test({ | ||
code: 'require("./internal.js")', | ||
options: [{ commonjs: true }], | ||
}), | ||
test({ | ||
code: 'require("./app/index.js")', | ||
options: [{ commonjs: true }], | ||
}), | ||
test({ | ||
code: 'require("package")', | ||
options: [{ commonjs: true }], | ||
}), | ||
test({ | ||
code: 'import("./internal.js")', | ||
}), | ||
test({ | ||
code: 'import("./app/index.js")', | ||
}), | ||
test({ | ||
code: 'import("package")', | ||
}), | ||
], | ||
|
||
invalid: [ | ||
test({ | ||
code: 'import foo from "../plugin.js"', | ||
errors: [ { | ||
message: 'Relative imports from parent directories are not allowed. Please either pass what you\'re importing through at runtime (dependency injection), move `index.js` to same directory as `../plugin.js` or consider making `../plugin.js` a package.', | ||
line: 1, | ||
column: 17, | ||
} ], | ||
}), | ||
test({ | ||
code: 'require("../plugin.js")', | ||
options: [{ commonjs: true }], | ||
errors: [ { | ||
message: 'Relative imports from parent directories are not allowed. Please either pass what you\'re importing through at runtime (dependency injection), move `index.js` to same directory as `../plugin.js` or consider making `../plugin.js` a package.', | ||
line: 1, | ||
column: 9, | ||
} ], | ||
}), | ||
test({ | ||
code: 'import("../plugin.js")', | ||
errors: [ { | ||
message: 'Relative imports from parent directories are not allowed. Please either pass what you\'re importing through at runtime (dependency injection), move `index.js` to same directory as `../plugin.js` or consider making `../plugin.js` a package.', | ||
line: 1, | ||
column: 8, | ||
} ], | ||
}), | ||
], | ||
}) |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters