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

Add allowAliasing option to import/no-default-export #1767

Draft
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

k-yle
Copy link

@k-yle k-yle commented May 18, 2020

The import/no-default-export rule has two parts

  • banning syntax like export default function () {} and export default foo;
  • banning aliased default exports (e.g. export { foo as default } from "./foot";)

In our use case, we only want to ban the first syntax, but allow aliased importing. This is because we use React lazy/suspense which requires default exports to be used.

This PR adds an allowAliasing option to allow just aliased exports

Hope this PR is okay. Haven't worked with eslint plugins much

@coveralls
Copy link

coveralls commented May 18, 2020

Coverage Status

Coverage increased (+0.002%) to 97.741% when pulling abbaabb on k-yle:master into 92caa35 on benmosher:master.

@ljharb
Copy link
Member

ljharb commented Jun 7, 2020

If aliasing is allowed, what's the point of disallowing default exports at all?

(note that I severely disagree with this rule existing, as I think a default export is what a module is, and a named export is something a module has, and most modules should be something)

@k-yle
Copy link
Author

k-yle commented Jun 7, 2020

If aliasing is allowed, what's the point of disallowing default exports at all?

In general we don't want to allow default exports, but aliassing should be allowed for when we need to use React's code-splitting.

@ljharb
Copy link
Member

ljharb commented Aug 8, 2021

Right, but why does "React.lazy requires default exports" not suggest that you should use default exports? Aliasing would just be a hack for using the wrong kind of export.

@ljharb ljharb marked this pull request as draft August 8, 2021 16:28
@k-yle
Copy link
Author

k-yle commented Aug 8, 2021

within the folder we still want to use named imports. We find named imports much more useful. If something outside of that folder wants to lazy load it, the index.js file re-exports it as a default export

@ljharb
Copy link
Member

ljharb commented Aug 8, 2021

In that case, you could use overrides to disable this rule for **/index.js, no option required?

@k-yle
Copy link
Author

k-yle commented Aug 8, 2021

for our specific case that could work. but is there a harm in adding an option?

@ljharb
Copy link
Member

ljharb commented Aug 9, 2021

There's always harm in adding things :-) it's just got to be weighed against the benefit. Since a) this rule imo should never have existed in the first place, and b) your use case is trivially solved with overrides, unless there's a use case for an option it doesn't seem worth it.

@ccjmne
Copy link

ccjmne commented Jul 14, 2022

@ljharb Hey!

I was about to suggest adding an option with a glob pattern for files to ignore... like import/no-extraneous-dependencies allows, for instance, and didn't know of the overrides you just mentioned.
Thanks for that! You just got me sorted :)

--

I'm curious as to why you think import/no-default-export should never have existed, though?

I favour named exports because they allow me to more easily refactor/auto-import bits and pieces from other parts of the project, and came to believe that now that ES Modules are pretty commonplace and well-established, maybe I could simply enforce avoiding default exports consistently, except when dealing with external libraries that require a "default" export.

I don't want to argue or anything, I genuinely want to know what your take on this matter is!
Maybe you figure that "enforcing" named exports in code that is not exposed as part of a library or anything isn't a worthwhile matter, maybe you feel that default exports are just simpler and should be preferred (after all, even conceptually, that's definitely true, and until pretty recently I was thinking just that 😂) or maybe it's something else altogether? For instance, the mere fact that many external tools likely always will be looking for a default export... may be sufficient to discard the idea of the import/no-default-export rule.

Welp, so much for asking a quick question, it already looks like a wall of text :')

Again, I'm just asking for you to elaborate, out of curiosity! Thanks for mentioning ESLint's overrides :)

@ljharb
Copy link
Member

ljharb commented Aug 30, 2022

@ccjmne
A default export is what a module is, a named export is something a module has. Most modules should be something.

It's a shame that IDEs have failed to implement the same autocorrect and auto-refactor tooling for default imports - there's zero technical reason why this is difficult that I'm aware of - but I'd encourage you not to alter how you architect your codebase solely based on the momentary flaws of your tooling.

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

Successfully merging this pull request may close these issues.

4 participants