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

[styles] Fix react-hot-loader regression #16195

Merged

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Jun 12, 2019

I have spent the afternoon/evening trying to migrate to react-jss's createUseStyles instead of us having to maintain the logic.
However, I'm short on time. It will be more challenging than I first thought. There are a couple of problems to solve, but it will worth it. I have benchmarked a x2 boost when using dynamic style functions.

Instead, I'm going with the "quick-win" approach proposed by @theKashey. Thanks! The logic looks simpler.

Closes #15999

I will continue the react-jss effort migration once this is fixed.

@oliviertassinari oliviertassinari added bug 🐛 Something doesn't work package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. labels Jun 12, 2019
@mui-pr-bot
Copy link

Details of bundle changes.

Comparing: cddc4cf...fa7c146

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core -0.02% -0.01% 318,722 318,659 87,596 87,586
@material-ui/core/Paper -0.09% -0.04% 68,349 68,286 20,370 20,361
@material-ui/core/Paper.esm -0.10% -0.03% 61,642 61,579 19,172 19,166
@material-ui/core/Popper 0.00% 0.00% 28,966 28,966 10,414 10,414
@material-ui/core/Textarea 0.00% 0.00% 5,513 5,513 2,370 2,370
@material-ui/core/TrapFocus 0.00% 0.00% 3,755 3,755 1,581 1,581
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16,016 16,016 5,821 5,821
@material-ui/core/useMediaQuery 0.00% 0.00% 2,529 2,529 1,088 1,088
@material-ui/lab -0.05% -0.01% 139,044 138,981 42,787 42,781
@material-ui/styles -0.12% -0.05% 51,769 51,706 15,344 15,336
@material-ui/system 0.00% 0.00% 14,926 14,926 4,269 4,269
Button -0.07% -0.03% 84,337 84,274 25,711 25,703
Modal 0.00% 0.00% 20,343 20,343 6,682 6,682
colorManipulator 0.00% 0.00% 3,904 3,904 1,544 1,544
docs.landing 0.00% 0.00% 55,232 55,232 13,947 13,947
docs.main -0.01% -0.00% 650,888 650,827 204,998 204,993
packages/material-ui/build/umd/material-ui.production.min.js -0.02% -0.01% 292,119 292,054 83,462 83,452

Generated by 🚫 dangerJS against fa7c146

@theKashey
Copy link

The Web have been fixed again ;)

@oliviertassinari
Copy link
Member Author

One day, we will have this leverage, not yet :). @jonschlinkert is fixing the web!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug 🐛 Something doesn't work package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

useSynchronousEffect is not React-Hot-Loader compatible
3 participants