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 import/no-default-export rule #182

Merged
merged 2 commits into from
Jan 27, 2021
Merged

Add import/no-default-export rule #182

merged 2 commits into from
Jan 27, 2021

Conversation

umpox
Copy link
Contributor

@umpox umpox commented Jan 18, 2021

Thoughts on this?

  • We're already discouraging their usage in some of our documentation
  • Our usage of default exports is fairly low and this will help catch issues before they hit PR

Further reading on why default exports are bad

@tjkandala
Copy link

Great idea! How should we handle our webpack config exports?

@umpox
Copy link
Contributor Author

umpox commented Jan 19, 2021

Great idea! How should we handle our webpack config exports?

It's a fair point, we're going to run into tools or libs (hence the *.d.ts exclusion) that still make use of default exports, although it should be relatively rare that there won't be a better solutions to work with these. For example, we can still use module.exports with webpack, which is still a better solution than default, or even we could export const each part of the webpack config. These particular exports are actually passed around in our code and are used to call webpack directly as a function, so these should work fine as named exports.

@felixfbecker
Copy link
Contributor

@umpox I see you made this a draft PR, do you not want review on it yet? (I only stumbled across this by accident)

@umpox
Copy link
Contributor Author

umpox commented Jan 21, 2021

@felixfbecker Left it as a draft as I need to make changes to other repos before the build can pass, I guess there's no reason this can't be reviewed now though! Will change

@umpox umpox marked this pull request as ready for review January 21, 2021 10:38
@umpox umpox requested a review from a team January 25, 2021 11:19
@umpox umpox merged commit 0893896 into master Jan 27, 2021
@umpox umpox deleted the no-default-export branch January 27, 2021 14:15
@sourcegraph-buildkite
Copy link
Collaborator

🎉 This PR is included in version 0.21.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

4 participants