Skip to content

Commit

Permalink
[New]: add no-relative-packages
Browse files Browse the repository at this point in the history
Use this rule to prevent importing packages through relative paths.

It's useful in Yarn/Lerna workspaces, were it's possible to import a sibling
package using `../package` relative path, while direct `package` is the correct one.
  • Loading branch information
panrafal authored and ljharb committed Nov 2, 2017
1 parent 0b81052 commit b934ed2
Show file tree
Hide file tree
Showing 9 changed files with 219 additions and 2 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
Expand Up @@ -12,6 +12,7 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
- [`order`]: Add support for TypeScript's "import equals"-expressions ([#1785], thanks [@manuth])
- [`import/default`]: support default export in TSExportAssignment ([#1689], thanks [@Maxim-Mazurok])
- [`no-restricted-paths`]: add custom message support ([#1802], thanks [@malykhinvi])
- add [`no-relative-packages`] ([#966], thanks [@panrafal])

### Fixed
- [`group-exports`]: Flow type export awareness ([#1702], thanks [@ernestostifano])
Expand Down Expand Up @@ -814,6 +815,7 @@ for info on changes for earlier releases.
[#1068]: https://github.com/benmosher/eslint-plugin-import/pull/1068
[#1049]: https://github.com/benmosher/eslint-plugin-import/pull/1049
[#1046]: https://github.com/benmosher/eslint-plugin-import/pull/1046
[#966]: https://github.com/benmosher/eslint-plugin-import/pull/966
[#944]: https://github.com/benmosher/eslint-plugin-import/pull/944
[#912]: https://github.com/benmosher/eslint-plugin-import/pull/912
[#908]: https://github.com/benmosher/eslint-plugin-import/pull/908
Expand Down Expand Up @@ -1195,3 +1197,4 @@ for info on changes for earlier releases.
[@adjerbetian]: https://github.com/adjerbetian
[@Maxim-Mazurok]: https://github.com/Maxim-Mazurok
[@malykhinvi]: https://github.com/malykhinvi
[@panrafal]: https://github.com/panrafal
66 changes: 66 additions & 0 deletions docs/rules/no-relative-packages.md
@@ -0,0 +1,66 @@
# no-relative-packages

Use this rule to prevent importing packages through relative paths.

It's useful in Yarn/Lerna workspaces, were it's possible to import a sibling
package using `../package` relative path, while direct `package` is the correct one.


### Examples

Given the following folder structure:

```
my-project
├── packages
│ ├── foo
│ │ ├── index.js
│ │ └── package.json
│ └── bar
│ ├── index.js
│ └── package.json
└── entry.js
```

And the .eslintrc file:
```
{
...
"rules": {
"import/no-relative-packages": "error"
}
}
```

The following patterns are considered problems:

```js
/**
* in my-project/packages/foo.js
*/

import bar from '../bar'; // Import sibling package using relative path
import entry from '../../entry.js'; // Import from parent package using relative path

/**
* in my-project/entry.js
*/

import bar from './packages/bar'; // Import child package using relative path
```

The following patterns are NOT considered problems:

```js
/**
* in my-project/packages/foo.js
*/

import bar from 'bar'; // Import sibling package using package name

/**
* in my-project/entry.js
*/

import bar from 'bar'; // Import sibling package using package name
```
3 changes: 1 addition & 2 deletions src/index.js
Expand Up @@ -10,16 +10,15 @@ export const rules = {
'no-restricted-paths': require('./rules/no-restricted-paths'),
'no-internal-modules': require('./rules/no-internal-modules'),
'group-exports': require('./rules/group-exports'),
'no-relative-packages': require('./rules/no-relative-packages'),
'no-relative-parent-imports': require('./rules/no-relative-parent-imports'),

'no-self-import': require('./rules/no-self-import'),
'no-cycle': require('./rules/no-cycle'),
'no-named-default': require('./rules/no-named-default'),
'no-named-as-default': require('./rules/no-named-as-default'),
'no-named-as-default-member': require('./rules/no-named-as-default-member'),
'no-anonymous-default-export': require('./rules/no-anonymous-default-export'),
'no-unused-modules': require('./rules/no-unused-modules'),

'no-commonjs': require('./rules/no-commonjs'),
'no-amd': require('./rules/no-amd'),
'no-duplicates': require('./rules/no-duplicates'),
Expand Down
72 changes: 72 additions & 0 deletions src/rules/no-relative-packages.js
@@ -0,0 +1,72 @@
import path from 'path'
import readPkgUp from 'read-pkg-up'

import resolve from 'eslint-module-utils/resolve'
import importType from '../core/importType'
import isStaticRequire from '../core/staticRequire'
import docsUrl from '../docsUrl'

function findNamedPackage(filePath) {
const found = readPkgUp.sync({cwd: filePath, normalize: false})
// console.log(found)
if (found.pkg && !found.pkg.name) {
return findNamedPackage(path.join(found.path, '../..'))
}
return found
}

function checkImportForRelativePackage(context, importPath, node) {
const potentialViolationTypes = ['parent', 'index', 'sibling']
if (potentialViolationTypes.indexOf(importType(importPath, context)) === -1) {
return
}

const resolvedImport = resolve(importPath, context)
const resolvedContext = context.getFilename()

if (!resolvedImport || !resolvedContext) {
return
}

const importPkg = findNamedPackage(resolvedImport)
const contextPkg = findNamedPackage(resolvedContext)

if (importPkg.pkg && contextPkg.pkg && importPkg.pkg.name !== contextPkg.pkg.name) {
const importBaseName = path.basename(importPath)
const importRoot = path.dirname(importPkg.path)
const properPath = path.relative(importRoot, resolvedImport)
const properImport = path.join(
importPkg.pkg.name,
path.dirname(properPath),
importBaseName === path.basename(importRoot) ? '' : importBaseName
)
context.report({
node,
message: `Relative import from another package is not allowed. Use "${properImport}" instead of "${importPath}"`,
})
}
}

module.exports = {
meta: {
type: 'suggestion',
docs: {
url: docsUrl('named'),
},
schema: [],
},

create: function noRelativePackages(context) {
return {
ImportDeclaration(node) {
checkImportForRelativePackage(context, node.source.value, node.source)
},
CallExpression(node) {
if (isStaticRequire(node)) {
const [firstArgument] = node.arguments
checkImportForRelativePackage(context, firstArgument.value, firstArgument)
}
},
}
},
}
1 change: 1 addition & 0 deletions tests/files/package-named/index.js
@@ -0,0 +1 @@
export default function () {}
5 changes: 5 additions & 0 deletions tests/files/package-named/package.json
@@ -0,0 +1,5 @@
{
"name": "package-named",
"description": "Standard, named package",
"main": "index.js"
}
1 change: 1 addition & 0 deletions tests/files/package/index.js
@@ -0,0 +1 @@
export default function () {}
4 changes: 4 additions & 0 deletions tests/files/package/package.json
@@ -0,0 +1,4 @@
{
"description": "Unnamed package for reaching through main field - rxjs style",
"main": "index.js"
}
66 changes: 66 additions & 0 deletions tests/src/rules/no-relative-packages.js
@@ -0,0 +1,66 @@
import { RuleTester } from 'eslint'
import rule from 'rules/no-relative-packages'

import { test, testFilePath } from '../utils'

const ruleTester = new RuleTester()

ruleTester.run('no-relative-packages', rule, {
valid: [
test({
code: 'import foo from "./index.js"',
filename: testFilePath('./package/index.js'),
}),
test({
code: 'import bar from "../bar"',
filename: testFilePath('./package/index.js'),
}),
test({
code: 'import {foo} from "a"',
filename: testFilePath('./package-named/index.js'),
}),
test({
code: 'const bar = require("../bar.js")',
filename: testFilePath('./package/index.js'),
}),
],

invalid: [
test({
code: 'import foo from "./package-named"',
filename: testFilePath('./bar.js'),
errors: [ {
message: 'Relative import from another package is not allowed. Use "package-named" instead of "./package-named"',
line: 1,
column: 17,
} ],
}),
test({
code: 'import foo from "../package-named"',
filename: testFilePath('./package/index.js'),
errors: [ {
message: 'Relative import from another package is not allowed. Use "package-named" instead of "../package-named"',
line: 1,
column: 17,
} ],
}),
test({
code: 'import foo from "../package-scoped"',
filename: testFilePath('./package/index.js'),
errors: [ {
message: 'Relative import from another package is not allowed. Use "@scope/package-named" instead of "../package-scoped"',
line: 1,
column: 17,
} ],
}),
test({
code: 'import bar from "../bar"',
filename: testFilePath('./package-named/index.js'),
errors: [ {
message: 'Relative import from another package is not allowed. Use "eslint-plugin-import/tests/files/bar" instead of "../bar"',
line: 1,
column: 17,
} ],
}),
],
})

0 comments on commit b934ed2

Please sign in to comment.