Material-ui: [RFC] Should we flatten the folder structure?

[oliviertassinari]

This issue is opened to discuss the following proposal: flattening the folder structure.

The problem

The problem I try to solve is the following. Let's say you want to use the circular progress component. You have to know two things to get the import working.

1. Know the name of the component: CircularProgress.
2. Know the import path: Progress/CircularProgress.

It's unclear to me why CircularProgress is under a Progress folder or why Radio isn't under a Switch folder. There are many more examples in the codebase.

The solution

I think that we can save (2.) complexity by flatting the folder structure. We can make the import path predictable.

import COMPONENT_NAME from 'material-ui/COMPONENT_NAME`;

Going back to the example, we would move src/Progress/CircularProgress.js to src/CircularProgress/CircularProgress.js. People will be able to use the following import:

-import CircularProgress from 'material-ui/Progress/CircularProgress';
+import CircularProgress from 'material-ui/CircularProgress';

Let me know what you think about this proposal! The issue was raised by @neoziro.

[thangngoc89]

Yes please. I have spent hours looking in the docs to know the exact path of the components for importing.

[mbrookes]

Note that there are two possible approaches to this:

1. Flatten the /src dir
2. Flatten at build time

The first has the advantage of simplicity, once it's done, it's done; but the with disadvantage of a large /src dir list.

The second has added complexity (build-time script), but:

• Maintains backwards compatibility with current import sytax (single line named import for multiple related components), whilst also allowing flat imports as described above.
• Maintains an ordered hierarchial /src directory

[oliviertassinari]

@mbrookes I would give a strong +1 for option n°1.

[oliviertassinari]

@mbrookes also, I know you did an experimentation in the direction with https://github.com/mbrookes/material-ui/tree/flat-src-dir/src. It's not exactly what I'm suggesting. I'm suggesting that we keep grouping the tests, TypeScript definitions, source of a single component under the same folder.

[mbrookes]

@oliviertassinari That'll help.

Edit: That should also allow us to maintain backwards compatible imports if desired, as the index.js for the root components can still provide named exports for the sub-components.

[mbrookes]

@oliviertassinari I've updated https://github.com/mbrookes/material-ui/tree/flat-src-dir/src per current suggested structure. Only index.js and index.d.ts in each base component directory is updated. each new subcomponent directory would still needs it's own index files.

[kgregory]

@mbrookes I like that. I'd also lobby for moving svg-icons and test-utils to internal.

[mbrookes]

@kgregory svg-icons I agree. test-utils are documented though: https://material-ui-next.com/guides/testing/

[kgregory]

test-utils are documented though: https://material-ui-next.com/guides/testing/

@mbrookes indeed they are, my bad!

[mbrookes]

All 👍, no 👎, I think it's a go!

[mbrookes]

We're just waiting for https://github.com/mui-org/material-ui/pull/9561 to be merged, rather than creating rebase hell!

[oliviertassinari]

@mbrookes I was waiting on getting approval for the scope package before starting this. Batching the effort will be more efficient for everybody.

[justinryder]

From a simplified import perspective, I like this idea, but from a breaking change perspective, I really don't. I get that the stated goal of this prolonged beta period is to allow for breaking changes, but doing things like moving files around is far from a stable change. I've already run into issues with things like the JSS import moving and having the latest documentation show it in a different place than it was in the MUI vesion we were depending on in an older feature branch. I understand that flattening the directory structure will be a developer usability win in the longterm, but can you please at least wait until just before you release v1 to do it to minimize how often early adopters need to resolve breaking changes?

[oliviertassinari]

@justinryder I believe that the earlier we perform the breaking changes the better. We might release a code mode for this one.

[justinryder]

Oooh, that would be handy. I've never had the chance to use codemod for a dependency update before, but it seems pretty sweet. I'm just glad you've all been doing a great job of documenting breaking changes (and all other changes for that matter) in the release notes so I always have good docs to read through which helps a _lot_ when debugging. 😄

[tony-kerz]

would situations like this be effected?

import Table, {TableBody, TableCell, TableHead, TableRow, TableSortLabel} from 'material-ui/Table'

[oliviertassinari]

@tony-kerz

-import Table, {TableBody, TableCell, TableHead, TableRow, TableSortLabel} from 'material-ui/Table'
+import {
+ Table,
+ TableBody,
+ TableCell,
+ TableHead,
+ TableRow,
+ TableSortLabel,
+} from 'material-ui'

or

-import Table, {TableBody, TableCell, TableHead, TableRow, TableSortLabel} from 'material-ui/Table'
+import Table from 'material-ui/Table'
+import TableBody from 'material-ui/TableBody'
+import TableCell from 'material-ui/TableCell'
+import TableHead from 'material-ui/TableHead'
+import TableRow from 'material-ui/TableRow'
+import TableSortLabel from 'material-ui/TableSortLabel'

[tony-kerz]

in the above case, since the components are strongly related (in the sense that they are so often used together), and if they were nested would appear in the same level, i think the single line import is more intuitive (and tidier), but i get trying to get away from the import path in general.

are you saying that both of the above forms would be supported in the strategy you are considering?

[kgregory]

@tony-kerz If the structure were flattened, each component would be its own importable module (second example), but they would also be offered in an index, allowing for the first example.

[mbrookes]

Although it should be stressed that, just as it does now, the first will import everything.

[tony-kerz]

@kgregory gotcha, works for me
@mbrookes gotcha, but should proper application of tree-shaking techniques strip unused components from the result bundle?

[mbrookes]

@tony-kerz In theory, but my understanding is that it doesn't work very well, possibly due to withStyles, and tree shaking having to assume the potential for side-effects, so not stripping unused imported components.

[Pajn]

I would be very sad to loose import Table, {TableBody, TableCell, TableHead, TableRow, TableSortLabel} from 'material-ui/Table'
Would you consider keeping reexports from the "parents" index files even with the flattened folder structure?

I think the grouping helps the readability quite a lot.

[olee]

@Pajn however this would mean requiring Table (which is always required) would still include all other components if no tree shaking is enabled.
However I also think having some kind of index file would be nice to have like

import Table, {TableBody, TableCell, TableHead, TableRow, TableSortLabel} from 'material-ui/TableComponents'

Though it's also an option to just create a reexport file of required MaterialUi components like this:

// maybe add file alias like '@mui/table' in webpack etc. to import it easily
export * from `material-ui/Table`
export * from `material-ui/TableRow`
export * from `material-ui/TableCell`

which you could then import

[oliviertassinari]

@olee The plan is to make the codemod match reality #11249 and release v1. We can start the discussion for v2 if you want.