eslint-plugin-import icon indicating copy to clipboard operation
eslint-plugin-import copied to clipboard

Add new rule `module-boundary`

Open artursvonda opened this issue 2 years ago • 13 comments

This rule is somewhat similar to no-internal-modules but doesn't require additional setup.

The basics of the rule are very simple – if one of the parent directories resolve to a module (usually, means directory contains index file), you should be importing from that module. This allows for setting up boundary and disallow deep imports where they aren't desired.

This could be merged into no-internal-modules rule as an option as well, I welcome feedback.

artursvonda avatar Dec 04 '21 12:12 artursvonda

@ljharb, good point on naming, definitely could change. We could even be very specific in name that this is about index-as-package-boundary..

“contains an index file” in no way indicates something is a package. Folders often have an index file (I’d argue they always should) and i have plenty of packages that don’t have one at all.

This is true which is why this probably isn't an universal rule and doesn't suit everyone. But I find it a good rule of thumb on how to manage boundaries in JavsScript code. This is sort of opt-in that from now on "index" file will indicate package, boundary of which should not be crossed.

Another option to solve issue of accessing private modules would be marking them with _ prefix (or something similar) but I personally am not fan of that.

My question then is – do you think this this rule could belong here or I best publish this in a separate package?

artursvonda avatar Dec 04 '21 15:12 artursvonda

Can you help me understand why it’s a good rule of thumb? Deep imports are objectively superior to always importing from a manifest file, since it obviates the need for treeshaking.

I completely agree that a mere convention for privacy is actively harmful; if it’s reachable it’s public. A package uses the “exports” field to restrict private things, but relative folders indeed don’t have the same protection available.

ljharb avatar Dec 04 '21 15:12 ljharb

Can you help me understand why it’s a good rule of thumb? Deep imports are objectively superior to always importing from a manifest file, since it obviates the need for treeshaking.

I think I should've made the context a bit clearer. What you're saying is definitely true for libraries. And this probably isn't very useful for producing 3rd party packages. When producing this, I was mainly concerned with large web monolith style apps. In our use case, we want to be able to define boundaries around packages easily and in scalable way so we can force developer to think, what's "public" (can be used by users of the package) and what can be easily refactored without worry of breaking code of others. In other words, we find it useful to be able to define what's public and what's private module within specific package without manually defining all patterns (like one would use with no-internal-modules). One example of that is we break our code down into feature packages but we don't want other packages to do deep imports into other packages. And packages might be nested. And we don't necessary want to have directory pattern (like nesting packages inside packages dir) because that in our opinion adds unnecessary level.

Going back to original description, the motivation is similar to that of no-internal-modules but provides other means to achieve the same.

artursvonda avatar Dec 04 '21 17:12 artursvonda

Right - I’m saying that especially in large monolithic apps, deep imports are objectively better and will result in smaller bundle sizes than treeshaking is capable of producing.

ljharb avatar Dec 04 '21 18:12 ljharb

That's true, but I think that's at the expanse of maintainability and encapsulation which at least to our team are very important. And with proper tree-shaking there shouldn't be any issues with producing correctly sized bundles.

I don't think this should be default rule for everyone but it looks to me like it fits in with goals of this plugin, given no-internal-modules is already part of it.

artursvonda avatar Dec 05 '21 12:12 artursvonda

There is no tree-shaking that can do as good a job as “only import what you need in the first place” - the limits of the JS language speak to that.

A rule that limits what you can import from where is a fit for this plugin - I’m not convinced that this particular boundary definition would be helpful, since it encourages what i consider to be a harmful practice (relying on manifest/barrel export files)

ljharb avatar Dec 05 '21 14:12 ljharb

Fair enough. Mind pointing me in direction where I could read more about these limitation? The only ones I'm aware of are related to import * and export * or when unshakable transforms are used to transform modern code into legacy compatible code. Otherwise, with properly set up bundler, circular dependency checks and proper imports and manifest files, I'm not aware of any inherent limitations to tree-shaking.

Also, maybe I'm approaching this wrong. Are you aware of other lightweight ways to limit access to package internals?

artursvonda avatar Dec 05 '21 14:12 artursvonda

The best way is to use “exports”, and in a monolithic app, break up the monolith into separate actually-published (even internally) packages.

ljharb avatar Dec 05 '21 14:12 ljharb

I had written a similar rule for our private repo. the background: we split the app to several sub-modules:

~/app/src
├── common
├── pages
|  ├── moduel1
|  ├── module2
|  ├── ...
  • import common in module1/module2: ✅allowed
  • import module1 in module2 ❌not allowed.

What do you think make the rule configurable - allowing users to define what's a module-boundary?

aladdin-add avatar Dec 07 '21 08:12 aladdin-add

I had written a similar rule for our private repo. the background: we split the app to several sub-modules:

~/app/src
├── common
├── pages
|  ├── moduel1
|  ├── module2
|  ├── ...
  • import common in module1/module2: ✅allowed
  • import module1 in module2 ❌not allowed.

What do you think make the rule configurable - allowing users to define what's a module-boundary?

I think you could use https://github.com/javierbrea/eslint-plugin-boundaries plugin for that. It allows defining packages using patterns and defining relationships between them. We use it for similar purpose.

artursvonda avatar Dec 07 '21 09:12 artursvonda

👍 looks great, thanks!

aladdin-add avatar Dec 07 '21 09:12 aladdin-add

@ljharb are there any existing rules in the plugin that could be used to discourage barrel files?

jacobrask avatar Feb 02 '22 10:02 jacobrask

@jacobrask no, i don't think there's an existing rule for that. I'd be interested in adding one, though.

ljharb avatar Jun 30 '22 20:06 ljharb