Tslint: Rule suggestion: disallow relative path imports outside the current package in a monorepo

[tonyxiao]

We're using lerna for managing a typescript mono-repo and vscode for editing. Often times vscode will auto-import a type from a sibling package via import something from '../package2' rather than import something from 'package2'. This bypasses tslint's no-implicit-dependencies rule and is definitely not the intended way to import code from a mono-repo. Does tslint have ways to guard against that?

[ajafff]

I don't know of any existing TSLint rules for monorepos.

I have written such a rule for internal use within my linter (monorepo) project: https://github.com/fimbullinter/wotan/blob/master/packages/disir/src/rules/import-package.ts
This rule currently resolves the import and checks if .../packages/<name>/... is the same for the importing and the imported file.
I also have a rule to prevent importing from package1 from files inside package1 (though that's already covered by no-implicit-dependencies) with an autofixer to import from the declaring module instead.

Caveats:
It doesn't work with TSLint, it's intended for the use in the Wotan linter runtime.
It's currently not configurable, you can see for example that the scope @fimbul/ is hard coded in the rule.
The directory structure probably also need to be configurable to be really useful.
Wotan currently has no VSCode plugin. You will therefore not see these green squiggles while you type. Instead you need to execute it from the command line.

So if you are OK with using another linter (in addition), please open an issue in my repo and tell a bit about the directory structure in your monorepo. I will then refactor this internal rule and publish it.

Otherwise you can use the code of my rule and port it to your own custom rule for TSLint.

[johnwiseheart]

Hi there,

It would appear that this issue is not directly related to a bug or feature request. We request that you use StackOverflow to ask questions about tslint, and tslint rules. Going to close this issue, but please feel free to re-open if you think this is a bug.

Thanks,
TSLint Team

[dgreene1]

@tonyxiao I'd be very interested to see if you found a solution for this. My team and I are also using a monorepo in this way that you've suggested. So automatically bringing in includes from the adjacent repos should be a warning at least.

[adidahiya]

I think this is a valid rule request, re-opening and adjusting the title

[JoshuaKGoldberg]

Possible overlap with #3980?

[dgreene1]

@JoshuaKGoldberg I don't think it's an overlap. More importantly, the new rule (https://github.com/palantir/tslint/pull/4318) that I made is designed to be more declarative about which folders in the monorepo are actually workspaces.

@tonyxiao please feel free to review that PR so you can let me know if it works for your needs. However, I don't believe that it is exactly what you need because (I believe) you're asking to not let developers include the path of the other workspaces. For that scenario, I bet that https://github.com/palantir/tslint/pull/3504 is going to be the correct solution since you can just put a regex in that puts your workspace folders on a blacklist. But please not that importBlacklistRule still allows nested folders, which is where my new rule has lots of value because you can prevent the following problem:

Let's say we have a section of the code that abstracts away the database access:

// in file "src/workspaces/businessLayerWorkspace/mongoHelpers.ts"
export function getSomethingFromTheDatabase()

But you don't want the UI to be monkeying around with Mongo. Wouldn't it be great if the import below fails? Well it will with PR https://github.com/palantir/tslint/pull/4318 :)

// in file "src/workspaces/userInterfaceWorkspace/uiControllers.ts"
import * as mongoHelpers from "src/workspaces/businessLayerWorkspace/mongoHelpers.ts"
export function getSomethingFromTheDatabase()

And the best part of the new noWorkspacePollutionRule is that it doesn't require Lerna or any monorepo setup at all. You can start putting fences around your various concerns without too much setup.

[JoshuaKGoldberg]

Re-posting from #4318: There hasn't been a solid proposal for exactly how to solve the issue. We'd need to see a few areas fleshed out in the issue's discussions, such as:

• What are all the common monorepo managers (Lerna, Rush, ...?)?
• How is this different from good-fences or wotan's import-package, and should it be?

It seems like this rule could go a lot of different ways, and before TSLint itself commits to a single interpretation of how to fix the issue, there should be some consensus from the community.

[greenecreations]

Yes @JoshuaKGoldberg, I think a comparison would be useful. I have been following the various threads related that are trying to solve separation of concerns and I think that this is pretty good comparative analysis:

I'll now detail how I arrived at the above conclusions:

"prevents importing a nested module from another workspace relatively."

• This is the critical path feature that I was personally looking for. I'm more concerned with the separation of concerns being enforced than I am interested in publishing separate modules. All of the solutions listed solve this problem (with the exception of Lerna which is simply there for comparisons sake since Lerna's design goals relate more to publishing rather than "putting up fences").

"does fencing in JS as well"

• only no-workspace-pollution and wotan solve the critical path feature in both TS and JS.

"does not break tslint from running"

• There are two open issues (https://github.com/palantir/tslint/issues/4137 and https://github.com/palantir/tslint/issues/4148) that show how TypeScript's "project references" are not currently compatible with tslint. The other two competitors (good-fences and wotan) live outside of tslint by design.

"usable without having to employ monorepo solution"

• Since I'm simply trying to solve the task of putting up helpful fences for separation of concerns, having a mono repo is not a requirement. For my current team's project, I'm not looking to take on the complexity of monorepos at this time (i.e. more involved CI/CD pipelines ala CodeFresh, etc). I'm simply looking for a way to say that only certain parts of the code are allowed to communicate with other parts.

"the fences are configurable"

• As @ajafff shared about wotan, "It's currently not configurable, you can see for example that the scope @fimbul/ is hard coded in the rule. The directory structure probably also need to be configurable to be really useful."

"declarative (i.e. here is my workspace vs. imperatively telling to restrict)"

• This point is a subjective one (so please forgive me for including it here) but I like code that simply states "this is where my workspaces are" and then the processing is offloaded elsewhere. The imperative style configuration of good-fences requires that you state which tags are allowable imports which (although it is more configurable) leads to more complex configuration and possible backdoors. That's why I chose to create a simple configuration for my rule (no-workspace-pollution) that is as follows:

    [true,
        {
            workspaceEntryPoint: "entrypoint.ts",
            workspaceName: "workspaceNumOne",
        },{
            workspaceEntryPoint: "entrypoint.ts",
            workspaceName: "workspaceNumTwo",
        }
    ]

"stays in sync with folder structure due to nested config files (ala fences.json)"

• This is the primary down-side of my implementation in that it doesn't infer the location of the workspaces via a file like good-fences' fence.json or typescript's _eventual goal_. So that means that my rule (no-workspace-pollution) has the possibility of breaking if you change your structure. However, if my rule (no-workspace-pollution) is accepted, I would like to immediately follow-on with a PR that allows the rule to check for the validity of the folder structure upon construction.

"feedback within vscode"

• good-fences and wotan do not have VsCode plugins.

"correctly allows standard "pathless" imports from node_modules"

• I have to file a bug in good-fences (Update: bug can be seen here), but I discovered that when you use good-fences you get errors that prevent you from importing "lodash", "bluebird", etc. That's a major issue if you can't bring in external dependencies.

"easy configuration"

• This is subjective, so please forgive me again. But I found the configuration to be much easier than the others. That's probably due to my rule (no-workspace-pollution) having a smaller scope. Here is a more detailed comparison of the configurations:
  
  
  
  • good-fences attempts to do more and therefore has more options. I found the system of "tags" to be more complicated than I would hope for. And since I'm currently evangelizing TypeScript over JavaScript at my company, I need to keep things as simple as possible.
  
  
  • Wotan was easy enough but it doesn't allow for configuration. If the porridge was too hot for good-fences, then Wotan's import-package rule was too cold (from a configuration perspective).
  
  
  • typescript's workspaces / project references has some planned enhancements for making life nice for developers (see @ryancavanaugh's fantastic post about their future plans). That being said, at the moment it doesn't meet the goal of easy configuration.
  
  

"does not require changes to your TypeScript transpilation build process"

• I think it's important that quality of life improvements like my new rule can be included without having to dramatically change your build. And if you already have tslint set up... then you can get the advantages of enforced separation of concerns. The best competitor is probably TypeScript's project references since it checks the most boxes, but there are so many open PRs for it that I would not feel comfortable using it at this time. That being said, I see no-workspace-pollution as being a very helpful stopgap until TypeScript project references complete it's remaining design goals.

Thank you everyone for any and all feedback. :)

[greenecreations]

Still soliciting community review on my above ^ summary of the current state of "fencing". Anyone else want to weigh in? @ericanderson / @tonyxiao / @JoshuaKGoldberg / @adidahiya

[ericanderson]

I am about to start using TS project references internally and I would like to get an understanding of how the world works with them before I am able to make a comment. My current guess is:

• If TS project references prevent this from happening any more, then there is no work to be done.
• If TS project references do not prevent this, they will provide us with a mechanism which we can use to validate that no imports resolve relatively to a project reference.

[tommedema]

Since Typescript has native references (monorepo) support, and you're obviously not supposed to import with a relative path outside the current package, I don't see why anyone would be opposed to this rule? This as well as references support (#4137) is what's lacking in tslint in our case.

[abierbaum]

The ts-import-zones rule from https://github.com/vladimiry/tslint-rules-bunch may address some of the usecases here. Not as full featured as some of the other solutions but definitely a good starting point if you need something now.

[adidahiya]

~also consider https://github.com/sverweij/dependency-cruiser~
edit: nvm, it doesn't handle this issue very well right now

[JoshuaKGoldberg]

The points made earlier this year are fantastic and the use case is certainly valid... but not many folks have seemed interested in this rule yet _(perhaps it's a bit ahead of its time?)_, and TSLint is being deprecated: #4534. I'd suggest bring this issue up in typescript-eslint.

Thanks for the discussions, folks! I'm hopeful the code in #4318 ends up being used by the community in some way - either as a standard ESLint rule or within the new core ruleset.