Nx: Allow to disable deep imports check of `nx-enforce-module-boundaries` tslint rule (or remove this check completely)

Created on 21 Jan 2019  路  17Comments  路  Source: nrwl/nx

Expected Behavior

We'd like to have the possibility to disable some of the checks made by nx-enforce-module-boundaries tslint rule. In particular, we'd like to disable deep imports check.

Current Behavior

We can either enable all checks or disable all of them. There's nothing in between.

Context

Use cases

There are two use cases that we'd like to be supported.

Migration to Nx Workspace

We have a quite large Angular codebase build on standard Angular CLI approach and we are migrating it to Nx. Since there are lot of people working on the codebase, we are performing this process on a step by step basis (and we expect it to take quite some time). To make the migration smoother and less intrusive for other teams, we need to allow deep imports into libs during the transition period. At the moment it's not possible without disabling nx-enforce-module-boundaries. This is not an option for us, as we'd like to benefit from other checks made by this rule (like dependency constraints checking).

Secondary lib entry points

We'd like to have the possibility to add secondary entry points for some libs (e.g. it should be possible for some libs to also expose testing.ts entry point). Again, it's not possible without disabling nx-enforce-module-boundaries because of deep library imports check.

Solution one: split the nx-enforce-module-boundaries to multiple rules

The rule is making many checks at once and there's no possibility to disable only some of them. It would be different if boundaries checking would be implemented as a set of independent tslint rules. In that case there would be an option to disable each of them individually.

Solution two: remove the deep import check completely

I might be missing something, but is this check really needed?
Given that the path mapping for a lib in tsconfig.json looks like this:

"@workspace/lib-name": [
    "libs/lib-name/src/index.ts"
],

there's no option to use deep imports since there's no path mapping allowing it (code will simply not compile). On the other hand, if the path mapping for, say, secondary entry point would be added to tsconfig.json like this:

"@workspace/lib-name": [
    "libs/lib-name/src/index.ts"
],
"@workspace/lib-name/testing": [
    "libs/lib-name/src/testing.ts"
],

any attempt of using it will be blocked by the deep imports check, despite the fact that it was a conscious decision to expose this entry point.

Other

We are willing to contribute if you find this request reasonable.

feature

Most helpful comment

I agree that we should make the change transparent to legacy users and leave existing nx-enforce-module-boundaries rule as it is.

I've been looking into how we could split the existing rule. the solution seems to be pretty much straightforward and it follows directly from how the nx-enforce-module-boundaries code is structured right now. This is my proposition:

  1. nx-no-circular-deps - prevents circular dependencies between projects,
  2. nx-dep-graph-constraints - enforces imposed dependency constraints,
  3. nx-no-app-imports - prevents importing apps,
  4. nx-no-unscoped-lib-imports (or nx-no-non-entry-point-lib-imports, or nx-workspace-imports) - prevents importing libs using relative and absolute paths,
  5. nx-no-lazy-loaded-lib-imports - prevents importing a lib using es6 import when the lib is lazy loaded
  6. nx-no-deep-lib-imports - prevents deep imports into a lib; note that this one would not be needed for projects using new path mappings.

For each of those rules we can also add the allow option, which would work same as the allow option of the nx-enforce-module-boundaries rule.

What do you guys think about it? Maybe we could come up with better rule names?

@FrozenPandaz If this proposal looks fine to you, I can start working on it in the next week.

All 17 comments

I agree, with the new path mappings, it is not necessary to have this rule anymore.

For users which have been using Nx since v1, it's possible they still have legacy code which are using deep imports.

I think Solution 1 sounds reasonable and would be more transparent for what exactly is being checked. Perhaps for backwards-compatibility, we can keep the old rule?

+1 for allowing multiple lib entry points.

We have a few libs that contain ngrx feature stores and selectors. Mocking up the store is tedious, so we'd like to export test-only functions out of our libs to help set things up.

I want that import to look like:

import { ... } from '@workspace/lib-name/testing'

We were able to get that done by adding an entry in the root tsconfig.json path config and adding that testing 'lib' (path, really) to the root tslint nx-enforce-module-boundaries's 'allow' parameter. Having this as a first-class supported concept would be great.

I agree that we should make the change transparent to legacy users and leave existing nx-enforce-module-boundaries rule as it is.

I've been looking into how we could split the existing rule. the solution seems to be pretty much straightforward and it follows directly from how the nx-enforce-module-boundaries code is structured right now. This is my proposition:

  1. nx-no-circular-deps - prevents circular dependencies between projects,
  2. nx-dep-graph-constraints - enforces imposed dependency constraints,
  3. nx-no-app-imports - prevents importing apps,
  4. nx-no-unscoped-lib-imports (or nx-no-non-entry-point-lib-imports, or nx-workspace-imports) - prevents importing libs using relative and absolute paths,
  5. nx-no-lazy-loaded-lib-imports - prevents importing a lib using es6 import when the lib is lazy loaded
  6. nx-no-deep-lib-imports - prevents deep imports into a lib; note that this one would not be needed for projects using new path mappings.

For each of those rules we can also add the allow option, which would work same as the allow option of the nx-enforce-module-boundaries rule.

What do you guys think about it? Maybe we could come up with better rule names?

@FrozenPandaz If this proposal looks fine to you, I can start working on it in the next week.

Just got hit by this as well ):

I would love to see this. I am porting our codebase to Nx right now and while I love the scoped checking rules, the deep lib imports is causing problems with gradually porting code over.

I'm really sorry for the late response.

Are you really interested in deep imports or in multiple entry points?

I can see a clear use case for multiple entry points (testing, non-testing), but I'm not sure what would be a good use case for arbitrary deep imports. When would you want it?

When would you want to disable the checks?

  • nx-no-circular-deps and nx-no-app-import are similar. When would you want to disable these?
  • nx-no-unscoped-lib-imports When would you want to disable this?
  • nx-no-lazy-loaded-lib-imports When would you want to disable this?
  • nx-no-deep-lib-import When would you want to disable this?

I get it. Sometimes, especially when migrating into Nx, you have a couple of situations where you violate the lint rule, and you can list them in allow. I wonder if you would even want to disable a check (say nx-no-lazy-loaded-lib-imports) altogether.

@vsavkin I can't speak for everyone else on this issue, but what I would like to do is imports similar to Angular material and a few other libraries we use. Basically allow one level deep imports to pass the lint rules.

So imagine I have a library setup like this:

libs/common/src/lib
  - dialogs
    - index.ts
  - map
     - index.ts
  - services
     - index.ts
  - util
     - index.ts

I would like to setup the library so instead of having everything exposed through one entry point at the base level of the library we can instead have one level of nesting to group related items. This would support imports like:

import { ... } from '@corp/common/map';
import { ... } from '@corp/common/dialogs';

For our team we find this makes it easier to see clearly where various piece of code are coming from and allow sub-teams to manage and determine exposed APIs for sub-systems within their own sub directory.

That's multiple entry points and I'd love to have that as well. (:

So... what's the update on this? Because I'm currently not using any Nx rules because the circular dependency errors can't be swallowed.

@abierbaum I think it's a meaningful use case. We should support it. At the same time, I don't want to allow arbitrary deep imports. Would extending the "allow" to support wildcards be sufficient?

So you could say something like that @myorg/common/*?

I'm actually using that pattern on tsconfig.json in path aliases. (we currently have this linting rule disabled so that we can have 1 level deep imports)

@vsavkin I think that would cover our case well. Thanks for thinking through support for this.

+1

Awesome. I'll make the change so it lands in the next release.

I haven't test that, but I think that the implemented solution is not sufficient. Consider the following scenario (possible even without the wildcards change).

Let's say I have 3 libraries:

  • @workspace/foo, tags: foo,
  • @workspace/bar, tags: bar,
  • @workspace/baz, tags: baz,

and that I have boundaries configured like this:

{
    "allow": [],
    "depConstraints": [
        {
            "sourceTag": "foo",
            "onlyDependOnLibsWithTags": ["baz"]
        },
    ]
}

In this setup it's not possible to import @workspace/baz from @workspace/bar, but it's possible to import it from @workspace/foo.

Let's now say that I want to add a secondary entry point to @workspace/baz, for example @workspace/baz/testing.
The recommended way to do this is to configure the boundaries config to look like this:

{
    "allow": ["@workspace/baz/testing"],
    "depConstraints": [
        {
            "sourceTag": "foo",
            "onlyDependOnLibsWithTags": ["baz"]
        },
    ]
}

The result is that now I can import @workspace/baz/testing from @workspace/bar.
Since my primary intent was to add a secondary entry point for @workspace/baz and not to allow @workspace/baz/testing imports everywhere,
I'd like the configured dependency constraints to apply so that it's only possible to import @workspace/baz/testing from @workspace/foo library.

We also want to allow deep imports via path, but still want to impose the tag-constraints (i.e. multiple entrypoints).
If I understood the current allow feature on nx-enforce-module-boundaries correctly, this is not yet possible: see the example in the comment above by @kamknitt

@vsavkin: Can you reopen this issue, or should we file a new feature-request?

note: also related to my comment in #1320

Was this page helpful?
0 / 5 - 0 ratings

Related issues

joelmuskwe picture joelmuskwe  路  3Comments

vimalraj-a picture vimalraj-a  路  3Comments

zpydee picture zpydee  路  3Comments

Svancara picture Svancara  路  3Comments

MichaelWarneke picture MichaelWarneke  路  3Comments