No relative parent imports rule

[chrislloyd]

no-relative-parent-imports

Use this rule to prevent imports to folds in relative parent paths.

It's useful for large codebases codebases to enforce directed-acyclic-graph like folder structures.

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:

/**
 *  in my-project/lib/a.js
 */

import bar from '../main'; // Import parent file using a relative path

The following patterns are NOT considered problems:

/**
 *  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

[chrislloyd]

As discussed IRL with @ljharb, this is a more extreme version of #966. This helps engineers write code that fits well with DAG based build tools like Bazel or Buck. The inspiration for this change comes from this essay by the Google Fuschia team and the OCAML module system.

This is my first commit to the project so please let me know if I've missed anything or I haven't followed norms.

[ljharb]

This looks great - can we add some tests for requires, and also, can we try to handle import()?

[ljharb]

what about import() calls?

[coveralls]

Coverage decreased (-0.2%) to 96.992% when pulling 2e05c3f on chrislloyd:no-relative-parent-imports into 8f668c7 on benmosher:master.

[ljharb]

LGTM!

Is there a way to use this rule to restrict parent imports out of a subdir (say, projects/*) - but allow parent imports up to that subdir? #966 doesn't seem to have a solution for this either.

[chrislloyd]

My general approach for that would be either a.) create a projects folder relative to the file ('./projects') or b.) create a projects package which is somehow available to the file. This rule tends to pair particularly well with yarn workspaces so that common code can be quickly and easily extracted into packages (which indicates explicit share-intent by the author).

Also, holy moly you're the fastest PR reviewer ever.

[chrislloyd]

Actually, re-reading your question - I think I misunderstood. Could an example of what you're asking for be:

project-foo/
    tests/
        a.test.js
    a.js
main.js

And you'd like to allow project-foo/tests/a.test.js to import ../a.js but disallow project-foo/a.js to import ../main.js?

[ljharb]

@chrislloyd yes, precisely. project-foo should be a jail, but everything inside it can import anything inside it.

[chrislloyd]

I think you could achieve that with overrides?

  "overrides": [
    {
      "files": [ "./project-foo/*.js" ],
      "excludedFiles": "./project-foo/tests/*.test.js",
      "rules": {
        "import/no-relative-parent-imports": "error"
      }
    }
  ]

[ljharb]

Wouldn't that allow the test file to import from ../../../, though?

[chrislloyd]

🤔 yep you're right. I think this rule could definitely take options to override some of the behavior but a.) I'd like to understand the use-cases a bit better. The way I intend to use this, exceptions won't be a problem. b.) They can be introduced in further PR's as they'd be non-breaking changes to the rule.

[ljharb]

I totally agree that such options can be added later, but I'd like to get an idea of what the schema might look like before merging the rule, to ensure future compatibility :-)

[chrislloyd]

Ok! Are you looking to perhaps exclude certain folder types and also use no-restricted-paths to "gaol" the whole project?

{
  "rules": {
    "import/no-relative-parent-imports": [2, { "exclude": "*/tests/*.js" }],
    "import/no-restricted-paths": [2, { "zones": [ { "target": ".", "from": "./project-foo" } ] }]
  }
}

[ljharb]

Hmm - it's more like i want to have:
"import/no-escaping-imports": [2, { "jails": ["project-*/", "packages/*/"] }]

Maybe I don't understand the use case of never wanting to do a parent import from anywhere. Certainly tests always have to do a parent import, unless the test file always lives as a direct sibling of the implementation (which i think even the pattern of colocating tests doesn't provide) - but in general, having a dir with a "helpers" directory, and having all sibling folders of "helpers" able to import "../helpers/blah", is a common pattern.

[chrislloyd]

Ah great, yeah. I think in those cases I'd like to see a layout like:

helpers/
  blah.js
foo.js
foo.test.js

That effectively shifts files further up and creates lots of big directories with lots of files in it. The rule is intended to be opinionated. I think what you're asking for could be achieved perhaps with more flexible configuration to no-restrictive-paths?

"import/no-restricted-paths": [2, { "jails": ["project-*/", "packages/*/"] }]

[ljharb]

Yes, that’s possible.

Maybe since the rule is intended to be opinionated, we should really flesh out the docs so the intention and reasoning is clear?

[benmosher]

I'd like to see this use utils/moduleVisitor instead of isStaticImport, etc., and enhance utils/moduleVisitor to visit dynamic imports (import()).

See the no-unresolved rule for an example. Enhancing moduleVisitor to support dynamic import() means every rule using it will test import() as well!

[chrislloyd]

Awesome, thanks for the reviews @ljharb & @benmosher. Addressed your feedback in followup commits.

[chrislloyd]

The CI failure above seems to be because of a flakey test environment (passing on all other environments, randomly timing out) - could ya'll restart the job? I don't think I have permissions in Travis without being a contributor to the repo.

[chrislloyd]

@benmosher review ping!

[benmosher]

looks great! thanks!

[ljharb]

Released in v2.13.0

[Alex-Sokolov]

Thanks for adding rule!

But found a bug:
import blockIcons from '../../components/test/test.js' — FAIL
but
import blockIcons from './../../components/test/test.js' — OK

[ljharb]

@Alex-Sokolov please file a new issue if you’ve found a bug