Eslint-plugin-import: Add new rule or replace no-restricted-paths

[Bessonov]

I need more sophisticated rules to restrict the import paths. For example, I have following package structure:

src/common/api/
src/auth/api/
src/auth/core/
src/auth/entities/
src/auth/ui/
src/another/entities/
src/index.tsx

My rules are:

• auth/api can import from auth/entities and common/api only
• auth/core can import from auth/api and auth/entities only
• */entities can't import anything, except other entities
• auth/ui can import from auth/core only
• index.tsx can import anything, except */api/*

I can imagine following rules:

import/no-restricted-paths-extended:
  - error
  - basePath: 'src'
    zones:
      - target: [ 'auth/api/**' ]
        from:
          - path: [ 'auth/entities/**', 'common/api/**' ]
            rule: 'allow'
          - path: [ '**' ]
            rule: 'block'
      - target: [ 'auth/core/**' ]
        from:
          - path: [ 'auth/api/**', 'auth/entities/**' ]
            rule: 'allow'
          - path: [ '**' ]
            rule: 'block'
      - target: [ '**/entities/**' ]
        from:
          - path: [ '**/entities/**' ]
            rule: 'allow'
          - path: [ '**' ]
            rule: 'block'
      - target: [ 'auth/ui/**' ]
        from:
          - path: [ 'auth/core/**' ]
            rule: 'allow'
          - path: [ '**' ]
            rule: 'block'
      - target: [ 'index.tsx' ]
        from:
          - path: [ '**/api/**' ]
            rule: 'block'

Some considerations:

1. allow and block rules can be used multiple times. First match wins.
2. Use regexp instead of globing
3. Allow pass regexp match to rules to describe "every ui module can access core module in same bundle":

      - target: [ '(P<module>[^/]+)/ui/.*' ]
        from:
          - path: [ '<module>/core/.*' ]
            rule: 'allow'
          - path: [ '**' ]
            rule: 'block'

Any thoughts on this? Would a pull request desirable?

Maybe relevant: https://github.com/benmosher/eslint-plugin-import/issues/834

[ljharb]

Since you can override rules by glob path, or with nested eslintrcs, I'm not sure this is needed. Have you tried that?

[Bessonov]

I think I can achieve that with nested eslintrcs and decentralized approach is good for some use cases. But it introduce a lot of management complexity to add (easy to forget), maintain and change rules. From my point of view, nested eslintrcs are good for overriding some rules or settings, but not to enforce structure like described above.

For us, it's better do disallow internal access by default and allow only desired layers, modules and/or individual files.

While described approach is very powerful and used in the wild (spring security, for example), it maintain a reasonable level of complexity. Again, from my perspective.

[ljharb]

If you don't want nested eslintrcs, you can use glob-based configuration to centralize the configuration in one file.

[Bessonov]

@ljharb

Thanks, you are right about glob-based configuration!

But I'm struggling to get it right. Because there is only "block" and no allow rule, it can be very complex.

For testing I have rule:

  overrides:
    - files:
        - 'src/**/api/**'
      rules:
        import/no-restricted-paths:
          - error
          - zones:
              - target: '.*'
                from: '.*/(?:(?!api)|(?!xxx))/.*'

I'm expecting, that all api imports from api are allowed and others not. I use xxx instead of entities to check that rule is applied correctly. What I get is:

| is OK? | file | import | rule |
true /home/developer/frontend/src/test/api/Configuration.ts /home/developer/frontend/node_modules/apollo-cache-inmemory/lib/index.js /home/developer/frontend/.*/(?:(?!api)|(?!xxx))/.*
true /home/developer/frontend/src/test/api/Configuration.ts /home/developer/frontend/node_modules/apollo-client/index.js /home/developer/frontend/.*/(?:(?!api)|(?!xxx))/.*
true /home/developer/frontend/src/test/api/Configuration.ts /home/developer/frontend/node_modules/apollo-link-rest/bundle.umd.js /home/developer/frontend/.*/(?:(?!api)|(?!xxx))/.*
true /home/developer/frontend/src/test/api/network/NetworkApi.ts /home/developer/frontend/node_modules/graphql-tag/src/index.js /home/developer/frontend/.*/(?:(?!api)|(?!xxx))/.*
true /home/developer/frontend/src/test/api/network/NetworkApi.ts /home/developer/frontend/src/test/api/Configuration.ts /home/developer/frontend/.*/(?:(?!api)|(?!xxx))/.*
true /home/developer/frontend/src/test/api/network/NetworkApi.ts /home/developer/frontend/src/test/entities/network/Group.ts /home/developer/frontend/.*/(?:(?!api)|(?!xxx))/.*
true /home/developer/frontend/src/test/api/auth/AuthApi.ts /home/developer/frontend/src/test/api/Configuration.ts /home/developer/frontend/.*/(?:(?!api)|(?!xxx))/.*
true /home/developer/frontend/src/test/api/auth/AuthApi.ts /home/developer/frontend/src/test/entities/auth/UserDetails.ts /home/developer/frontend/.*/(?:(?!api)|(?!xxx))/.*

Is there any example how to get it right? Here I get just opposite (click on "switch to unit test and then run).

Overall, I think to define a bunch of rules with block or allow actions is easier and maintainable.

Another thing is, that used contains-path introduces a breaking change and doesn't support regexp anymore:

As of v1.0.0, this library no longer uses regex for matching.

Currently, eslint-plugin-import uses ^0.1.0.

[ljharb]

^0.1.0 doesn't include v1, so that's not really an issue.

[Bessonov]

I know, but that's not the point. The point is, that no-restricted-paths uses old dependency and new version introduces breaking change.

[ljharb]

That’s fine, we can keep using the old one forever. There’s no reason that deps have to be updated, if they work.

[Bessonov]

I don't think that keep old version forever is a good approach for development. Especially, if I look inside of contains-path.

Anyway, I've created sophisticated rule to make above things possible and tested it against my requirements. I don't published it now. Following outcome I have:

1. target is now string and not array
2. target and path are described as regular expressions
3. Named groups are not supported well in JS. Therefore I use counted groups. Real example (no pseudo code):

          - target: (.*)/api/.*
            from:
              - path:
                  - M<1>/entities/.*
                  - M<1>/api/.*
                  - /common/api/.*
                action: allow
              - path: [ .* ]
                action: block

where M<1> would be replaced with first match in target and in this case it means "allow import from api and entities from same module or api from common module".

1. First match of target and path use specified action. Remaining rules are ignored. It's really powerful, because you can combine allow and block at very fine granular level and in desired order. You can even implement whitelisting (block everything except x, y, z) and blacklisting (allow everything except x, y, z).

@ljharb is there any interest for pull request? If (hopefully :D) yes, then how I should name the rule? currently I named it no-restricted-zones.

[ljharb]

It’s a fine approach; there’s nothing inherently valuable about upgrading abstractions.

I still am not convinced that a separate rule is warranted here. How is glob overrides in your root eslintrc insufficient?

[Bessonov]

Globs are working on its own. But to define desired rules leads to very complex regular expressions and in the end, I don't get it working with no-restricted-paths.

Therefore, my proposal, which avoids complex regular expressions and introduce new features.

It must not be a separate rule. Sure, no-restricted-paths can be updated. But it introduces a breaking change. Fine for me. Do you mean I should prepare PR for no-restricted-paths?

[ljharb]

I’m not even sure this use case should be supported, to be honest - why are your restrictions so complex? How can your developers hope to understand the rationale, even if the linter enforces it?

[Bessonov]

We create an app with distinct, swappable, layers in each module. Therefore, it's important to ensure access/import pattern of these layers.

[ljharb]

If they’re distinct, shouldn’t they be separate packages (or in a monorepo, separate top-level directories)?

[Bessonov]

Distinct in sense we have api layer, ui layer and so on in most of modules. This structure was inspired by Bundles from Symfony. But overall, this is still one app. Separate packages would help here, but introduce complexity on the other end.