Skip to content

Conversation

@mkermani144
Copy link
Contributor

@mkermani144 mkermani144 commented Mar 18, 2019

Related to #10778

Memoizing context value is another solution to https://reactjs.org/docs/context.html#caveats

Copy link
Member

@eps1lon eps1lon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's currently an issue with shallow rendering + forwardRef + hooks. We need to adjust our test. I have prepared something locally which also improves this test as a whole unless you insist on doing this yourself.

The implementation is fine as-is.

@mui-pr-bot
Copy link

mui-pr-bot commented Mar 18, 2019

Details of bundle changes.

Comparing: 02b6dab...7e18acd

bundle parsed diff gzip diff prev parsed current parsed prev gzip current gzip
@material-ui/core +0.01% 🔺 +0.02% 🔺 357,579 357,618 91,073 91,093
@material-ui/core/Paper 0.00% 0.00% 68,634 68,634 19,979 19,979
@material-ui/core/Paper.esm 0.00% 0.00% 62,366 62,366 19,060 19,060
@material-ui/core/Popper 0.00% 0.00% 30,454 30,454 10,525 10,525
@material-ui/core/styles/createMuiTheme 0.00% 0.00% 17,390 17,390 5,730 5,730
@material-ui/core/useMediaQuery 0.00% 0.00% 2,471 2,471 1,042 1,042
@material-ui/lab 0.00% 0.00% 152,165 152,165 44,548 44,548
@material-ui/styles 0.00% 0.00% 53,839 53,839 15,566 15,566
@material-ui/system 0.00% 0.00% 17,143 17,143 4,522 4,522
Button 0.00% 0.00% 90,924 90,924 26,941 26,941
Modal 0.00% 0.00% 84,711 84,711 25,208 25,208
colorManipulator 0.00% 0.00% 3,234 3,234 1,301 1,301
docs.landing 0.00% 0.00% 51,843 51,843 11,349 11,349
docs.main +0.01% 🔺 +0.01% 🔺 649,999 650,044 201,578 201,596
packages/material-ui/build/umd/material-ui.production.min.js +0.01% 🔺 +0.02% 🔺 305,844 305,881 83,862 83,876

Generated by 🚫 dangerJS against 7e18acd

@eps1lon eps1lon self-requested a review March 18, 2019 08:01
@eps1lon eps1lon changed the title [List] Use useMemo for context value in order to prevent extra re-renders [List] Memoize context value Mar 18, 2019
@eps1lon eps1lon added performance scope: list Changes related to the list labels Mar 18, 2019
@oliviertassinari
Copy link
Member

oliviertassinari commented Mar 18, 2019

@mkermani144 It's a great first pull request on Material-UI 👌🏻. Thank you for working on it!

@henrylearn2rock
Copy link

Any chance of merging to 3.9.x? Thanks

@ryancogswell
Copy link
Collaborator

Any chance of merging to 3.9.x?

@henrylearn2rock Sorry, but that won't be possible since the solution requires the useMemo hook, and 3.9.x doesn't require a version of React that supports hooks.

@oliviertassinari
Copy link
Member

@henrylearn2rock v4.0.0-beta.0 is in good shape. You should be able to migrate, starting today. If you want to be more cautious, you can wait a week or two.

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

Labels

performance scope: list Changes related to the list

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants