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

@material-ui/styles duplication issue #15264

Closed
timaxxer opened this issue Apr 8, 2019 · 15 comments · Fixed by #15422
Closed

@material-ui/styles duplication issue #15264

timaxxer opened this issue Apr 8, 2019 · 15 comments · Fixed by #15422
Labels
package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. priority: important This change can make a difference

Comments

@timaxxer
Copy link

timaxxer commented Apr 8, 2019

On v4.0.0-alpha.7 release, dependency for @material-ui/styles was not aligned in package.json

When importing both core and styles v4.0.0-alpha.7 on my project, my ThemeProvider is not taken in consideration.

@oliviertassinari
Copy link
Member

@timaxxer Do you bundle two different versions of @material-ui/styles?

@timaxxer
Copy link
Author

timaxxer commented Apr 8, 2019

In your package.json for core alpha7, you have dependency on styles alpha6. If I use styles alpha 7 in my project, the Theme context provider doesn’t push down the right context to your style alpha 6 dependency.

I fix my current problem by using style alpha 6. Ionly pointed the fact that maybe poeple using both core alpha7 and styles alpha 7 will have theme problem

@oliviertassinari
Copy link
Member

We are using ^ as the dependency range it maches more recent versions. It shouldn't be an issue.

@timaxxer
Copy link
Author

timaxxer commented Apr 8, 2019

Effectivly, I’ll try to figure the problem, thanks for the quick response

@JeromeFitz
Copy link
Contributor

@timaxxer I was running into this conflict too and not understanding why this was happening yarn list @material-ui/styles:

├─ @material-ui/core@4.0.0-alpha.7
│  └─ @material-ui/styles@4.0.0-alpha.6
└─ @material-ui/styles@4.0.0-alpha.7

So I ended up removing @material-ui/styles from my package.json, getting rid of node_modules and re-installing @material-ui/core which resulted in yarn list @material-ui/styles:

└─ @material-ui/styles@4.0.0-alpha.6

Not sure why @material-ui/core is not picking up 4.0.0-alpha.7 in its package.json per @oliviertassinari :

"@material-ui/styles": "^4.0.0-alpha.6",

We may need to keep them explicit on next releases?

@eps1lon
Copy link
Member

eps1lon commented Apr 9, 2019

@JeromeFitz You can run yarn check to detect packages that can be deduplicated. I would usually recommend manually patching the lockfile instead. We recently did this with our own. In case you need some inspiration: #15260. You could also try out yarn-deduplicate. yarn v2 hopefully improves a lot of these edge cases.

@JeromeFitz
Copy link
Contributor

Thank you @eps1lon .

I tend to always forget removing yarn.lock when going the route of throwing node_modules out (and not the first time with this alpha 😆 ).

Did those steps and am back in business with explicitly having @material-ui/styles in the package.json.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 9, 2019

Another strategy is to remove the yarn.lock and to run a new installation. I use this strategy all the time. It's what I find the simplest without any significant downsides.

@eps1lon
Copy link
Member

eps1lon commented Apr 9, 2019

It's what I find the simplest without any significant downsides.

This is a YOLO approach. Don't do this in production unless you actually want to vet ~2k line changes.

@oliviertassinari
Copy link
Member

😆. If you have a great test suite, the capability to monitor issues and to quickly revert changes, it should be fine.

@oliviertassinari
Copy link
Member

I was careful with the release of 4.0.0-alpha.8. I have increased the dependency to all the packages to alpha.8

@oliviertassinari oliviertassinari added priority: important This change can make a difference package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. labels Apr 20, 2019
@oliviertassinari oliviertassinari changed the title [ ThemeContext ] Dependency not aligned @material-ui/styles duplication issue Apr 20, 2019
@oliviertassinari
Copy link
Member

I'm adding a warning as well as a FAQ about the problem.

@oliviertassinari
Copy link
Member

oliviertassinari commented Apr 20, 2019

The problem was reported multiple times. The pain is not likely to go away in the future. I would rather address it early. It's a good reminder that bundling everything in a single package is simpler. Entropy combined with Murphy's law will always hit you hard!
However, the separation of @material-ui/core with @material-ui/styles is an important step forward for us. I'm gonna apply the same solution than styled-components: styled-components/styled-components#1412.

@JeromeFitz
Copy link
Contributor

Thank you very much as always @oliviertassinari 💯

@sivaragu

This comment has been minimized.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. priority: important This change can make a difference
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants