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] Use react-jss hook API internally #16180

Closed
wants to merge 1 commit into from

Conversation

oliviertassinari
Copy link
Member

@oliviertassinari oliviertassinari commented Jun 12, 2019

As we want to move toward a style adapter architecture (JSS, styled-components, etc) #16238, the more we can depend on the existing style solutions, the better. @kof and his team have done a great job with their new react-jss hook API. it's x2 faster for dynamic props than what we have, and only ~25% slower than styled-components. It should solve most of the problems in

@oliviertassinari oliviertassinari added the package: styles Specific to @mui/styles. Legacy package, @material-ui/styled-engine is taking over in v5. label Jun 12, 2019
@mui-pr-bot
Copy link

mui-pr-bot commented Jun 12, 2019

Details of bundle changes.

Comparing: 4909bd8...66e8963

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core 0.00% 0.00% 318,982 318,982 87,573 87,573
@material-ui/core/Paper 0.00% 0.00% 68,281 68,281 20,353 20,353
@material-ui/core/Paper.esm 0.00% 0.00% 61,578 61,578 19,133 19,133
@material-ui/core/Popper 0.00% 0.00% 28,968 28,968 10,411 10,411
@material-ui/core/Textarea 0.00% 0.00% 5,513 5,513 2,374 2,374
@material-ui/core/TrapFocus 0.00% 0.00% 3,755 3,755 1,580 1,580
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 16,012 16,012 5,791 5,791
@material-ui/core/useMediaQuery 0.00% 0.00% 2,597 2,597 1,102 1,102
@material-ui/lab 0.00% 0.00% 140,580 140,580 43,454 43,454
@material-ui/styles 0.00% 0.00% 51,703 51,703 15,337 15,337
@material-ui/system 0.00% 0.00% 15,303 15,303 4,342 4,342
Button 0.00% 0.00% 84,279 84,279 25,630 25,630
Modal 0.00% 0.00% 20,345 20,345 6,689 6,689
Slider 0.00% 0.00% 74,681 74,681 23,221 23,221
colorManipulator 0.00% 0.00% 3,904 3,904 1,544 1,544
docs.landing 0.00% 0.00% 55,232 55,232 13,946 13,946
docs.main -0.94% -0.81% 651,051 644,923 205,054 203,399
packages/material-ui/build/umd/material-ui.production.min.js 0.00% 0.00% 292,226 292,226 83,498 83,498

Generated by 🚫 dangerJS against 66e8963

@oliviertassinari oliviertassinari force-pushed the styles-update branch 2 times, most recently from 2b3f2e6 to 1ea1d6b Compare June 12, 2019 16:04
@oliviertassinari
Copy link
Member Author

I will resume it later.

@joacub
Copy link

joacub commented Aug 17, 2019

@oliviertassinari what happens ? This not work correctly several empty tags classes don’t match and more not it’s only performance , don’t work box and others

Any plans for fix this or not ?

@oliviertassinari
Copy link
Member Author

oliviertassinari commented Aug 17, 2019

The long term priority is to leverage styled components. We will start to hide @material-ui/styles to the end users. So while this change is important, it's middle term oriented. If we can avoid any time investment in it, that would be perfect.

@joacub
Copy link

joacub commented Aug 17, 2019

I understand the problem it’s the system don’t work if you want to use Box this don’t match ssr with the client never.

Btw thanks for the response .

@oliviertassinari
Copy link
Member Author

They shouldn't be any SSR mismatch. It's very likely a miss configuration in the projet your are working with.

@joacub
Copy link

joacub commented Aug 17, 2019

I use the default test and the server render has a different result, this it’s becouse the useStyles it’s called two times on the first render so the first time match but after don’t match becouse it’s execute two time on the first render so react don’t rehydrate the class

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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants