Skip to content

Instantly share code, notes, and snippets.

@fzaninotto
Created November 8, 2017 21:10
Show Gist options
  • Save fzaninotto/0a59220e0460134de321b415d1418e99 to your computer and use it in GitHub Desktop.
Save fzaninotto/0a59220e0460134de321b415d1418e99 to your computer and use it in GitHub Desktop.
Feedback from migrating a material-ui app to 1.0

I have migrated about 11 kloc from material-ui 0.X to material ui 1.0 (a moderate size project). It has taken me more than 20 hours, and it was not a pleasant experience. cf https://github.com/marmelab/admin-on-rest/pull/1167

No Migration Path

I understand that you took the opportunity of a major BC break to rethink the API completely. But coming from 0.X, the changes often aren't consistent. Each component has changed in a different way, and for a given component, there is no way to guess the 1.0 API based on the 0.x API. Why did <ToolbarGroup> disappear? Why do I need to decompose a ListItem into so many sub components now? Why was Toggle renamed to Switch?

As a consequence, it takes a huge amount of time to migrate a mui 0.x codebase, and it's painful. I wouldn't recommend it to anyone.

I think this should be make clearer in the doc: don't try to migrate a 0.X app to 1.0. Instead, write a new app, it will take less time.

The New API Isn't Really Better

After such an overhaul, I expected that the new material-ui would bring a much better developer experience, but it turns out it doesn't. Sure the 0.x and 1.x APIs are different, but from a developer's point of view, there is no clear winner. Each has its strong points, and its pain points. My point is, there is not enough incentive to motivate people to switch to 1.0 - especially how painful the migration is. I was expecting more from the 1.0, and I'm a bit disappointed.

Documentation Structure Isn't Intuitive

This happens to me all the time: I'm looking for the way to use a given component. the way to use means both an example, and a list of options. But these two items are in different places in the doc, not necessarily under the same section. So for one component, I usually have to click five or six times to find the information I want. Much more than with 0.x

Same for styling and theming: the doc is spread across 3 chapters, I never know where to look for.

The information architecture of the documentation really needs a new look.

Imports Are Not Intuitive

What's the syntax for importing a component? It's impossible to guess in material-ui 1.0.

// sometimes it's
import { ListItem } from 'material-ui/List`;
// sometimes it's
import Divider from 'material-ui/Divider';

When should one use {}, or import directly? There is probably a logic behind it, but I can't find it. As a consequence, every time I have to use a mui component, I have to look at the doc. A big waste of time.

What I would love is to use:

import { ListItem, Divider } from 'material-ui';

Oh, you know what? It's possible! But it's not documented...

JSS Is Verbose

I understand the drawbacks of using direct style props in terms of theming, readbility, ability to override styles, lack of responsive support. I love the features that CSS-in-JS bring.

But the work that JSS demands to the developer in material-ui is reaaaally cumbersome. If I just want to change the color of a div, here is what I have to do now:

import { withStyles } from 'material-ui/styles';

const styles = {
  div: {
    color: 'red',
  }
};

const MyComponent = ({ classes }) => (
    <div className={classes.div}>
      Hello, World!
    </div>
);
export default withStyles(styles)(MyComponent);

Not to mention the extra depths withStyle adds in debugging tools and in enzyme.

When I compare that to 0.x:

const styles = {
  div: {
    color: 'red',
  }
};

export default () => (
    <div style={styles.div}>
      Hello, World!
    </div>
);

I really feel that I've lost in velocity with this solution. I don't know if a better CSS-in-JS solution exists, but this one feels so heavy that I am now reluctant to style my components.

Field Control In withStyles is Cumbersome

When a component uses withStyles, I can pass my own set of classes, and they will override the default component ones:

// in MyComponent.js
import { withStyles } from 'material-ui/styles';

const styles = {
  div: {
    color: 'red',
  }
};

const MyComponent = ({ classes }) => (
    <div className={classes.div}>
      Hello, World!
    </div>
);
export default withStyles(styles)(MyComponent);

// in MyApp.js
import { withStyles } from 'material-ui/styles';
import MyComponent from './MyComponent';

const styles = {
  div: {
    color: 'green',
  }
};

const MyApp = ({ classes }) => <MyComponent classes={classes} />;

export default withStyles(styles)(MyApp);

Great feature.

But it seems that withStyles, when merging the styles from the classes props with the one from the styles parameter, makes a coherence check. Any field from the classes props that is not also defined in the styles parameter is rejected.

This is annoying. It prevents me from passing a single classes object to override the styles of several components:

// in MyApp.js
import { withStyles } from 'material-ui/styles';
import MyComponent from './MyComponent';

const styles = {
  div: {
    color: 'green',
  },
  span: {
    color: 'yellow',
  }
};

const MyApp = ({ classes }) => (
  <span className={classes.span}>
    <MyComponent classes={classes} /> // fails because MyComponent doesn't have a `span` style
  </span>
);
export default withStyles(styles)(MyApp);

Theme Injection In JSS Is a Killer Feature

I really love the ability to use the theme in style functions. It's so much easier than injecting the theme in the component. But, I think that the defautl theme is hard to decode in the documentaiton, so it's often hard to find which theme setting to extend. for instance, what is the path for the default text color?

<Hidden> is Backwards

When I want to display something on mobile, <Hidden> forces me to think the other way around - where I want to hide it. It's a double negation kind of reasoning, and I can't get my mind around it. I always have to open the documentaton for an example, or do a blind test. Even when reading my own code, I can't find which <Hidden> concerns which form factor, so I add a key prop to remember it:

<Hidden smUp key="mobile">

A <Show> component would feel much more natural than a <Hidden>.

<Hidden> Doesn’t Accept More Props

I would love that <Hidden> accepts more props, and just passes them silently to its children. That would allow usage like:

<List
    {...props}
    perPage={6}
    filters={<CommentFilter />}
    pagination={<CommentPagination />}
>
    <Hidden implementation="css" smUp key="mobile">
        <CommentMobileList />
    </Hidden>
    <Hidden implementation="css" xsDown key="desktop">
        <CommentGrid />
    </Hidden>
</List>

Here, <List> fetches data, then passes to its children as data prop. But <Hidden> refuses this prop, and doesn't pass it down. So I had to keep using admin-on-rest's <Responsive> utility, as follows:

<List
    {...props}
    perPage={6}
    filters={<CommentFilter />}
    pagination={<CommentPagination />}
>
    <Responsive 
        small={<CommentMobileList />}
        medium={<CommentGrid />}
    />
</List>

As a consequence, I don't use <Hidden>.

<CardContent> Doesn’t Do Basic Typography

It really feels weird to have to add <Typography> inside a <CardContent> or <CardHeader>, especially since it's not necessary in other components (like <Table> or <ListItem>), nor was it necessary in 0.X. It makes Cards very verbose.

Children vs. Specialized Props: Not Consistent

Some components take children when they used to take label. For instance, <Button>:

// in 0.x
<FlatButton label="foo" />
// in 1.0
<Button>Foo</Button>

But this is not consistent across mui components. For instance <Chip> now expects its content as a prop, while it used children in 0.x.

Children vs. Specialized Props: Major Drawback

Besides, using sub components instead of named props breaks one basic expectation for the React developers - that they can replace one component by another.

For instance, in <ListItem>:

// in 0.x
<ListItem primaryText="Comment" leftIcon={<EditIcon />} />

// in 1.0
<ListItem>
  <ListItemText primary="Comment" />
  <ListItemSecondaryAction>
    <IconButton aria-label="Comments">
      <EditIcon />
    </IconButton>
  </ListItemSecondaryAction>
</ListItem>

This prevents conditional components (e.g. for roles, responsive, etc), because the main component introspects its children, and expects to find a certain muiClass:

// in 0.x
const AdminOnly = ({ role, children }) => role == 'admin' ? children : null;
const Foo = ({ role }) => <ListItem primaryText="Comment" leftIcon={<AdminOnly role={role}><EditIcon/></AdminOnly>} />

// in 1.0, this is not possible
const AdminOnly = ({ role, children }) => role == 'admin' ? children : null;
const Foo = ({ role }) => (
<ListItem>
  <ListItemText primary="Comment" />
  <AdminOnly role={role}>
    <ListItemSecondaryAction>
      <IconButton aria-label="Comments">
        <EditIcon />
      </IconButton>
    </ListItemSecondaryAction>
  </AdminOnly>
</ListItem>

I personnaly think that introspecting children and doing things based on the child type is a bad separation of concerns. It's the child's responsibility to render itself, not the father's. Every time we've used child introspection in admin-on-rest, it had bad consequences.

Generic TextField Is Bad Naming

Material-ui offers one <TextField> component to deal with... dates, numbers, and selects. Very unfortunate naming IMO, because of the 'Text part. That component should have been named <Input>.

Inputs No longer Looks Good By Default

<TextField> no longer has a default width (256px in material ui 0.x). This means much more work for the developer, who has to define and attach classes to every form input for them to look OK. Besides, <TextField> no longer have a default margin normal, so the default layout is ugly.

Persistent Drawer : Fabulous

The work on the drawer component is a major improvement. I had to implement some kind of "persistent" drawer with 0.x, and it was a pain. With 1.x, it's a breeze. Kudos for that!

Defaulting to Native DatePicker Is A Good Choice

I'm not bothered by the new <DatePicker> - it handles localization pretty well. Besides, I think that relying on a string value instead of a Date object removes plenty of localization problems. I suppose the material design date picker will come back - I just hope it uses string values, too.

AutocompleteInput Is Not a Finished Component

The doc gives an exemple implementation of an autocomplete component. Well, it took me about 4 hours to make it work with redux-form.

Autocomplete is a core requirement, and I thing mui should have a working implementation, not a snippet to copy/paste in the doc.

Icon Paths Are Not easy To Find

In mui 0.x, icon paths followed the https://material.io/icons/ structure. So when I saw an icon there, I knew how to import it in mui.

In mui 1.x, icons are all in the same directory. How can I know the path of an icon I see in the visual list of icons? I can't.

No Compulsory Theme Provider: A Big Win For Testability

I love that it's no longer necessary to decorate mui components with a <MuiThemeProvider>. It makes complex component tests with enzyme easier.

@bill-vatomic
Copy link

Hear, hear!

@nickhallph
Copy link

Well said and I agree 100%. This is so backwards

@Jwing28
Copy link

Jwing28 commented Oct 3, 2018

This document is helpful!

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