Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[New] no-parent-barrel-import: Forbid import from parent barrel file #2779

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions CHANGELOG.md
Expand Up @@ -6,6 +6,10 @@ This change log adheres to standards from [Keep a CHANGELOG](https://keepachange

## [Unreleased]

### Added

- Add [`no-parent-barrel-import`] rule: forbids a module from importing from parent barrel file

### Fixed
- [`no-duplicates`]: remove duplicate identifiers in duplicate imports ([#2577], thanks [@joe-matsec])
- [`consistent-type-specifier-style`]: fix accidental removal of comma in certain cases ([#2754], thanks [@bradzacher])
Expand Down Expand Up @@ -1057,6 +1061,7 @@ for info on changes for earlier releases.
[`no-relative-packages`]: ./docs/rules/no-relative-packages.md
[`no-relative-parent-imports`]: ./docs/rules/no-relative-parent-imports.md
[`no-restricted-paths`]: ./docs/rules/no-restricted-paths.md
[`no-parent-barrel-import`]: ./docs/rules/no-parent-barrel-import.md
[`no-self-import`]: ./docs/rules/no-self-import.md
[`no-unassigned-import`]: ./docs/rules/no-unassigned-import.md
[`no-unresolved`]: ./docs/rules/no-unresolved.md
Expand Down
1 change: 1 addition & 0 deletions README.md
Expand Up @@ -60,6 +60,7 @@ This plugin intends to support linting of ES2015+ (ES6+) import/export syntax, a
| [no-cycle](docs/rules/no-cycle.md) | Forbid a module from importing a module with a dependency path back to itself. | | | | | | |
| [no-dynamic-require](docs/rules/no-dynamic-require.md) | Forbid `require()` calls with expressions. | | | | | | |
| [no-internal-modules](docs/rules/no-internal-modules.md) | Forbid importing the submodules of other modules. | | | | | | |
| [no-parent-barrel-import](docs/rules/no-parent-barrel-import.md) | Forbid a module from importing from parent barrel file. | | | | | | |
| [no-relative-packages](docs/rules/no-relative-packages.md) | Forbid importing packages through relative paths. | | | | 🔧 | | |
| [no-relative-parent-imports](docs/rules/no-relative-parent-imports.md) | Forbid importing modules from parent directories. | | | | | | |
| [no-restricted-paths](docs/rules/no-restricted-paths.md) | Enforce which files can be imported in a given folder. | | | | | | |
Expand Down
63 changes: 63 additions & 0 deletions docs/rules/no-parent-barrel-import.md
@@ -0,0 +1,63 @@
# import/no-parent-barrel-import

<!-- end auto-generated rule header -->

Forbid a module from importing from parent barrel file, as it often leads to runtime error `Cannot read ... of undefined`.

It resolves the missing circular import check from [`no-self-import`], while being computationally cheap (see [`no-cycle`]).

## Rule Details

### Fail

```js
// @foo/index.ts
export * from "./bar";

// @foo/bar/index.ts
export * from "./baz";
export * from "./qux";

// @foo/bar/baz.ts (cannot read property `X` of undefined)
import { T } from '../..'; // relative
import { T } from '..'; // relative
import { T } from '@foo'; // absolute
import { T } from '@foo/bar'; // absolute

export const X = T.X;

// @foo/bar/qux.ts
export enum T {
X = "..."
}
```

### Pass

```js
// @foo/index.ts
export * from "./bar";

// @foo/bar/index.ts
export * from "./baz";
export * from "./qux";

// @foo/bar/baz.ts (relative import for code in `@foo`)
import { T } from "./baz"; // relative
import { T } from "@foo/bar/qux"; // absolute

export const X = T.X;

// @foo/bar/qux.ts
export enum T {
X = "..."
}
```

## Further Reading

- [Related Discussion](https://github.com/import-js/eslint-plugin-import/pull/2318#issuecomment-1027807460)
- Rule to detect that module imports itself: [`no-self-import`], [`no-cycle`]

[`no-self-import`]: ./no-self-import.md
[`no-cycle`]: ./no-cycle.md
1 change: 1 addition & 0 deletions src/index.js
Expand Up @@ -15,6 +15,7 @@ export const rules = {
'consistent-type-specifier-style': require('./rules/consistent-type-specifier-style'),

'no-self-import': require('./rules/no-self-import'),
'no-parent-barrel-import': require('./rules/no-parent-barrel-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'),
Expand Down
45 changes: 45 additions & 0 deletions src/rules/no-parent-barrel-import.js
@@ -0,0 +1,45 @@
/**
* @fileOverview Forbids a module from importing from parent barrel file
* @author jonioni
*/

const { parse } = require('path');
const resolve = require('eslint-module-utils/resolve').default;
const moduleVisitor = require('eslint-module-utils/moduleVisitor').default;
const docsUrl = require('../docsUrl').default;

function isImportingFromParentBarrel(context, node, requireName) {
let filePath;
if (context.getPhysicalFilename) {
filePath = context.getPhysicalFilename();
} else if (context.getFilename) {
filePath = context.getFilename();
}
const importPath = resolve(requireName, context);
console.info(`File Path: ${filePath} and ${importPath}`);
const importDetails = parse(importPath);
if (importDetails.name === 'index' && filePath.startsWith(importDetails.dir)) {
context.report({
node,
message: 'Module imports from parent barrel file.',
});
}
}

module.exports = {
meta: {
type: 'problem',
docs: {
category: 'Static analysis',
description: 'Forbid a module from importing from parent barrel file.',
recommended: true,
url: docsUrl('no-parent-barrel-import'),
},
schema: [],
},
create(context) {
return isImportingFromParentBarrel((source, node) => {
moduleVisitor(context, node, source.value);
});
},
};
1 change: 1 addition & 0 deletions tests/files/no-parent-barrel-import/foo/bar.js
@@ -0,0 +1 @@
// Used in `no-parent-barrel-import` tests
1 change: 1 addition & 0 deletions tests/files/no-parent-barrel-import/foo/baz.js
@@ -0,0 +1 @@
// Used in `no-parent-barrel-import` tests
1 change: 1 addition & 0 deletions tests/files/no-parent-barrel-import/foo/index.js
@@ -0,0 +1 @@
// Used in `no-parent-barrel-import` tests
2 changes: 2 additions & 0 deletions tests/files/no-parent-barrel-import/index.js
@@ -0,0 +1,2 @@
// Used in `no-parent-barrel-import` tests

70 changes: 70 additions & 0 deletions tests/src/rules/no-parent-barrel-import.js
@@ -0,0 +1,70 @@
import { test, testFilePath } from '../utils';

import { RuleTester } from 'eslint';

const ruleTester = new RuleTester();
const rule = require('rules/no-parent-barrel-import');

const error = {
message: 'Module imports from parent barrel file.',
};

ruleTester.run('no-parent-barrel-import', rule, {
valid: [
test({
code: 'import _ from "lodash"',
filename: testFilePath('./no-parent-barrel-import/index.js'),
}),
test({
code: 'import baz from "./baz"',
filename: testFilePath('./no-parent-barrel-import/foo/bar.js'),
}),
test({
code: 'import bar from "./bar"',
filename: testFilePath('./no-parent-barrel-import/foo/baz.js'),
}),
],
invalid: [
test({
code: 'import baz from "."',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/foo/bar.js'),
}),
test({
code: 'import baz from "../.."',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/foo/bar.js'),
}),
test({
code: 'var foo = require("./index.js")',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/foo/bar.js'),
}),
test({
code: 'var foo = require(".")',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/foo/baz.js'),
}),
test({
code: 'var foo = require("..")',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/foo/bar.js'),
}),
test({
code: 'var foo = require("././././")',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/foo/index.js'),
}),
test({
code: 'var root = require("../../..")',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/foo/index.js'),
}),
test({
code: 'var root = require("..")',
errors: [error],
filename: testFilePath('./no-parent-barrel-import/index.js'),
}),

],
});