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

Make Material-UI a peer dependency? #472

Closed
oliviertassinari opened this issue Apr 16, 2019 · 15 comments
Closed

Make Material-UI a peer dependency? #472

oliviertassinari opened this issue Apr 16, 2019 · 15 comments
Assignees
Labels
enhancement New feature or request wontfix This will not be worked on

Comments

@oliviertassinari
Copy link

What do you think of making Material-UI a peer dependency? You probably don't want people to bundle the library twice. It's what notistack and material-ui-pickers are doing.

@mbrn
Copy link
Owner

mbrn commented Apr 16, 2019

Of course i can. Just one thing. It makes development harder, reason of npm install --no-save @mate... everytime dependencies updated.

But i will do. I can change demo/test page as a another module by using lerna.

@mbrn mbrn self-assigned this Apr 16, 2019
@mbrn mbrn added the enhancement New feature or request label Apr 16, 2019
@oliviertassinari
Copy link
Author

@mbrn I'm not 💯 sure it's the right approach either. A few points to take into account:

  • We should take https://bundlephobia.com/result?p=material-table into account. Something might be wrong with tree shaking, it's a different problem.
  • Bundling multiple versions of Material-UI can create class name mismatches and bundle bloat.
  • It makes installation more painful. It's true.

@eps1lon
Copy link
Contributor

eps1lon commented May 14, 2019

Might be a good idea to specify the peer version of react as well.

@Foosballfan
Copy link
Contributor

Strongly in favour of this, will save ~300KB by not bundling material-ui twice.

@eps1lon
Copy link
Contributor

eps1lon commented May 31, 2019

Strongly in favour of this, will save ~300KB by not bundling material-ui twice.

@Foosballfan You should be able to avoud this by resolving duplicate entries of @material-ui/core in your lockfile.

Bundling @material-ui/core twice can cause actual bugs (due to bundling @material-ui/styles twice).

@Foosballfan
Copy link
Contributor

@eps1lon So you mean remove this entry within material-table dependencies from the package-lock.json manually?

"@material-ui/core": {
          "version": "3.9.3",
          "resolved": "https://registry.npmjs.org/@material-ui/core/-/core-3.9.3.tgz",
          ...
}

@eps1lon
Copy link
Contributor

eps1lon commented May 31, 2019

Only if they don't match i.e. if tables requires 3.x then you should also use 3.x. I can't give any guarantees that 3.x and 4.x in the same bundle works.

@Foosballfan
Copy link
Contributor

Ok great, I made sure they matched exactly in the hope that webpack would treeshake better so no problem there.

Feels like a dodgy fix to edit an auto generated file!

Thanks for the tip

@eps1lon
Copy link
Contributor

eps1lon commented May 31, 2019

Feels like a dodgy fix to edit an auto generated file!

Well flattening trees is pretty hard so yarn will occasionally miss some optimizations. yarn check can help detect those.

@ryancogswell
Copy link

FYI -- someone encountering issues likely due to this (though they didn't provide a full reproduction of the problem so I can't be certain this is the cause):

https://stackoverflow.com/questions/56794892/material-table-styling-is-overiding-all-custom-and-material-ui-styling-and-icon

@ryancogswell
Copy link

@stale
Copy link

stale bot commented Jun 15, 2020

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. You can reopen it if it required.

@stale stale bot added the wontfix This will not be worked on label Jun 15, 2020
@stale stale bot closed this as completed Jun 30, 2020
@WalidKurchied
Copy link

@oliviertassinari Any update on this ? Im currently working with multiple microprojects and was looking into a way to use material ui without bundling it in each project, My initial approach was to use the cdn version of material ui but if its possible to use peer dep that would be much better I'd say. Thank you.

@oliviertassinari
Copy link
Author

oliviertassinari commented Jul 28, 2020

@WalidKurchied I don't have any specific update on this problem. The Material-UI team has its effort on the grid under https://github.com/mui-org/material-ui-x. We are working on an MIT and advanced enterprise data grid there.

@WalidKurchied
Copy link

@oliviertassinari Thank you for letting me know.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

6 participants