[IFI-419] Remove redundant export from generator-metal-clay template

[wincent]

I asked about the convention of doing a double export:

export Thing;
export default Thing;

here:

#1440 (comment)

I just wanted to throw up this PR to have something concrete to use as a basis for discussion about potentially moving away from this pattern. I think it would be desirable in the interests of being "conventional" (down with innovation! viva la convention!).

We'd need to do 2 or potential 3 steps to make this happen:

1. (This PR) Make the generator produce only the default export.
2. In the existing packages, consider emitting a deprecation warning if people use the non-default export.
3. In a future major semver release, remove the non-default exports.

Step 2 there is optional, but would consist of some (potentially tricky) wrapping/subclassing of the exported class that would produce a once-only console.warn in the constructor.

Related: https://issues.liferay.com/browse/IFI-419

[bryceosterhaus]

Just read this article yesterday, thought it might be good food-for-thought in regards to this conversation.

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

[wincent]

@bryceosterhaus: Yeah, I saw that too. I think most of his confusion stems from the fact that the files he's importing from are not named after the thing that is their default export, but it should be. That is, if the file exports a LinkedList class by default, it should be called LinkedList.js and not linked-list.js. For names that you don't control (eg. NPM packages), there's no avoiding the need to look at what they export anyway the first time you use them.

[jbalsas]

Hey @bryceosterhaus, @wincent, I think we tried to standardize around both exports because initially we had plenty some modules doing both, exporting defaults and named and consumers where importing them either way too...

import dom from 'metal-dom';
import {contains} from 'metal-dom';

I think this was early days, as you can see from metal-dom/dom.js and metal-dom/domNamed.js

In clay, I think we keep a pretty tight single-export approach that I assume makes using default exports as convention simpler. Since we're doing that, I assume we wouldn't be hurting tree-shaking in any meaningful way.

I can't remember another reason why we decided to do this dual export, so I'd be fine to revert this decision and follow a deprecation plan like @wincent proposed.

/cc @carloslancha, @julien, @matuzalemsteles, @diegonvs?

[matuzalemsteles]

LGTM! I do not see any problem in following this, I like the idea of ​​following the depreciation plan, I think it's important to issue the warnings so people can prepare.

[julien]

I also remember asking this question a while ago.

As far as I'm concerned, I've read a couple of articles/threads regarding default exports vs named exports:

https://blog.neufund.org/why-we-have-banned-default-exports-and-you-should-do-the-same-d51fdc2cf2ad
https://basarat.gitbooks.io/typescript/docs/tips/defaultIsBad.html
webpack/webpack#706

I prefer named exports, but that's just a personal preference

[wincent]

@julien: There are some pretty good arguments against default exports in those links. I think there are a few positions that we could adopt:

1. Never use default exports.

• Pros: simple rule, easily explained and enforced
• Cons: (minor) even when a module exports a single thing you always have to explicitly extract the desired thing

2. Use default exports whenever you feel like

• Pros: ability to use your own discretion
• Cons: need to use your own discretion 😂

The main other disadvantage I see is brittleness in the face of refactoring: if somebody changes which export is the default (a terrible idea) you rely on your type system, tests, or manual verification to catch the issue; if somebody changes the name of the module or the default export, there is no built-in enforcement that obliges you to update the importing code as well.

3. Use default exports only in the simple case where a module is exporting a single thing.

• Pros: makes for easy importing in many cases
• Cons: some judgement required to prevent abuse (eg. exporting a default object with multiple properties); plus the cons of case 2 apply as well.

Having written a lot of CommonJS, it was pretty natural for me to use style "2" or "3" because that is what seemed to map most closely to the CommonJS way of being able to immediately use the export of a module that only had a single export, but I think a pretty good case can be made for "1" because it is the most fool-proof approach.

[diegonvs]

LGTM!

[carloslancha]

LGTM, can't remember any particular reason to do this. Anyway, if we change it in the generator shouldn't we update all the components to follow this rule and keep consistency?

Thx @wincent !

[wincent]

Anyway, if we change it in the generator shouldn't we update all the components to follow this rule and keep consistency?

Yes, @carloslancha, I think that's right. As I said in my original post, that would be a breaking change though, so probably want to do it in three steps (change generator; issue deprecation warnings; actual remove in a semver-major release).

[matuzalemsteles]

hey guys, as we all agree, I think we can merge this and create an issue so we can track the progress of the steps of adding the warnings and we can start removing it from the next version v3.x.

[matuzalemsteles]

I created issue #1495, to go ahead with it and merge into the 2.x-develop branch.

[carloslancha]

@matuzalemsteles @wincent remember to use convention for the commit messages please.

[matuzalemsteles]

hey @carloslancha, Sorry about that 😅.

[wincent]

Sorry about that @carloslancha. I wrote this one 14 days ago, before I knew about the convention.