Skip to content
This repository has been archived by the owner on Nov 30, 2020. It is now read-only.

import/no-default-export #39

Closed
TkDodo opened this issue Nov 20, 2019 · 8 comments
Closed

import/no-default-export #39

TkDodo opened this issue Nov 20, 2019 · 8 comments

Comments

@TkDodo
Copy link
Contributor

TkDodo commented Nov 20, 2019

PR #63

https://github.com/benmosher/eslint-plugin-import/blob/master/docs/rules/no-default-export.md

We should turn on that rule. In some repos (presense-frontend, design-system), we could easily adapt that rule. In insights and adverity-components, it is rather difficult. We have 3 options now:

  • enable those rules in the repos directly that can already support it (not that good).
  • enable it here per default, release as a breaking change, and disable it in repos that still have default exports (better, but breaking).
  • enable it here per default as a warning, and lint the repos where we already have no default exports with --max-warnings=0, which basically makes warning to errors

@namjul @vlabs @danielbartsch

See also: https://github.com/pmedianetwork/handbook/issues/13

@namjul
Copy link
Contributor

namjul commented Nov 21, 2019

a fourth option would be to enable it here as an arror but disable it again in the repos which cannot be turned easily to named export.

@TkDodo
Copy link
Contributor Author

TkDodo commented Nov 21, 2019

a fourth option would be to enable it here as an arror but disable it again in the repos which cannot be turned easily to named export.

That’s just Option 2 - the breaking change ...

@danielbartsch
Copy link
Contributor

What's the downside of default exports?
Why was this suggested?

I personally haven't seen any downsides to it.
Also the new storybook file format would violate this rule, as there must be a default export, and several named exports: https://storybook.js.org/docs/formats/component-story-format/

@namjul
Copy link
Contributor

namjul commented Nov 21, 2019

What's the downside of default exports?
Why was this suggested?

I personally haven't seen any downsides to it.
Also the new storybook file format would violate this rule, as there must be a default export, and several named exports: https://storybook.js.org/docs/formats/component-story-format/

That is true storybook requires default export. We can enable it for *.[story|stories].js files.

@TkDodo
Copy link
Contributor Author

TkDodo commented Nov 21, 2019

Why was this suggested?

to get a consistent style throughout all repositories.

What's the downside of default exports?

https://humanwhocodes.com/blog/2019/01/stop-using-default-exports-javascript-module/

That is true storybook requires default export

If we need to, we can make an exception for stories.

@TkDodo
Copy link
Contributor Author

TkDodo commented Nov 21, 2019

What's the downside of default exports?

Also, I might be mistaken, so correct me if I'm wrong, but I think that if you directly export an anonymous function react component as default export, like this:

export default () => (...) 

the component will show up with the name _default in the react dev tools. It will get a correct name if you do:

export const Foo = () => (...)

export default Foo

but at that point, why not just do:

export const Foo = () => (...)

@TkDodo
Copy link
Contributor Author

TkDodo commented Nov 30, 2020

code splitting also requires default exports, but we are not actively doing this (at the moment), and there are also workarounds. The handbook also mentions that we don't want default exports, so I think warning should be a good compromise for this rule.

@TkDodo TkDodo closed this as completed Nov 30, 2020
@TkDodo
Copy link
Contributor Author

TkDodo commented Nov 30, 2020

Issue moved to pmedianetwork/adverity-frontend-monorepo #1237 via ZenHub

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants