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

Rule to prohibit default exports #889

Closed
dead-claudia opened this issue Jul 8, 2017 · 19 comments
Closed

Rule to prohibit default exports #889

dead-claudia opened this issue Jul 8, 2017 · 19 comments

Comments

@dead-claudia
Copy link
Contributor

TSLint has a similar rule, where it prohibits default exports. Basically this (second case generating a warning):

// good-1.js
export function foo() {}

// good-2.js
function foo() {}
export {foo}

// bad-1.js
export default function foo() {}

// bad-2.js
function foo() {}
export {foo as default}
@ljharb
Copy link
Member

ljharb commented Jul 9, 2017

Why?? Default exports are the way things should operate most of the time.

The link says "named imports/exports promote clarity", which I find blatantly false - a single default export + the filename is the most clear thing in my experience. Additionally, "current tooling" doesn't "differ" in how to handle default exports; handling them is pretty clear.

@dead-claudia
Copy link
Contributor Author

@ljharb

It's mostly a style preference - some people feel it's clearer/easier to maintain when you can't have effectively anonymous default exports. You can basically pick whatever name you want for default exports, but it's harder to do that with named ones, and some people prefer the increased structure.

For similar reasons, TSLint also has a similar rule that requires the imported and exported names to match.

@ljharb
Copy link
Member

ljharb commented Jul 10, 2017

The filename is the name for a default export - they're never anonymous.

So far all the reasons I've heard are covered by "have the most basic unit tests you can think of", and "provide a non-useless filename" - both of which anyone using a linter is likely already doing.

I'm a -1 on having this rule; I think it's a very harmful style preference.

@dead-claudia
Copy link
Contributor Author

I personally just like the consistency of only having one type of import used throughout. I do agree many of their claims (on the TSLint side) are invalid, though, and my reasoning differs from theirs.

  • I find it easier to track files in larger projects when you have one less variable. One of my personal projects is literally 40k lines of code of plain ES5 + CommonJS.
  • I prefer thinking of modules as boxes, not particular items within them, and default exports make it harder to keep this distinction. This is the difference between Java classes and Python modules - Java classes are single items with their own unique box, while Python modules are boxes with one or more items in them.

(I probably should've used this from the onset...)

@ljharb
Copy link
Member

ljharb commented Jul 11, 2017

JS isn't Java or Python. A JS Module isn't a box; it's either a thing or it's not (default), and then separately, it has 0-N additional named exports. Most things should be things - not "not a thing, but has an extra thing".

@benmosher
Copy link
Member

Default exports are the way things should operate most of the time.

Strong statement, @ljharb. I also have a tendency toward default exports but I agree with @isiahmeadows that it's a style thing. and I can imagine a team gelling around it and using a lint rule to enforce/communicate it with new hires/contributors.

I'd accept a PR implementing a no-default-export rule, but I wouldn't enable it by default anywhere.

@mordaha
Copy link

mordaha commented Sep 2, 2017

@ljharb JS module is not a thing. It is an abstraction for thinking about a project structure.

It is good to export default from library stored in npm, so consumers can choose any name they like (especially if you name you lib offensive or sexist!))

import NiceName from 'ugly-name-lib';

But in a project with naming conventions and rules it is a dangerous freedom.

What names the module exports - that names should be used, and when you refactored that names - you should refactor every name in consuming modules in your project.

Good rule for application projects. TS-team right.

@ljharb
Copy link
Member

ljharb commented Sep 2, 2017

@mordaha in fact, it is a thing, ES2015 adds a parsing goal to the language for "Modules", which is where import and export come from. Short of that, the defacto standard for describing a "module" in JS is a file in the CJS (or AMD or UMD) format.

No, in fact they are wrong - you should never have to refactor the names in consuming modules. The choice of consuming name is up to the consumer and it should not impact the use of that module whatsoever.

If you want to restrict that freedom in your own consuming project, you can use linter rules to do so - it is simply user-hostile and incorrect for the providing module to attempt to force that preference upon you.

@vadimcoder
Copy link

If you want to restrict that freedom in your own consuming project, you can use linter rules to do so

@ljharb what is that wonderful rule you mentioned? Neither import/named nor import/default solves the problem.

@ljharb
Copy link
Member

ljharb commented Oct 16, 2017

@vinogradov #936 is a PR to add a rule that can force this; but the "problem" you linked to imo is not a problem. A default export is never anonymous; the filename is its name.

A default export is what a module (a file) is, a named export is what a module has.

@vadimcoder
Copy link

@ljharb If I was a library author, I would definitely use export default to expose my code to the world. Because I don't want to force people to dig into my module finding out what the hell my export names are. People are free to use filename or whatever name they wish. I won't see the client code anyway. But the situation is a bit different when I write an application. Now I am fully responsible both for the utils code ("libraries") and for the client code using it. I see the code on both sides and sometimes it hurts my eyes:

// campaign.js
export default class ClientCampaigns {}
// we may use eslint-plugin-filenames to check if filename matches the default export name
// no way to check this, let's forbid this use case:
import OldUglyNameForgetToRefactor from './campaign.js'; 

@ljharb
Copy link
Member

ljharb commented Oct 16, 2017

@vinogradov in that case, campaign.js should default-export something named "Campaign", and if it's going to export ClientCampaigns, it should be named client_campaigns or ClientCampaigns.

There is not yet a rule for #325, but that would address that use case.

@vadimcoder
Copy link

vadimcoder commented Oct 16, 2017

@ljharb selaux/eslint-plugin-filenames addresses that case already. Please address "import-default" case instead. It would be great to have a rule saying: "rename OldUglyNameForgetToRefactor to myBeautifulModuleName" when you do this:

import OldUglyNameForgetToRefactor from './myBeautifulModuleName.js';

If it is not possible please consider accepting the PR #936

@ljharb
Copy link
Member

ljharb commented Oct 16, 2017

@vinogradov I'm already a -1 on #936 (and this issue's rule), but i'm not blocking. Totally separately, and in parallel, we absolutely should add a rule that enforces the identifier you use when importing a default, and ensuring it matches the filename.

Also, I think filenames-match-exported is a rule that belongs in this plugin, since it's specific to imports and exports, so I think that should be added too.

@dead-claudia
Copy link
Contributor Author

@ljharb Note that I'm a heavy -1 on making it a recommended plugin. My only request is its existence.

Totally separately, and in parallel, we absolutely should add a rule that enforces the identifier you use when importing a default, and ensuring it matches the filename.

Definitely also a good idea, since I know of several that do it already. TypeScript itself independently enforces this via a compiler option.

@dead-claudia
Copy link
Contributor Author

Note that my main point is that eslint-plugin-import doesn't have to be super opinionated about everything.

@tobilen
Copy link

tobilen commented Nov 24, 2017

@isiahmeadows since this appears not to be happening, would you mind publishing your good work as a separate plugin?

@ljharb
Copy link
Member

ljharb commented Nov 24, 2017

@tobilen #936 is merely waiting for reviews from other collaborators.

@halfzebra
Copy link

halfzebra commented Jan 16, 2018

@isiahmeadows good stuff, I would love to have this rule available! ❤️

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

No branches or pull requests

7 participants