[theme] Convert transitions from object to ES module #22517
Conversation
| @@ -6,7 +6,7 @@ export { default as createStyles } from './createStyles'; | |||
| export { default as makeStyles } from './makeStyles'; | |||
| export { default as responsiveFontSizes } from './responsiveFontSizes'; | |||
| export { default as styled } from './styled'; | |||
| export * from './transitions'; | |||
| export { duration, easing } from './transitions'; | |||
There was a problem hiding this comment.
This exports the same methods as before but needed to change since create and getAutoHightDuration are now also named exports in transitions
| @@ -17,18 +17,6 @@ export interface Duration { | |||
| } | |||
| export const duration: Duration; | |||
|
|
|||
| export function formatMs(milliseconds: number): string; | |||
| // export type TransitionsOptions = DeepPartial<Transitions>; | ||
|
|
||
| declare const transitions: Transitions; | ||
| export default transitions; |
There was a problem hiding this comment.
Default export is gone but was never part of the public interface (only first and second level imports are public).
eps1lon
force-pushed
the
fix/transitions-module
branch
from
e73a956 to
32c5335
Compare
eps1lon
changed the title
Bug confirmed in https://next.material-ui.com/ with
Note that it also impacts zIndex, shadows, and shape.
Why going down this road, the module is private? Would it be simpler with? diff --git a/packages/material-ui/src/styles/createMuiTheme.js b/packages/material-ui/src/styles/createMuiTheme.js
index 9904bff8b5..89cde86d74 100644
--- a/packages/material-ui/src/styles/createMuiTheme.js
+++ b/packages/material-ui/src/styles/createMuiTheme.js
@@ -33,9 +33,9 @@ function createMuiTheme(options = {}, ...args) {
- shadows,
+ shadows: [...shadows],
typography: createTypography(palette, typographyInput),
spacing,
- shape,
- transitions,
- zIndex,
+ shape: { ...shape },
+ transitions: { ...transitions },
+ zIndex: { ...zIndex },
},
other,
); |
oliviertassinari
changed the title
Why not? We're currently mixing named exports with namespace exports. Might as well enable future optimizations. Curious why consistency doesn't apply here all of the sudden.
Indeed but the fixes there are different. I spotted this weird pattern and moved to ES modules which incidentally fixes the mutation side-effects. |
|
@eps1lon No strong preferences, I was surprised to see the change, either way, It doesn't seem to make a significant difference, so happy either way. Properties of keeping it as-is:
Properties of moving the es modules
|
eps1lon
force-pushed
the
fix/transitions-module
branch
from
67d3eb8 to
5d0776e
Compare
View diff without whitespace changes advised.
We're currently exporting an object (namespace) from
core/styles/transitionsinstead of a proper ES module. This means that mutatingtheme.transitionshas global side-effects. We could freeze the object to catch these but we might as well switch to proper ES modules.