Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build as a multi-repository #7839

Closed
avocadowastaken opened this issue Aug 19, 2017 · 8 comments
Closed

Build as a multi-repository #7839

avocadowastaken opened this issue Aug 19, 2017 · 8 comments

Comments

@avocadowastaken
Copy link
Contributor

avocadowastaken commented Aug 19, 2017

Lot of us currently using stable version and it would be much easier to migrate each component separately.

So at the start we will be able to load components separately:

import { Card, CardText, CardActions } from 'material-ui';  // 0.19.0
import { Button } from "@material-ui/button"; // ^1.0.0

Idea from material-components.

Where user can install whole lib from material-components-web repo or each component separately from @material/*.

See lerna.

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 19, 2017

I have been giving some thought about this option in the past. I have even migrated one of my repositories to lerna. My conclusion was: it doesn't worth it for Material-UI.

Pros:

Cons:

  • Lose the shorter import option import { A, B, C } from 'material-ui' as a user. While it requires making sure tree shaking work, it's making the code more readable.
  • Extra release complexity as a maintainer.

Other:

  • Can allow splitting dependencies (I needed to split react & react-native for react-swipeable-views).

Unless I'm missing something, I haven't changed my mind yet.

@avocadowastaken avocadowastaken changed the title Build v1 as a multirepo Build v1 also as a multirepo Aug 19, 2017
@oliviertassinari
Copy link
Member

Oh, and we already have some other packages:

  • material-ui-icons
  • material-ui-codemod
  • eslint-plugin-material-ui

@avocadowastaken
Copy link
Contributor Author

Lose the shorter import option import { A, B, C } from 'material-ui' as a user. While it requires making sure tree shaking work, it's making the code more readable.

Sorry I wasn't clear about this part.

Of course you should not remove material-ui repo itself, it just would be as another package that will reexport each component.

See material-components-web@0.17.0/index.js.

@oliviertassinari
Copy link
Member

oliviertassinari commented Aug 19, 2017

@umidbekkarimov Doesn't this index requires a more complex tree shaking solution?

Also, Lodash is doing something similar. I have never understood the motivation, I have always thought it was doing more harm than good to the community as increasing the module duplication likelihood for some unclear advantage. On the other hand, this seems to fit well for Babel needs.

@oliviertassinari oliviertassinari changed the title Build v1 also as a multirepo Build as a multi-repository Aug 19, 2017
@avocadowastaken
Copy link
Contributor Author

Doesn't this index requires a more complex tree shaking solution?

Side note: This PR webpack/webpack#5435 will kill all Tree Shaking issues of material-ui, of course with few changes in index.es file. (rip babel-plugin-direct-import)

Lodash is doing something similar. I have never understood the motivation

I agree, it was overkill in case of lodash.

I have always thought it was doing more harm than good to the community as increasing the module duplication likelihood for some unclear advantage.

🤔 I only see the benefits in seamless migration to v1 (but as you said it can be done with yarn).

@oliviertassinari
Copy link
Member

I'm closing the issue, as always in OSS a no is temporary. We will see if more voices are raised to go into this direction. Right now, I don't think that that change is worth the effort as a maintainer. I would rather see some other issues prioritized. Thanks for the discussion, it's a good seed 👍 .

@oliviertassinari
Copy link
Member

oliviertassinari commented May 10, 2018

I went back to this issue. It's haunting me. We are now releasing 5 different packages. They are hosted under the /packages folder. We have absorbed most of the maintenance overhead of such change. I have tried to perform critical thinking on this issue a second time. Again, I couldn't see this change being a win for us. How I see it 6 months later.

The pros:

  • It's a safer feeling. You know you are not going to import 1 MB of JavaScript, no matter what. It provides a sense of scope safety.
  • more ideas?

The cons:

  • After [RFC] Should we flatten the folder structure? #9532, it would make the import path harder to get right. It would require more back and forth with the documentation.
  • It's increasing the npm overhead, slower to download, more room for mistakes (more Degree of Freedom + Murphy's law). We will most likely move all the packages at once no matter what. It's what Babel do for the operational simplicity it provides.
  • It's making the onboarding experience harder. You have installed one component and you are happy with it? Great, let's add another one! Oh, wait, an extra step. I need to npm install it.

@oliviertassinari
Copy link
Member

That being said, #6218 could be a good candidate for multi-repository, to evaluate.

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

No branches or pull requests

2 participants