[import/no-internal-modules] path normalisation makes preventing package internal imports impossible

[gordonmleigh]

It feels like the this rule ought to be support my use case of preventing an import from deep inside any package, e.g. somepackage/some/internal/path. There doesn't appear to me to be any way to achieve that, due to the normalisation that takes place.

So I set an "allow" config for "allow": ["./**"], so that reaching is prevented except for relative paths; however, the normalisation that happens inside the rule scrubs all . and .. segments away for some reason. There doesn't therefore be any possibility of distinguishing between relative and package imports.

How is this use case supposed to be achieved?

[ljharb]

Why would you want to prevent that tho? Deep imports are the objectively superior way to get at subcomponents of a package; they obviate the need for treeshaking and result in much smaller bundle sizes and programs.

[gordonmleigh]

Cool, thank you for sharing your subjective opinion with me.

I need it because VS code auto-import messes up and imports stuff from deep inside packages sometimes, especially in monorepos, and I'd like to enforce that it shouldn't happen—a lot (most?) of NPM packages are written with barrel export files and the implicit assumption that anything not in the barrel is internal and therefore subject to change.

Typescript is also going through a phase (in the process of being fixed) of preferring to select auto-imports from a barrel file in the current package, meaning you end up with imports like from '../../..', which I'd like to prevent.

[ljharb]

It’s not subjective; it’s objective. Anything that’s deep importable should always be deep imported. Barrel files are an antipattern.

[gordonmleigh]

Opinions are subjective by definition. You don't define "deep importable" here, but if you're suggesting that a package consumer should expect to be able to import any file they please from deep inside a package, and therefore package maintainers should make guarantees (via version numbering, say) about the internals of a package, then I don't agree at all. I doubt that many package maintainers are bumping major versions when they make these kind of changes, which they would have to if the file structure formed part of the package contract.

I can't find anything setting out why barrel files are an anti-pattern as you suggest, and you don't provide any sources or clue to your thinking. My observation is that they are pretty common in the ecosystem.

[IAmNatch]

+1 on this issue.

I have an example that might help.

I have a monorepo with a structure like this:

/common
/domains
   /domain1
      index.ts
      other.ts
   /domain2
      index.ts
      other.ts

I'd like to be able to ensure that each domain only imports from the exported interface of each domain, not from deep imports.

For example import {x} from "domain1" should be allowed, whereas import {x} from domain1/other should not be allowed. This is valuable as it helps enforce clear boundaries and interfaces between each domain, and will make extraction easier in the future if any of these domains are extracted into their own codebase or package.

Following a normal minimatch pattern **/domains/*/** should allow for exactly the pattern outlined above. However, due to normalization, import {x} from "domain1" is disallowed. Furthermore, even relative imports within the same domain, for example `import {x} from './other.ts' is disallowed as well.

This seems like a common use case, and it would be great to provide an option to disable this normalization step to support this. If this is not your intention for this rule, would you be open to a PR for a new rule that supports this?

[xenoterracide]

because, the first is an external library that should not be used like that, and the latter is a lot of code that I want warnings for while we fix instead of 300 changes...

  11:30  error    Reaching to "xstate/lib/types" is not allowed                                                import/no-internal-modules
  14:31  error    Reaching to "./workflows/create-action" is not allowed                                       import/no-internal-modules

[ljharb]

@IAmNatch it's a common use case, but it's still always a bad idea.

[xenoterracide]

perhaps a better solution, at least in my case would be to have an [import/no-internal-from-external-modules] rule... which could potentially use the rule that defines what's internal

[xenoterracide]

bad, no argument, but progressive migration ftw.

[Lonli-Lokli]

My workflow is to not allow imports too far from current file, eg import { smth} from '../../../ui/src/package';

Usually it means that you are trying to import file which is outside of boundaries in your project. Usually it happens due to VSCode/WebStorm imports.

Unfortunately, the following code does not work

"import/no-internal-modules": [
         "error",
         {
           "forbid": ["../../../**/*"]
         }
       ],

[Lonli-Lokli]

Finally I solved my issue with another rule

"no-restricted-imports": [
          "error", 
            {
              "patterns": [
                "../../../*",
                "libs/**/*"
              ]
            }
        ]

[weiwei-lin]

@IAmNatch it's a common use case, but it's still always a bad idea.

The name of the rule is

no-internal-modules

And the rule description says

Use this rule to prevent importing the submodules of other modules.

Doesn't that mean this rule is literally for preventing deep imports from other modules? If preventing deep imports is such a bad idea with no good use case what-so-ever, then the rule shouldn't have existed in the first place.

In the current form, the path normalization makes the rule effectively

Use this rule to prevent importing the ~~submodules of other~~ modules.

[weiwei-lin]

For those who are interested, here's a workaround for the problem.

Note that this will make your eslint significantly slower in large projects.

// The internal files of a package (any folder with an index.ts?(x) file)
// shall not be directly imported by other packages.
// This helps reducing the surface of a package.
'import/no-restricted-paths': [
  'error',
  {
    basePath: './src',
    zones: glob
      .globSync(['**/index.ts', '**/index.tsx'], {
        cwd: 'src',
        withFileTypes: true,
      })
      .map((p) => {
        const parent = p.parent.relative();
        const parentSegs = parent.split('/');

        // The following code is basically trying to construct a pattern
        // that is equivalent to `!${parent}/**/*`. Unfortunately, we cannot
        // use negated patterns. The plugin will try to resolve the pattern
        // like a path, making it invalid (e.g. `!dir/**` will become
        // `home/user/src/!dir/**`).

        // Nothing in the inner most directory shall be matched.
        let pattern = '{}';
        while (parentSegs.length !== 0) {
          const seg = parentSegs.pop();

          // Matches
          // 1. anything under `${seg}/` that matches the previous pattern,
          // 2. anything not under `${seg}`,
          // 3. anything not under a directory.
          //
          // Effectively, in each iteration, we expand the pattern to
          // include siblings in the parent directory.
          pattern = `{${seg}/${pattern},!(${seg})/**,*}`;
        }

        return {
          target: pattern,
          from: parent,
          except: [p.name],
        };
      }),
  },
],