Material-ui: Simplify import for complex components

[unional]

Will you consider to support:

import Card from 'material-ui/lib/card';
import List from 'material-ui/lib/list';

let MyCard = () => (
  <Card>
    <Card.Header ... />
    <Card.Media ... />
    <Card.Title ... />
  </Card>
);

let MyCard = () => (
  <List>
    <List.Item>
  </List>
);

along with the current way:

import Card from 'material-ui/lib/card/card';
import CardActions from 'material-ui/lib/card/card-actions';
import CardHeader from 'material-ui/lib/card/card-header';
import CardMedia from 'material-ui/lib/card/card-media';
import CardTitle from 'material-ui/lib/card/card-title';

I remember seeing this recommendation on react.io, but couldn't find the source.

[newoga]

@unional We are currently looking into approaches to simplifying, but don't think this approach is currently planned or has been discussed. The only current component that is doing something like this to the best of my knowledge is <AutoComplete/>.

@oliviertassinari @alitaheri What do you think about this? The only concern I could imagine having is would implementing this approach mean that if you import Card it in effects imports all related components. Maybe that's okay for components like that though when you are likely to use them.

Related to #2679

[alitaheri]

@newoga I'm afraid doing this will force import many unused components leading to increased size of the final build. @oliviertassinari What do you think about this?

I believe we should actually remove the component that are using this approach too. If we want to support orthogonality.

[unional]

If the component are or can be used orthogonally, then of course they should be separate. This only applies to sub-component/sub-section-component that are meant to be used under the main component.

Another (better) example would be Table vs TableBody etc.

[alitaheri]

@unional I see. might not be a bad idea after all. @oliviertassinari what do you think?

[newoga]

@alitaheri I had same concern as you but am starting to agree that this could be a good approach for some of the more specific use cases. I know react-bootstrap uses this approach for some of their nav stuff too.

[newoga]

@unional I guess it's worth mentioning that a similar approach to this is using the * syntax for importing...

import * as Card from 'material-ui/lib/card'

let MyCard = () => (
  <Card.Card> 
    <Card.Header ... />
    <Card.Media ... />
    <Card.Title ... />
  </Card.Card>
);

That should work right now.

[oliviertassinari]

I'm afraid doing this will force import many unused components leading to increased size of the final build.

I agree. For instance, I don't like this line https://github.com/callemall/material-ui/blob/master/src/auto-complete.jsx#L354. It can lead to unused imported files. The three shaking of rollup or webpack has no way to know if the Divider will be used or not.

The solution proposed by @newoga seems to address the issue :+1: .

[alitaheri]

@oliviertassinari I don't think that solution can be implemented if we plan on flattening the folder structure. I think we should flatten the structure, so every component (no matter how tightly coupled) are imported using the exact same pattern material-ui/lib/XXX

[unional]

IMO AutoComplete should not export MenuItem and Divider as it currently is. When designing API, we should ask ourselves what is the intent:

1. Here is AutoComplete, as well as AutoComplete.Divider which is a sub-component that can only be used under AutoComplete, or
2. Here is AutoComplete, as well as AutoComplete.Divider which I provide as I think it would be convenience for you as you would use it in most common scenairos. It is Divider by itself and can be use elsewhere too.

In my world, (2) always lose.
(1) is the proper use case for Table, Table.Body, Table.Row, etc.

[unional]

@newoga , that would work. I would suggest to maintain some form of it after the flattening.

Another way would be:

import {Card, CardHeader, CardTitle} from 'material-ui/lib/card';

[alitaheri]

@unional Hmmmm... You make some valid points!

@newoga @oliviertassinari I have a proposal:

how about we flatten the ENTIRE structire:

import Card from 'material-ui/lib/Card';
import {CardHeader} from 'material-ui/lib/CardHeader';
import {CardTitle} from 'material-ui/lib/CardTitle';

but still provide a way to require all the related components through re-exports:

import {Card, CardHeader, CardTitle} from 'material-ui/lib/card-all'; // or whatever

[unional]

@alitaheri , you may draw some similarity from lodash modern:
https://github.com/lodash/lodash/tree/3.10.1-es
where lodash.js => material-ui or material-ui/index
and each component has its own export, such as array.js => Card.js

import _ from 'lodash'; // Get everything
import array from 'lodash/array'; // Get only array related

[alitaheri]

@unional That's how it is right now. :grin:

[unional]

I see. Do you see the tree-shaking in webpack would become a standard? I'm using jspm, not webpack.

[alitaheri]

@unional I will eventually. But I don't think we should rely on it. If it doesn't work as exactly as expected the output file can become devastatingly big!

[newoga]

@alitaheri I think standard ES6 module syntax is all we need. For example, another approach that currently works is:

// assuming Card is exported by default and CardHeader and CardTitle is normal exported
import Card, {CardHeader, CardTitle}  from 'material-ui/lib/Card';

That would eliminate our need to have some material-ui/lib/card-all import path.

Also, nothing is stopping us from still exporting everything at root material-ui path. I think supporting both is fine and not too hard. :smile:

[alitaheri]

@newoga :scream: :scream: I take my suggestion back it was stupid :laughing:. Good idea :+1: :+1:

I really like it :grin:

@oliviertassinari I think we should follow this, do you agree?

[oliviertassinari]

Well, I think that the best way is to let people use

import {Card, CardHeader, CardTitle} from 'material-ui/lib/card';

or

import * as Card from 'material-ui/lib/card'

In order to do so, I guess that we just need this in our card/index.js

import Card from './card';
import CardHeader from './card-header';
import CardTitle from './card-title';
import CardMedia from './card-media';
import CardText from './card-text';
import CardActions from './card-actions';
import CardExpandable from './card-expandable';

export {Card};
export {CardHeader};
export {CardTitle};
export {CardMedia};
export {CardText};
export {CardActions};
export {CardExpandable};

[alitaheri]

That can't happen if we decide to flatten the folder structure.

P.S. you don't have to re-export the default, this is equivalent and simpler:

export {default as Card} from './card';

[unional]

How about separating export and implementation?
Implementations still sit under material-ui/lib/**, while exporting at top level:

material-ui/TextField.jsx:

export {TextField as default} from './lib/TextField.jsx';

material-ui/Table.jsx:

import {Table} from './lib/Table.jsx';
import {TableBody} from './lib/TableBody.jsx';

export default Table;
export {TableBody};

// Or
export {Table as default} from './lib/Table.jsx';
export * from './lib/TableBody.jsx'

---

Another point to discuss is whether to do default or named export, i.e. export default class TextField ... vs export class TextField ... in the implementation.

In the framework that I wrote, I prefer to only do named export for all internal files. This is because each file _may_ have multiple exports (while violating SRP, some functions are either too small, too parallel in feature (e.g. variations for the same responsibility), too couple, or exposed to facilitate testing), and the consumers of the files many time need to open the file to find out which is exported as default.

---

To sum everything up in one sentence:

Only do export default for the module's single point of entry

In material-ui, each orthogonal component is a module.

[yongxu]

AutoComplete.Divider was a bad design. When I initially wrote this component ThemeManager was still entangled with most components and the everything was imported at once.

Thinking about the old way react addons got imported, I quite like the solution @unional proposed:

import Card from 'material-ui/lib/card';
import Card from 'material-ui/lib/card/card'; //no extra

[unional]

Fyi, tree-shaking may be available in jspm.
https://github.com/systemjs/builder/pull/442

[newoga]

I'm going to close this discussion (it's planned for 0.15.0), if anyone has anymore thoughts we can continue in #2679.