airbnb / javascript

JavaScript Style Guide
MIT License
145.27k stars 26.52k forks source link

What is the benefit of prefer-default-export? #1365

Closed CWSpear closed 7 years ago

CWSpear commented 7 years ago

The docs don't have a Why section for prefer-default-export, and I'm not seeing the benefit of it on my own. I would think that not using default is preferred. With default exports, you lose refactoring power (if you rename the source const/function/class, it won't rename default imports).

As more of an edge case: it makes code less future-proof. i.e. if you create a file that will be a collection of errors, but it only starts with one error, to follow the linting rules, you'll have to have it export default, but then when you add the 2nd error at a later time, you'll have to do a bunch of refactoring that could have been prevented if the recommendation was to avoid default export.

ruslanvs commented 4 years ago

It is very simple imho:

ljharb commented 4 years ago

@ruslanvs named exports can be renamed, as can defaults; the two methods are utterly identical in this regard. If you enforce sameness with a linter, you can do so with either method as well.

ruslanvs commented 4 years ago

@ljharb

ljharb commented 4 years ago

@ruslanvs import { foo as bar } from ‘path’ is legal JS syntax. The “issue” is impossible to eliminate without additional tooling, because JS in no way forces any specific identifier name ever.

ruslanvs commented 4 years ago

@ljharb

ljharb commented 4 years ago

@ruslanvs if you follow the other advice in this guide, the default export's name will match the filename anyways, so the information is in the specifier. Nothing is lost.

gaurav- commented 4 years ago

@ruslanvs I don't find import bar from 'path' harder to trace – isn't it explicitly importing the default export in 'path'?

As far as the prefer-default-export rule is considered, to me it's a reminder to include only closely related stuff in a single module (high cohesion) and not export unnecessarily (encapsulation). There are also valid cases where it makes sense to export multiple symbols from the same module.

ruslanvs commented 4 years ago

@gaurav- can you please share a code of workflow sample, in which it would be easier to trace an export of 'abc' from 'path', while it is being imported as import bar from 'path' as opposed to when it is imported as import { abc } from 'path'?

prefer-default-export is just what it is to me, although you may be associating some good practices with it, it does not stop many other people from misusing default imports.

@ljharb please check out the sample provided by @ljharb above. It shows why " the default export's name will match the filename anyways" does not work.

Question again, what is the benefit of not using language's native way to enforce imports to be more explicit and consistent, using names of the imports with named imports in favor of a less explicit default import, that does does not natively enforce consistent import naming?

ljharb commented 4 years ago

That’s not the purpose of named exports - it’s literally zero of the intent.

ruslanvs commented 4 years ago

@ljharb can you please provide any code or workflow sample that would make default exports more useful than named exports? You're yet to show one. By the way, did you notice that some 50% of all posts in this thread are all yours?

Named exports make imports more explicit and consistent than default exports:

Argument about "exporting just one thing" with default exports does not work: exporting that "just one thing" using named export guarantees that it would also be imported under the same name everywhere, making the code base more traceable and consistent. Default exports lack that.

ljharb commented 4 years ago

@ruslanvs Yes, I maintain this guide, so it's appropriate that the majority of comments in the repo are all mine.

There's no code example that matters one way or the other - see https://github.com/airbnb/javascript/issues/1365#issuecomment-548202974:

A default export is what a module is, a named export is what a module has. The module's "name" is its specifier/file path/url.

Keeping the identifier name the same does not aid readability in an objective way.

However, if you disagree with this guide, you are more than welcome to fork it and modify it as you see fit!

gaurav- commented 4 years ago

@gaurav- can you please share a code of workflow sample, in which it would be easier to trace an export of 'abc' from 'path', while it is being imported as import bar from 'path' as opposed to when it is imported as import { abc } from 'path'?

@ruslanvs I didn't claim one is "easier" than the other. I only wrote that I don't find one to be "harder" than the other; like you seem to find default to be harder than named. In other words, they are the same to me in terms of traceability.

prefer-default-export is just what it is to me, although you may be associating some good practices with it, it does not stop many other people from misusing default imports.

Isn't the whole point of linting rules to help with the best practices that you agree with? If you don't agree with it, you can define your own linting rules!

I'm curious about the misuse of default though. Could you show some examples or point me to the comments that already have these examples?

ruslanvs commented 4 years ago

@ljharb thank you for your work. If I did not really love arbnb's Javascript ruleset overall, I would not be spending time here today.

The guide is opinionated for a good reason, much of which is about code readability and maintainability - and for 99.99% of rules in this guide - that practical approach makes me love this guide and makes eslint + airbnb style experience overall so phenomenally good.

I do believe that this particular rule is detrimental to code readability and maintainability. I have provided very straightforward reasoning above for that.

However, none of the reasoning provided in this thread, #1365 (comment) including, have addressed the practical reality of code readability and maintainability. Your "There's no code example that matters one way or the other" demonstrates that.

Perhaps, just like any other rule that is revisited from time to time here to make the guide even better, this one should also be revisited with practical code quality and workflow purposes in mind.

Once again, thanks for the overall phenomenally good guide to you and the team.

ruslanvs commented 4 years ago

@gaurav- speaking of how default export does not help embrace "exporting just one thing" - how about this (rather benign) example? -

// util.js
export default {
  doSomething() {},
  doSomethingTotallyUnrelated() {},
  evenMoreUnrelatedData: 'Only I know where to look for this data',
}

// app.js
import appUtil from './util';
appUtil.doSomething();

// MyComponent.jsx
import xyz from './util';
xyz.doSomethingTotallyUnrelated();

Let me know if this is not horrible enough. The point is, enforcing default export for single exports does not address the issue of exporting balls of mud.

With named exports you are at least guaranteed that (a) there is a name to the export, and (b) that this name will show up in every file this thing is imported in.

ljharb commented 4 years ago

You continue to assert that these things have value - but without any backing except that it's more readable to you. If that's something you want, fork the guide and change it. It's not something desired in this guide at this time (it's something explicitly unwanted by this guide, in fact)

gaurav- commented 4 years ago

@ruslanvs that example doesn't fare any better with named exports, at least not to me.

// util.js
export function doSomething() {}

export function doSomethingTotallyUnrelated() {}

export const evenMoreUnrelatedData = 'Only I know where to look for this data';

// app.js
import * as appUtil from './util';
appUtil.doSomething();

// MyComponent.jsx
import * as xyz from './util';
xyz.doSomethingTotallyUnrelated();

This just reiterates my point about them being no different in terms of "traceability".

Modularity is of course not determined merely by the use of default vs named. Like I mentioned earlier, it's poor modularity to keep unrelated code in the same module, even if they are exported as named symbols.

ruslanvs commented 4 years ago

@gaurav- 1) That example was addressing your question on misuse of default import. Hopefully it leaves no doubt that default import itself has no effect on good or bad code modularization practices. 2) Please notice that in your example every instance of the named export is still referred to with its very name in every import - there is no way around it. You can easily grep / global search that name and be guaranteed to find every import instantly. You do not get it with default export (yes there are other ways, but those require marginally more work, so "why?").

Based on maintainer's feedback, this discussion has no effect, hence this is my last comment in this thread unless the context changes. Please feel free to DM.

gaurav- commented 4 years ago
  1. Going by the language semantics, knowing that default is the module does serve as a guideline/reminder to me, as I mentioned in https://github.com/airbnb/javascript/issues/1365#issuecomment-579432945. Of course, it doesn't automatically make your code good or bad in terms of modularity – something I never suggested.

  2. If it makes sense for your module to export many symbols, or even just one named symbol, please do so by all means if that works better for your project. The prefer-default-export rule doesn't ban named exports. It just checks with you if maybe you wanted that one named export to be the default export. Please read https://github.com/airbnb/javascript/issues/1365#issuecomment-416031279 for my detailed take on it

jonyyz commented 4 years ago

@ruslanvs named exports can be renamed, as can defaults; the two methods are utterly identical in this regard. If you enforce sameness with a linter, you can do so with either method as well.

The point is, there is no authoritative name for default exports other than "default". Therefore consumers can name the export every possible permutation of characters in a character set. That can make a codebase completely unreadable. In file 1, "foo" refers to "bar" but in file 2 "foo" refers to "baz". If something is exported with a name, you have the option to rename. That is typically used in the case where imports would have naming collisions like:

import { Widget } from "widget-library-1";
import { Widget } from "widget-library-2";

the as semantic is for this specific case where you can

import { Widget } from "widget-library-1";
import { Widget as WidgetAvoidingNameCollision } from "widget-library-2";

This is a common semantic in many languages including .NET languages. Even if you claim that both of these are equivalent, which I would argue they aren't, you've essentially pointed out that ES6 imports are poorly designed with respect to these topics we are discussing and therefore no choice can remedy that other than to redesign it.

jonyyz commented 4 years ago
  1. If it makes sense for your module to export many symbols, or even just one named symbol, please do so by all means if that works better for your project. The prefer-default-export rule doesn't ban named exports. It just checks with you if maybe you wanted that one named export to be the default export. Please read #1365 (comment) for my detailed take on it

The problem with this argument is let's say I have a file of constants. In version 1 of my application I start out with 1 constant and thus have to use export default. The consumers then consume the default export. Then in version 2 of my application I add another constant, now I have to name it because there cannot be two default exports. Now constant 1 is imported as default (for backwards compatability) and constant 2 is imported named and for no apparent semantic reason. To change constant 1 to a named export means I have to refactor all the existing consumers. Or just use named exports from the beginning because this linter rule is silly. That's why this linter rule makes no sense because you can't apply it to all situations. It's up to the developer to decide which one applies best using best judgment.

ljharb commented 4 years ago

@jonyyz for a file of constants, it's appropriate to override this rule, so that even with 1, you'd want it to be a named export. Linter rules don't have to apply to all situations; that's what overrides are for.

jonyyz commented 4 years ago

@jonyyz for a file of constants, it's appropriate to override this rule, so that even with 1, you'd want it to be a named export. Linter rules don't have to apply to all situations; that's what overrides are for.

And a list of helper functions... and a list of typescript types. You seem to be claiming this situation is normally the rule and there are very few exceptions. I don't think that's true at all. I think it's up to the developer to decide what works best on a case by case basis (the default). There are many cases in software engineering where the answer to "Should I do X?" is "it depends." In other words, there are frequently situations where there are not absolute or objective truths in software engineering. A software engineer who expects that to be the case 100% of the time is a very unhappy software engineer. This is coming from a software engineer who has been around for some time and used to be a more youthful and idealistic engineer. 😄 I have defended many an ivory tower and find it humorous when I look back on my younger self.

ljharb commented 4 years ago

Helper functions should be separate files rather than a bag-o-helpers; I'm skeptical that a single list of TS types (as opposed to each type living with each thing it's describing) makes much sense.

It's entirely up to the developer - nobody's forcing you to follow this guide or to use the eslint configuration unmodified. I've explicitly said upthread that there's exceptions, but the rule is highly useful since those exceptions are, in most projects I've worked in, rare.

dfernandez-asapp commented 4 years ago

Wow, this long thread shows that this rule divides opinions.

I use default exports because I'm used to the Java-style of having one file per class, even if I don't use classes much in JavaScript code.

But lately, I'm thinking to switch to the opposite: disallow default exports.

My main reason doesn't have to do with code style, refactoring, or tree-shaking. The problem that I'm having with default exports is with re-exports.

Here is an example of what I mean: https://codesandbox.io/s/nice-sammet-jem3c?file=/src/index.ts

In the example, there are two components ComponentA and ComponentB, and a parent module called components. The parent module re-exports all the components.

According to the spec: export * from './ComponentA' will not do a default export: https://exploringjs.com/es6/leanpub-endnotes.html#fn-modules_2

But the problem is that sets an export named default! See the linked example, when you do: import * as components from './components'

The components variable will contain an entry called default that shouldn't be there. And there is more... the value of the entry depends on the re-export order, meaning that the order of re-exports have side-effects 😱:

export * from './ComponentA'
export * from './ComponentB'

is not the same as

export * from './ComponentB'
export * from './ComponentA'

because it changes the value of components.default.

The example uses Parcel which uses Babel internally. But the problem is also present in plain TypeScript. I couldn't check it with the browser.

Possible solutions:

  1. Stop using export * and do explicit exports. But as you can imagine, as the component library grows it's painful.
  2. Do not use default exports.

I'm leaning towards 2.

Any opinions? How others resolve re-exports with default exports?

slikts commented 4 years ago

You should do export { default as ComponentA } from './ComponentA' when using default exports.

dfernandez-asapp commented 4 years ago

You should do export { default as ComponentA } from './ComponentA' when using default exports.

Yes, I'm doing that, see the example: https://codesandbox.io/s/nice-sammet-jem3c?file=/src/components.ts

The problem is not with export {default as ComponentA} from ... the problem is that I also need to export the non-default stuff: export * from './ComponentA' according to spec this should not export default which is fine (is just two lines of code).

But in practice it does export a value named default which is visible only if you do import * as components (see the example).

ljharb commented 4 years ago

@dfernandez-asapp this is a separate topic, but might i suggest that the problem is re-exports :-) your consumers should be importing exactly what they need, from the deep path it's in, as opposed to pulling from a "manifest" export bag that borgs up all the things in your project into one entry point. Treeshaking is only a thing that has an effect when folks are sloppily importing one thing from a big bag-o-things; when using proper deep imports, treeshaking has no value and isn't needed.

dfernandez-asapp commented 4 years ago

@ljharb having a single entry point, doesn't affect treeshaking (if is done right) and is a practice that makes working with ESM modules easier.

If you package ESM+CommonJS, the single entry point hides the implementation detail and the bundler (Webpack, Rollup or Parcel) will pick the ESM declaration and do the treeshaking. Some examples in the wild: Material UI encourages you to use import {} from '@material-ui/.. instead of path imports, API Extractor from MS enforces that you have one entry point only, RxJS does the same, and React does the same (import React from 'react' only works with module interop).

I know it's a different topic, but I disagree with the "export bag that borgs all the things"... in fact having a single entry point is the best way to hide the internal file structure of your package. Treeshaking works fine in that case.

ljharb commented 4 years ago

@dfernandez-asapp it doesn't affect it, it causes it to be necessary. You shouldn't need treeshaking at all except to clean up sloppiness.

Jamesernator commented 4 years ago

@ljharb having a single entry point, doesn't affect treeshaking (if is done right) and is a practice that makes working with ESM modules easier. @dfernandez-asapp it doesn't affect it, it causes it to be necessary. You shouldn't need treeshaking at all except to clean up sloppiness.

I suggested a proposal a long time ago that would offer the benefits of tree-shaking as part of the language, but in a predictable way, however it never went anywhere unfortunately.

ljharb commented 4 years ago

@Jamesernator there aren't any benefits to it. All it does is attempt - badly and incompletely - to clean up the mess made by importing from modules that export too many things.

Fabyao commented 4 years ago

I have decided to turn off the default export rule in favour of named exports. This is based on the recommendation of the creator of ESLint Nicolas Zakas. Find more details here: https://humanwhocodes.com/blog/2019/01/stop-using-default-exports-javascript-module/

More importantly, named export add consistency that I find useful in my projects. @ljharb I take your point that one can easily rename the import however the default behaviour of named export vs export default makes named exports more suitable.

ljharb commented 4 years ago

The thrust of that article is about not knowing what you're importing, using named exports doesn't actually help this. imo the article does not actually present a good argument for using named exports, just their preference for it.

Fabyao commented 4 years ago

@ljharb we will have to agree to disagree :smile: . The argument is that using named exports gives you a better experience. That is if you export using export default, when importing the module, you have no indication (at least in VS Code) as to what you are dealing with. If however you export using a named export, VS Code gives you some useful information. Also the re-export statements are less verbose with named exports.

ljharb commented 4 years ago

Changing how you write your code because of flaws or gaps in your tooling is a very short-sighted approach (and i'd suggest not re-exporting things anyways; deep import what you need, don't make a big manifest file to import from).

ruslanvs commented 4 years ago

@Fabyao Awesome article link, thanks!

aryzing commented 4 years ago

@Fabyao excellent article, @ljharb please reconsider: the upvotes outnumber the downvotes 9.8 to 1 at time of writing. Given most of the feedback is around refactoring and dev speed, this seems to be slowing down a lot of people.

ljharb commented 4 years ago

Decisions aren't made by voting, and as I said above, the article does not in fact make any arguments against default exports that hold up to scrutiny.

aryzing commented 4 years ago

Not saying that they are, just pointing out that a change would benefit most the participants of this thread. Regarding scrutiny, it's not an objective thing out there in the ether, you're the one scruitananzing, and actively deciding that it does not hold up to your standards, despite an overwhelming majority of participants stating otherwise.

halfzebra commented 4 years ago

@ljharb Hi Jordan, thanks for your dedication to this discussion! 🙌

I've tried to understand your position on this and stumbled upon the mention of:

The ideal module/file only default-exports one thing, ideally a pure function. There are always exceptions and justifications for deviating from this, of course - but that's the default I start from.

I'd like to hear a little more about the reasoning behind this if possible. What is the thought process exactly behind this definition?

Fabyao commented 4 years ago

@ljharb I strongly disagree with that statement

Decisions aren't made by voting

Many rules (some more important than named export vs default export) and leaders driving our every day life are decided by voting.

Coming back to our debate about default export, the majority of devs here do not see the benefits of forcing a default export. This is further endorsed by other experts (Nicolas Zakas). To ignore to overwhelming majority against default export is short-sighted (to use your own words).

I am working on a large project with 22 other devs and the benefits of named export are quiet evident. We agreed that the default behaviour of exporting defaults would affect our naming standard. The rule is now off:

rules: { 'import/prefer-default-export': ['off'] }

ljharb commented 4 years ago

@Fabyao in this project, decisions aren't made by voting, and they're also not made at all when they contradict internal airbnb consensus. Until that changes, this stands no matter how many people disagree with it.

There's 35 participants in the thread, and less than 300 emoji reactions on the OP. That doesn't represent a majority among Airbnb's frontend engineers, let alone the wider JS community.

ljharb commented 4 years ago

@halfzebra that's not a definition, just an opinion formed through years of experience with many large and small codebased.

halfzebra commented 4 years ago

@ljharb fair enough! I'm coming from a different development environment and probably many of my experiences are not representative.

I respect Airbnb engineers and it would be valuable to hear about the benefits behind practices this rule encourages. I'm sorry if this looks like an invitation for a debate, I've been furiously advocating against default exports and I'd like to know whether I've made a mistake.

What am I missing?

ljharb commented 4 years ago

@halfzebra the points have been enumerated many times in this thread :-) happy to discuss it further if you want to find me in a chat medium!

halfzebra commented 4 years ago

@ljharb I'm afraid that proposed benefits confuse me more than give an answer 😕

Thanks that would be great! What's the best way to reach out to you?

ljharb commented 4 years ago

Best is probably the node slack, or on freenode IRC.

gaurav- commented 4 years ago

It's amusing to see the same already-addressed points that keep coming back again 🙂

My longer answers are buried in this long thread.

The only new point I'd like to add is the example of a popular library that extensively uses default exports https://github.com/date-fns/date-fns/blob/v2.14.0/src/addDays/index.js#L27

At the end of the day, decide what works best for your project/team and set the rule accordingly. But please also allow for the fact that there are many for whom prefer-default-exports is the preferred default (pun intended), especially the maintainers of this repo.

bradennapier commented 4 years ago

Interesting points when using with Typescript - just as a thought:

  1. Auto Imports are greatly affected
  2. Typescript has auto fixing when moving files around
  3. VSCode has the Rename Symbol feature
slikts commented 4 years ago

So there can't be justified convictions or categorical opinions? You might disagree with the reasons, but they're stated clearly, and it comes down to a judgement call. Replacing judgement with a vote makes it needlessly manipulable, because you can just canvass for votes. Being able to utter "subjective" doesn't entitle you to being right if you don't explain your reasons. Just a reaction to an "attitude" (someone thinking they're right) is not a reason, and it's also not a contribution to the discussion.