-
-
Notifications
You must be signed in to change notification settings - Fork 32.3k
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] Make the palette always return new light and dark object #44059
Conversation
Netlify deploy previewhttps://deploy-preview-44059--material-ui.netlify.app/ Bundle size reportDetails of bundle changes (Toolpad) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Look good 👍
expect(palette1.background.default).to.equal('#fff'); | ||
expect(palette2.background.default).to.equal('#fff'); | ||
|
||
palette1.background.default = '#000'; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, we had the same bug since Material UI v1, I guess.
const modes = { dark, light }; | ||
const modes = { dark: getDark(), light: getLight() }; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We should refactor this then? It sounds wasteful to call getDark()
or the other to then throw away the object: #44075.
The issue popup in #43708 when there are multiple calls from
createTheme
.The root cause comes from the
palette.background
is not being recreated every time that thecreatePalette
is called,so if
palette.background.*
is mutated, it affect all of the instances.The fix is to wrap the
light
anddark
object in a function and call them inside thecreatePalette
. Tests added.