Material-ui: [RFC] Use `import * as Mui from 'material-ui'` in the demos

[o-alexandrov]

• [x] I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

Instead of recommending to use named exports, how about recommending importing all modules?

• it won't increase the bundles and is not worse in terms of initial parsing for development workflow than named imports

Examples 🌈

Before:

import { Button, Table, ... } from "@material-ui/core"

After:

import * as Mui from "@material-ui/core"

Motivation 🔦

Pros:

• improves consistency
  
  
  
  • because a similar decision was made here
  
  
  • material-ui doesn't import named exports from react, why should imports from material-ui be different?
  
  
• improves DX
  
  
  
  • as it removes the need to manually clean up unused imports (when there is a linter rule in development workflow).
  
  
  • as it removes the need to manually add new imports from Mui
  
  
  • as IDEs most of the time fail to auto import from libraries already present in the module
  
  
  • as it is cleaner, since less individual imports are in the module
  
  
  • it is easier to understand where the variable is coming from while looking at the code, as you know the variable in question belongs to Mui
    
    
    
    
    
    • for example: Mui.Button is a pure Mui button, whereas Button is part of your project's modifications that fell outside of what you can do with theme's customizations
    
    
    
  
  
• it's better marketing for material-ui and useful for beginners, as developers are reminded on what they use as they use values from Mui.

Cons:

• the amount of code writing (less lines, but a bit more actual text) is more as there'd be a prefix everywhere: Mui.
  
  
  
  • but the clear indication where these variables are coming from is far more important
  
  

Example project

For your convenience I created a simple example that showcases:

• tree-shaking of material-ui works when importing all

Basically I put @oliviertassinari example function into a new CRA project. Please follow commits as it's easier to understand what has been changed in a pure CRA project.

And here is a screenshot of the webpack-bundle-analyzer. We can clearly see that only the button and what's necessary for it is imported (to see it for yourself, run yarn build):

[eps1lon]

material-ui doesn't import named exports from React, why should imports from material-ui be different?

We ship a bundle with actual ES modules but React doesn't. Import style is personal preference. We won't enforce a particular style.

[oliviertassinari]

The reason why it will improve consistency is because a similar decision was made here.

@o-alexandrov Note that we might revisit how we import React if Facebook adds support for tree-shaking in the future. Until they do, a single import is simple and avoids back and forth with the top of the file, ask Python's developers.

[o-alexandrov]

@oliviertassinari you contradict yourself:

Until they do, a single import is simpler, ask Python's developers.

then why don't you:

import * as Mui from "@material-ui/core"

instead of recommending named imports (based on your own statement it is more complicated), if both lead to the exact same bundle size?

[o-alexandrov]

@eps1lon you also contradict your own statement in this comment and also Olivier comment

[oliviertassinari]

@o-alexandrov Your proposal would break tree-shaking (as far as I know). Assuming no tree-shaking, I don't think that it's an option for the demos of the documentation. Developers with less experience will blame Material-UI for being bloated once they look at the bundle size in production. It also doesn't account for all the new components coming (doesn't scale).

[o-alexandrov]

@oliviertassinari what makes you think it will break tree-shaking?
I am using import * as in every webpack environment and nowhere tree-shaking was ever broken.

But you don't need to trust me, please review the following as tree-shaking for import * as has been supported by webpack for a long time:

• one related issue
• actual example it has been in the description of this Issue btw

[o-alexandrov]

also updated Motivation part of the Issue description, so benefits are clearly highlighted

[oliviertassinari]

I am using import * as in every webpack environment and nowhere tree-shaking was ever broken.

@o-alexandrov Interesting, I didn't know! I wonder about the esthetic.

[o-alexandrov]

It also doesn't account for all the new components coming (doesn't scale).

I don't know about what's coming, but if you are talking about modules with side effects:

• the side effects should introduce the exact same problems with the named imports as all exported modules are parsed

[o-alexandrov]

@o-alexandrov Interesting. I wonder about the esthetic.

Other arguments to consider and will copy them to Motivation above:

• it is cleaner, since less individual imports are in the module
  
  
  
  • it is easier to understand where the variable is coming from while looking at the code, as you know the variable in question belongs to Mui
  
  
• and not only it improves DX when removing unused imports
  
  
  
  • it also improves DX when importing more API from Mui, as IDEs sometimes fail to auto import from libraries already present in the module
  
  

[eps1lon]

@eps1lon you also contradict your own statement in this comment and also Olivier comment

React's exports and Material-UI's exports are not the same. React does not ship ES modules. Material-UI does. Opinions I hold about importing React do not automatically apply to Material-UI.

[o-alexandrov]

@eps1lon please be informed the links to your comments above lead to your phrase:

So we do want to influence them. If we want to follow then we do what we're told. If we want to lead we're doing what we think they want.
What you're describing is incoherent.

That’s what I referred to in the links above and that’s what I described as contradiction when here you say:

We won't enforce a particular style.

But the statement itself is incorrect as by making choices you enforce a particular style anyway.
This issue asks to change your opinion on what you enforce.

[o-alexandrov]

@eps1lon regarding:

React's exports and Material-UI's exports are not the same. React does not ship ES modules.

I don’t know why you bring it up, especially twice.
What does it change in terms of the importing style?

• if you say it makes named imports better, then why? What’s your point?

[oliviertassinari]

@o-alexandrov So if we take this demo: https://material-ui.com/components/buttons/#contained-buttons, it would become:

import React from 'react';
import * as mui from '@material-ui/core';

const useStyles = mui.makeStyles((theme) => ({
  root: {
    '& > *': {
      margin: theme.spacing(1),
    },
  },
}));

export default function ContainedButtons() {
  const classes = useStyles();

  return (
    <div className={classes.root}>
      <mui.Button variant="contained">Default</mui.Button>
      <mui.Button variant="contained" color="primary">
        Primary
      </mui.Button>
      <mui.Button variant="contained" color="secondary">
        Secondary
      </mui.Button>
      <mui.Button variant="contained" disabled>
        Disabled
      </mui.Button>
      <mui.Button variant="contained" color="primary" href="#contained-buttons">
        Link
      </mui.Button>
    </div>
  );
}

and we would still have tree-shaking working. Maybe it's worth asking on Twitter how the community feels about using the approach by default for the demos. 🤔

[o-alexandrov]

@oliviertassinari I apologize, I don't know whether you were asking or making a statement, so for your convenience I created a simple example that showcases:

• tree-shaking of material-ui works when importing all

Basically I put your example above into a new CRA project. Please follow commits as it's easier to understand what has been changed in a pure CRA project.

And here is a screenshot of the webpack-bundle-analyzer. We can clearly see that only the button and what's necessary for it is imported (meaning tree-shaking is working):

[o-alexandrov]

Could you kindly open the issue, as it seems you support further discussion on this topic?
I also added another note to the Pros in the Description of the issue above:

it's better marketing for material-ui and useful for beginners, as developers are reminded on what they use as they use values from Mui.

[oliviertassinari]

Let's see if it resonates with developers: https://twitter.com/olivtassinari/status/1304385408525045760, we can re-close the issue if not.

[oliviertassinari]

Another pros I didn't thought about before. The mui.* pattern makes it easier to identify what's used in the codebase. Say you have multiple custom buttons, searching for mui.Button will give you more accurate results.

We never destruture the theme object in the codebase for this very reason. We always do theme.palette.divider, not const { divider } = theme.palette.

It's important for refactoring.

[oliviertassinari]

It doesn't seem to resonate a lot with the community. It seems that they are mostly used to the following approach:

import { Button, makeStyles } from '@material-ui/core';

On Vue land, they seem to basically make all the components global:

import * as mui from '@material-ui/core';

Object.keys(mui).forEach(key => {
  window[key] = mui[key];
});

https://vuetifyjs.com/en/getting-started/quick-start/#vue-cli-install

And then, some have a smart loader to have tree-shaking.

[o-alexandrov]

I am not surprised pretty much none of the responders there read this GitHub issue 😅

I read all of the responses there:

• people ask: would it hurt bundle size
  
  
  
  • even though with absolutely no modifications to webpack's bundling optimization defaults, tree-shaking works the same for both types of imports
  
  
• some agree and pretty positive about it
• some don't like it due to legacy projects

Personally, I don't think v5 should be held by old inconvenient patterns and fear. You can set a new trend with mui.*

I believe whatever you choose will be the most common pattern, so it's your call, as simple as that.

[o-alexandrov]

Stats based on your tweet:

• 16 different users answered (not including Sebastian and Olivier)
  
  
  
  • 7 undecided: @sandeep0695, @TandonKrishna12, @AndaristRake, @erikras, @thegoldenshun (mentioned only with icons, so I'd say more positive than negative), @arunkumar413, @MelamedYoav
  
  
  • 6 like it: @ee0pdt (likes it, but held with a legacy project), @garbage_value, @alchemist_ubi, @Lani78, @ModularCoder, @kandelborg
  
  
  • 3 don't like it: @pflevy,@MelloGarrett, @dfmartin
  
  

Then, here is @eps1lon (I really hope you'd read the description completely), who seems to not like it due to:

Explicit imports make it easier to spot what is used

And seems like @oliviertassinari also likes the pattern.

[oliviertassinari]

@o-alexandrov I will personally snooze the matter for the next 2+ months. I don't think that we need to rush any changes. We have until v5 land to make bold changes (if we see them appropriate). Regarding the esthetic, Ant Design and react-bootstrap: does that too, e.g.

[o-alexandrov]

I totally understand decisions like this in a popular project are hard to make.
I am already happy you were open to reconsider.

From here, please do as you see best for the community, you know what's best much better than I do.