Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

no-internal-modules: configuration to opt in/out to enable/disable exports linting #1920

Open
Hotell opened this issue Oct 5, 2020 · 3 comments

Comments

@Hotell
Copy link

Hotell commented Oct 5, 2020

Some time ago no-internal-modules rule added ES modules exports linting as a "fix" -> minor version bump (which was a breaking change btw).

From my experience, linting exports with this rule doesn't make much sense, as barrel/public-api files, can re-export from any nested path.

for example angular public api patterns, nx patterns (with React as well)

For better imagination I'm adding showcase of scaffold/public API

2020-10-05 at 2 20 PM

feature proposal

  • to mitigate these kind of discussions, I'd like to propose to add configuration API to this rule to opt out/in for linting exports
rules: {
        'import/no-internal-modules': ['error',{lintExports:false}],
      }
@ljharb
Copy link
Member

ljharb commented Oct 5, 2020

It's not a breaking change for eslint plugins, where the bug being fixed can be "the build passed, but should not have". Semver works a bit differently in the eslint ecosystem.

Can you clarify a bit what's getting warned here but shouldn't be?

Typically, no-internal-modules should be defined such that within the folder that contains them, they're allowed to import each other.

@Hotell
Copy link
Author

Hotell commented Oct 31, 2020

Can you clarify a bit what's getting warned here but shouldn't be?

After that "fix", all of those exports in barrel file (see image in issue description) are violating no-internal-modules rule.

From my experience, exporting from nested paths is rarely a violation in a proper constraint systems with proper public API designs.

If we could add my proposed feature to opt out from applying this rule on exports it would be wonderful.

// lint rule config
"rules": {
    "import/no-internal-modules": [ "error"]
}

↓

// ❌lint error
export * from './lib/button/button'

// ❌lint error
import {Button} from './lib/button/button
// lint rule config
"rules": {
    "import/no-internal-modules": [ "error", {lintExports:false}]
}

↓

// ✅ ok
export * from './lib/button/button'

// ❌lint error
import {Button} from './lib/button/button

@ljharb
Copy link
Member

ljharb commented Oct 31, 2020

This sounds like a different rule to me. Barrel exports increase bundle size and memory footprint, and are the only reason treeshaking is needed (to only-partially clean up sloppy importing), and in my experience, are best avoided, especially in any codebase of significant scale/size.

no-internal-modules should be configured to warn for a directory inside a “package”, but only applied to things outside the “package”, and thus should be allowed for a barrel export which is inside the package but outside the directory.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

2 participants