Material-ui: [RFC] Avoid or "prefer not to use" default exports

[dmtrKovalenko]

Introduction

This discussion was initiated during the migration of eslint configs of material-ui/pickers and convention and code style consolidation. (https://github.com/mui-org/material-ui-pickers/pull/2004)

The problem

The core repository is using export default everywhere as a primary way to export React components. This RFC is about why it is not the best approach – the main problem of the default exports is implicit renames. And this unexpected renames leads to the following problems:

Consistent naming between all files

When export default is used it allows anonymous renames on the importing side:

export default function Modal() {
... 
}

// importing file
import Dialog from '../..'

Here it appeared that the component that has displayName="Modal" and actually is a Modal in the code reads like Dialog which is weird because it has its own defined name. This can also lead to the discrepancy between the component name in the component and display name in the dev tools.

Problems with refactoring

If we have an allowed implicit rename it could be tricky to find all the places when the component is used. And automated refactoring will also do nothing because rename is totally allowed

Make sure that for named exports there is a strong AST link for the rename import { Modal as Dialog } which is simplifying refactoring and make any rename explicit

Poor importing experience

Discoverability is very poor for default exports. You cannot explore a module with IntelliSense to see if it has a default export or not. With export default, you get nothing here (maybe it does export default / maybe it doesn't (¯_(ツ)_/¯). And also it doesn't autocomplete by the component name, while for named exports – it does.

import /* here */ from 'something';

Without export default you get a nice intellisense here:

import { /* here */ } from 'something';

More solid imports section

When there are no default exports through the codebase

import * as React from 'react'; 
import Typography from './somewhere' 
import { util } from './somewhereElse'
import Default, { andSomething } from '../'

Without

import * as React from 'react'; 
import { Typography }  from './somewhere' 
import { util } from './somewhereElse'
import { Default, andSomething } from '../'

And as huge bonus for typescript users: Props importing experience will be much clearer

import { Typography, TypographyProps } from '@material-ui/core'

For now, the only way to import props is:

import Typography { TypographyProps } from '@material-ui/core'

Normal flow of things

Originally the goal of export default (I am 99% sure) was intended to export something when we don't really know what we exporting. The best case of default export power is dynamic examples – when we don't really know which component we need to import and render. Or for example next.js's page loader – when it also doesn't know which pages it needs to process exactly.

When we have a specific object that we need to export – we should name it and use this name consistently through the codebase.

Potential problems

Default exports were loved by react community because of HOC. It was really useful to do

export default withStyles(Component)

It is not possible with named exports, but we could have a convention about naming private components in the file

function _Component() { 

}

export const Component = withStyles(_Component)

[eps1lon]

the main problem of the default exports is implicit renames.

This is not a problem. It causes problem you later describe:

If we have an allowed implicit rename it could be tricky to find all the places when the component is used. And automated refactoring will also do nothing because rename is totally allowed

Types solve this.

Q: Why is renaming not a problem?

There's a boundary between interface and implementation that default exports naturally expose. Let's say I have a file where I need some list. How this list is implemented does not matter when reading the code (i.e. understand what it does). So what I do is pick one list implementation e.g. a linked list. At the call sites it doesn't matter so default imports solve this easily: import List from 'linked-list'. Even better: I can pick an arbitrary package later and don't have to rename all the callsites: import List from 'some-blazing-fast-list'

The package that exports a function never decides the name for the importee. If I import a Select from react-select I'm already omitting naming information. The correct name (following your logic) would be ReactSelectSelect. The point is that we (as an importee) already make arbitrary decisions about the naming boundary. Default or named exports have nothing to do with it.

This issue is later repeated:

When we have a specific object that we need to export – we should name it and use this name consistently through the codebase.

I don't agree. Implementation (code) and interface (name) are separate. I don't want to change every call site just because I switch to a different implementation.

And as huge bonus for typescript users: Props importing experience will be much clearer

import { Typography, TypographyProps } from '@material-ui/core'

For now, the only way to import props is:

That is already the current state of affairs? If not then we can change this without having to forbid default exports.

Discoverability is very poor for default exports. You cannot explore a module with IntelliSense to see if it has a default export or not. With export default, you get nothing here (maybe it does export default / maybe it doesn't (¯_(ツ)_/¯).

In what editor? vscode does this just fine. If I type Typography I get the completion for the module containing Typography by the name of the module: @material-ui/core/Typography.

Originally the goal of export default (I am 99% sure) was intended to export something when we don't really know what we exporting.

I don't think that thought ever came into my mind. If I export default function FancyComponent() {} I know very well what I'm exporting.

Potential problems

You should add that React.lazy "just" works without default exports. For named exports I need to wrap it in an extra promise that doesn't actually resolve to a module but object with a default key. That is not intuitive.

And ultimately: What concrete, current problem are you addressing here?

[dmtrKovalenko]

Exactly this problem:

I don't agree. Implementation (code) and interface (name) are separate. I don't want to change every call site just because I switch to a different implementation.

It is not about interface and implementation it is different. There is nothing about interface and abstractions at all here, the only name – and this name should be consistent through the codebase.

I don't think that thought ever came into my mind. If I export default function FancyComponent() {} I know very well what I'm exporting.

You don't. And a developer who is working with your file – don't know as well. If you have a function DoX and you are using it as DoY – isn't it a problem?

[oliviertassinari]

@dmtrKovalenko Thanks for taking the time to write this detailed RFC!

From my perspective the current filename and export convention on the codebase is free from the problems mentioned for components. For non-components modules, I believe we encourage named exports, but I'm not sure, I have never put it a lot of thoughts as opposed to the components:

Consistent naming between all files

We solve this by enforcing the name of the file to match the name of the export and import. It's strict equality between these 3 items, nothing else is allowed. For developers consuming the library, they have the choice to use the barrel index or nested import. We also have an eslint rule that forbids the default export to match the name of a named export.

Problems with refactoring

Implicit rename isn't allowed, so free from this problem, regex works.

Poor importing experience

We solve this problem by forcing each component to be hosted on a single file. There is only the default export that is relevant.

More solid imports section

This argument seems to be a matter of taste, I don't see any pros or cons in any direction?

---

Reading on Twitter a thread on this matter, I find myself to agree with the vision of Jordan Harband: https://twitter.com/ljharb/status/1084131812559843328, which isn't surprising considering he's behind the eslint rules we use (airbnb). These rules ask for a default export when there is only one named export.

[dmtrKovalenko]

My question is: why we need to invent some conventions, create rules -- if we have an already name for that. And we don't need anything more?

Regarding imports section:

Doesn't this feel strange for you

import Typography, { TypographyProps } from "."

And this?:

import Text, { TypographyProps } from "."

[oliviertassinari]

@dmtrKovalenko I think that it's worth taking a step back into the history and see why the current structure of the repository. The current structure is designed so developers can find components and navigate the code as easily as possible. We achieved it with a couple of choices:

1. All the modules are flattened, one exported module, one file or one folder at the root of the src. The only leverage for organizing is the default ASC sorting of the file system. In the past, we used to nest modules. Treating each module is it could be its own package was one of the best changes of v1.
   For instance, this means that we need to move https://github.com/mui-org/material-ui-pickers/blob/dfbbe65d7ba4f858a2e660c19d49ab12f5e4fe87/lib/src/views/Calendar/Day.tsx under /src.
2. We organize the files so it can be easily deleted. In order to achieve this, we keep the test, source, types at the same location.
   For instance, this means that we need to move https://github.com/mui-org/material-ui-pickers/blob/next/lib/src/__tests__/DatePickerTestingLib.test.tsx right next to DatePicker.tsx. So for far, we have been using a folder for the components as we have 4 files. When we have only 2 files, so far, we don't create folders.
3. We export one and only one component per file. We name the file after the name of the public component, this ensures developers can find the source easily, without guessing.
   For instance, this means that we need to move https://github.com/mui-org/material-ui-pickers/blob/dfbbe65d7ba4f858a2e660c19d49ab12f5e4fe87/lib/src/DatePicker/DatePicker.tsx#L69 into its own file.
4. We forbid all circular dependencies.
   For instance, this means that we need to break this cycle: https://github.com/mui-org/material-ui-pickers/blob/dfbbe65d7ba4f858a2e660c19d49ab12f5e4fe87/lib/src/LocalizationProvider.tsx#L4 <-> https://github.com/mui-org/material-ui-pickers/blob/dfbbe65d7ba4f858a2e660c19d49ab12f5e4fe87/lib/src/_shared/hooks/useUtils.ts#L3.

Regarding the named export vs default exports, in your example, a. vs b.

a.

import Typography from '@material-ui/core/Typography';
import Typography, { TypographyProps } from '@material-ui/core/Typography';

b.

import { Typography } from '@material-ui/core/Typography';
import { Typography, TypographyProps } from '@material-ui/core/Typography';

Things I find valuable from a.:

• There is no room for mistake, you can only import one component from the location. It creates a healthy constraint.
• Unless you want to extend the component (10% of the cases?) and you use TypeScript (50% of the user base), so in 95% of the time, you will only use the default export, it's saving you the need for the developers to write {}, which adds visual noise. It also communicates that there isn't anything else to pay attention to, take the default export, you are done.
  
  
  
  • When you need to rename the file (e.g. you are going to write your own wrapper, for instance to MuiTypograpgy) you don't need to duplicate Typography twice. Here is how it looks like with b.
  
  

import { Typography as MuiTypography } from '@material-ui/core/Typography';

and a.

import MuiTypography from '@material-ui/core/Typography';

Things I find valuable from b.:

• You don't need to rewrite the import if you swap the target between '@material-ui/core' and '@material-ui/core/Typograpghy'. But is it something we should care about?
• Else?

---

This was for the public API of components. For internal usage and non components, I like how the current eslint rule is pragmatic. If you have a single export: default export. If you have multiple exports, you choose. If one export accounts for 80% of the value of the file, keep a default export, otherwise, flatten it with named exports, no default exports.
I believe it's what we do so far. In the case of components, 80% of the value is with the exported component, anything else is secondary.

[eps1lon]

I don't see any concrete problems in this issue. I can only see hypotheticals that I don't agree with.

[dtassone]

1. Let's say you have a filename something.tsx

then 2 files app.tsx and component.tsx, that imports it.

import something from 'something';

At a later stage you decide to refactor something and it becomes another.tsx, then you introduce another something.tsx that you need to use in app.tsx only.

As you use the refactoring feature of your IDE, your import in app.tsx becomes

import something from 'another';

So you rename something manually and add your other import, so app.tsx ends up like

import another from 'another';
import something from 'something';

But then components.tsx would still look like

import something from 'another';

So you would have to go and rename the variable manually. As everything works and compiles without error, you forget and when you come back to the body of components.tsx, you don't understand what is happening with something, until you realise that somethingis actually another.

if you use named imports, the issue above will not happen as your refactoring tool will automatically rename the variable used in in the import

import {another} from 'another.tsx' all done and no discrepencies.

Thus Default import would allow more discrepancies in the code. Named import is more strict and is more reliable with refactoring tools and IDE.

1. Using default to re-export a component is bit useless as you would have to name it anyway.

export {default as something} from 'something'
vs
export {something} from 'something'
export * from 'something'

If you rename something, to another, you will still export it as something.

export {default as something} from 'another'
vs
export {another } from 'another'
export * from 'another'

I'm in favor of using a really limited amount of default for the reason described above.
That said, it can be valuable to use it, in the highest level of exports
import XGrid from 'material-ui/x-grid'

• If I rename my exported component in the library then it won't create a breaking change for the consumer.

default exports were popular in CommonJs with module.exports = something and with require
const something = require(something) instead of const something = require(something).something

[oliviertassinari]

In my previous comment, I was trying to cover why the public API looks like what's now. It's the public API that has influenced the current eslint rules. These constraints are quite aligned with Airbnb's rule, so we kept it for internal usage too, so we don't have special treatments. I don't think that we have put special thoughts into using Airbnb's rule outside of the realization that we could trust them and offload the need to have discussions like this one (same value proposition as prettier).

Considering that the discussion, at its root, is about the eslint rule that fails (If a file has a single named export, it tells to move to a default export. If you have multiple exports, it's then up to you.). We can look at why this rule exists in the first place, from their creators:

Why? To encourage more files that only ever export one thing, which is better for readability and maintainability.

https://github.com/airbnb/javascript#modules--prefer-default-export

I have to admit that I still struggle to navigate the codebase of the pickers repository, and I think it's one of the reasons why (different conventions that have the potential to be marginally better but can kill productivity when you are not used to them). I think that we should keep this rule to encourage purposed files, no kitchen sinks by mistake, to ease the exploration of a codebase you don't know (e.g. code you wrote 2 months ago).

@dtassone Interesting consideration, I have never used a refactoring tool, and probably why I'm not fully in empathy with the concern. I think that code review can solve the problem.

This makes me think of a different file name convention that we need to update: name casing. For instance: https://github.com/mui-org/material-ui-x/blob/master/packages/grid/x-grid-modules/src/components/column-headers.tsx should be ColumnHeaders.tsx. The filename should match the default export name if there is one, or be snackCase if has a bunch of named exports.

---

So, I think that the rule should be:

If you have a single export: default export. If you have multiple exports, you choose. If one export accounts for 80% of the value of the file, keep a default export, otherwise, flatten it with named exports with no default exports.

[oliviertassinari]

Still, I think that we should ask everybody as it impacts the whole codebase, across components (we would need to migrate the main repository if we forbid default exports) cc @mui-org/core-team, @DanailH, @PaulSavignano.

The vote is between:

• 🎉 emoji for never using default exports, unless rare exceptions (proposal, see the description of the issue for more details against 👍 in https://github.com/mui-org/material-ui/issues/21862#issue-662009933 and https://github.com/mui-org/material-ui/issues/21862#issuecomment-662429002).
• 👍 emoji for enforcing a default export if the file has a single named export (current status explained in https://github.com/mui-org/material-ui/issues/21862#issuecomment-661351061, see https://github.com/mui-org/material-ui/issues/21862#issuecomment-662515143 for arguments against 🎉)

[dtassone]

@dtassone Interesting consideration, I have never used a refactoring tool, and probably why I'm not fully in empathy with the concern. I think that code review can solve the problem.

Refactoring tools are great and are very helpful, that's why I use webstorm :)
Renaming something across all the repo takes 5 secs, and has no errors.
If you do it manually, time for each build, a few code review iterations, and it's not even guaranteed to be perfect.

To encourage more files that only ever export one thing, which is better for readability and maintainability.

it's not the case if you allow 🤔

import Typography, { TypographyProps } from '@material-ui/core/Typography';

If we just want to encourage 1 component per file. Then it can be a practice.
And enforcing the practice, can be done differently.

A rule could be to default export components that will be consumed by users to avoid breaking changes if we rename them.

IMHO inside the component unit of work, exporting as default, hooks or types could result in a big mess for the reasons mentioned in my first comment. This obviously depends on the complexity of the component.

[rosskevin]

We refactored all of our internal code and removed default exports almost two years ago. It was a positive change and identified export name conflicts easily (coupled with export * from).

[ryancogswell]

The arguments for this related to refactoring/renaming don't seem very relevant. The vast majority of the components with default exports are part of the public API which means they can only be renamed with the release of a major version -- and even then it is rare. Doesn't seem like something worth optimizing for.

[ryancogswell]

@dtassone I can understand how renaming considerations would be more important in an application codebase, but given the requirement of stability of names in Material-UI, can you explain more why this would be important in this case. The main time when renaming may have some importance is when developing new components, and even then it would only add much value for complex components with many sub-components. I suppose your current work on the data grid would fall into this category and maybe the future work for enterprise components would have additional things like this, I just don't see it adding value (from a refactoring standpoint) for the existing components and even for new components it only adds value during their initial development stages. Once an initial release of components is in use, I would expect only rare name changes.

[eps1lon]

Can somebody explain to me why I have to rename every callsite if I rename the implementation? Doesn't the vscode refactor also keep the import name i.e.

// components.tsx
-export function Button() {}
+export function FancyButton() {}
// app.tsx
-import { Button } from './components' 
+import { FancyButton as Button } from './components' 

?

[dmtrKovalenko]

@eps1lon Because when you read the code

<FancyButton /> 

You probably would like to search "FancyButton" and open the component source, right?

But with default exports, you are doing exactly the same, but 2 times. Default exports work exactly like this

// component.js
export const default = FancyButton

import { default as Button } from './component'

[eps1lon]

You probably would like to search "FancyButton" and open the component source, right?

99% of the time the answer is no. The remaining 1% I do CTRL+Click in vscode and it goes to the declaration.

[dmtrKovalenko]

99% of the time the answer is no. The remaining 1% I do CTRL+Click in vscode and it goes to the declaration.

I have nothing to answer here... 99% of developers would like to have this ability because they are reading code not only in the editor. Moreover, core repo is written in typescript so ctrl+click follows to the typescript declaration file 🤷‍♂️

[eps1lon]

reading code not only in the editor

What other methods are you trying to optimize?

[dmtrKovalenko]

I have described everything in RFC – I`d like to get rid of unnecessary implicit rename of the exported component in each file in favor of defined, unique, and easy for search/debugging component name.

[eps1lon]

I have described everything in RFC

I've read it again and I can't find mentions of the way you consume and work with code. Maybe I missed it. Could you explain it again or link/quote it?

I`d like to get rid of unnecessary implicit rename of the exported component in each file in favor of defined, unique, and easy for search/debugging component name.

I don't understand how named exports ensure a unique name across the codebase. Even with named exports I could still do import { FancyButton as Button } from './FancyButton' which would lead to the same problems as import Button from './FancyButton'. That's why editors implement "goto definition" and "find all references"

Moreover, core repo is written in typescript so ctrl+click follows to the typescript declaration file 🤷‍♂️

Could you explain more about your workflow where checking out the component source is so important to you? Seems like we can improve some areas of our documentation.

[rosskevin]

I don't understand how named exports ensure a unique name across the codebase

Try exporting duplicate names, your tooling will pick it up and fail the build e.g. tsc or roll-up.

Using export * from ensures consistency of the public API and the source code. I think we can agree that is a positive attribute for beginners and experts alike

[ryancogswell]

What this seems to come down to is that we are split fairly evenly between developers who work in ways where we don’t experience the problems this is trying to solve and therefore don’t see any benefit to the change, and another group using different habits, tools, refactoring approaches, etc. that see a clear benefit to the change.

I don’t think the 🎉 group has much chance of convincing the 👍 group that it will provide these same benefits to the 👍 group who have different work habits than the 🎉 group. So I think the main question should be “are there enough downsides to this change to deny the benefits of this change to the 🎉 group?”

My biggest concern is the impact outside of Material-UI. This is a change that would affect every single Material-UI import in many codebases. Obviously there would need to be a codemod to help with the change, but it still will seem — to a large portion of library users — like an arbitrary change with no benefit that forces slightly more verbose syntax. There will also be a portion of users that are pleased with the change (based on the sampling here), but I suspect the annoyed group will be significantly louder than the pleased group. Code samples on hundreds of stackoverflow questions will no longer work in v5 without changing the imports.

These aren’t massive problems, but it will be a nuisance to a large number of people and it will be a barrier to upgrading. Upgrading will feel a lot riskier if you have to change nearly every file that uses Material-UI (even if that change is automated).

[DanailH]

I agree with @ryancogswell . If I look at the problem from a user/dev point of view I would like to have as staying forward migration from major to major version as possible. Having to change my exports would definitely be a pain especially if the reason why I'm migration from major to major is to use a specific feature, I would like to deal with the additional hassle of trying to figure out why my code isn't compiling.

I'm not saying that this change isn't worth it but rather that we should keep in mind that we will introduce additional effort when migrating.

PS: In my current company we solved a similar problem by having a specific file that exposes the public API for our npm modules. In addition, we have an npm barrow collection that combines all those components with their exported public APIs and puts them at the same level basically so you aways export everything from the same location. I'm not saying that this is good but that it can be solved on a packaging level. Basically we have an adaptor that sits between the codebase and the users and ensures that whenever we change something the public API is always respected and we don't introduce breaking changes by renaming files or changing exports.