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

New rule: no-wildcard-export #2493

Open
gajus opened this issue Jul 11, 2022 · 11 comments
Open

New rule: no-wildcard-export #2493

gajus opened this issue Jul 11, 2022 · 11 comments

Comments

@gajus
Copy link
Contributor

gajus commented Jul 11, 2022

Examples

✅ Valid with no-wildcard-export

export { parseZodIssue } from './parseZodIssue';

❌ Invalid with no-wildcard-export

export * from './parseZodIssue';

Why?

export * breaks static analysis of tools like unimported, effectively making it impossible to determine if there is unused code in the codebase.

@ljharb
Copy link
Member

ljharb commented Jul 11, 2022

I'm not familiar with that tool, but I'm not sure why it would break static analysis - you'd just bounce over to the other file and see what that exports.

@gajus
Copy link
Contributor Author

gajus commented Jul 11, 2022

I'm not familiar with that tool, but I'm not sure why it would break static analysis - you'd just bounce over to the other file and see what that exports.

It is possible that it is a limitation of the tool. Nonetheless, it is caused by export *.

Don't want to muddy the waters, but I would also argue that export * hurts legibility of the codebase by obscuring the origin of exports. Whenever I clickthrough in VSCode to an index.ts file that only contains export * statements, it is near impossible to find where the declaration is coming from.

@ljharb
Copy link
Member

ljharb commented Jul 11, 2022

I agree; re-exporting should be avoided whether it’s explicit or implicit. I don’t however think that the unnecessary limitations of one arbitrary tool is a reasonable motivation for a rule (considering no-restricted-syntax can already ban it).

@gajus
Copy link
Contributor Author

gajus commented Jul 11, 2022

If we rephrase the issue as the code legibility, is that not a good enough reason for introducing the rule?

Good hint about no-restricted-syntax. That does solve for our use case, for what it is worth it.

@ljharb
Copy link
Member

ljharb commented Jul 11, 2022

I think if a compelling enough case can be presented, then yes - but i suspect such a case would also discourage any re-exporting, whether with export *, export { x } from '…', or import x, { y } from '…'; export default y; export { x }` etc.

@me4502
Copy link

me4502 commented Aug 16, 2022

IMO the main usecase for a rule like this over no-restricted-syntax is that it can have an auto-fixer when type information is available (replace the wildcard with all named exports), as well as build tooling performance.

Especially in tests where modules are often isolated, knowing what exports are available from a file prevents having to parse that file when none of the exports are actually needed. This can substantially speed up build times on larger projects.

While barrel files aren't best practice, they're still widely used across the industry and ecosystem. A lint rule like this would remove some of the pitfalls (duplicate exports, lack of clarity on where exports are coming from, etc), and improve build speeds potentially substantially.

@JounQin
Copy link
Collaborator

JounQin commented Aug 16, 2022

My question would be why export * is introduced into the ES spec if it should be considered as harmful, and I personally love this syntax because it's short and we don't need to manage exports a lot of places. The original issue should reported to unimported instead IMO.

@me4502
Copy link

me4502 commented Aug 16, 2022

I feel that's somewhat of a poor reason to not have a linting rule. Linting rules exist to match preferences of a codebase, and for codebases that want this it can have a major impact.

While these seem to break unimported, the fact that wildcard exports slow down all build tools isn't something that build tools can avoid. They need to know if a file contains the export they're looking for, so they need to parse the file to determine that when a wildcard is used.

A rule like this that makes barrel files more usable and performant on larger codebases IMO makes a lot of sense. I don't believe this should be in the recommended set of linting rules, just one that people can enable themselves if they want.

@ljharb
Copy link
Member

ljharb commented Aug 16, 2022

@JounQin quite a lot of things that are now considered harmful exist in the ES spec - existence in any spec is never an inherent endorsement for using anything. Explicit > implicit, and if you don't want to manage exports in a lot of places, don't re-export them anywhere - instead, deep import exactly what you need from the lone place it should be exported from.

@me4502 Barrel files are harmful - they cause tons of bundle/memory bloat that treeshaking can never fully account for, as well as conceptual and maintenance burden. They should not be made easier to use, they should be actively prevented.

@AntonNiklasson
Copy link

AntonNiklasson commented Mar 5, 2023

I'd like to enable a rule like this in our codebase. It would be helpful to enforce more explicit reexports in a few places, but we still find the barrel file pretty ergonomic.

Are you open to contributions here? I could look into it, just want to make sure there's a green light.

@katryo
Copy link

katryo commented Mar 13, 2023

I came to this thread from Google Search. I think no-wildcard-export rule makes sense for many developers.

Just fyi. As people suggested above, we can ban export * by no-restricted-syntax in this way.

"no-restricted-syntax": [
      "error",
      {
        selector: ":matches(ExportAllDeclaration)",
        message: "Export only modules you need.",
      },
    ],

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

No branches or pull requests

6 participants