Is it more preferable to use @material-ui/core/styles
instead of @material-ui/styles
if material-ui is used?
Also according to the Minimizing Bundle Size Guide, anything below 2nd level import is considered private and can cause module duplication in your bundle. So should I import makeStyles
like this to minimize bundle size?
import makeStyles from '@material-ui/core/styles/makeStyles';
If you follow the documentation, you should be fine. To answer your question, in your case, the best approach is:
import { makeStyles} from '@material-ui/core/styles';
(the example you have shared imports 3 levels deep, it's not OK)
Hum, in the documentation we say:
Anything below is considered private and can cause module duplication in your bundle.
It seems that it has confused you. Below is used with the "deeper" semantic. I think that we should update the wording.
The docs are very confusing. What are the best practices?
@kurtwilbies what do you mean?
@matthewkwong2 What do you think of this diff?
diff --git a/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md b/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md
index 0cf8c2443..c615f5586 100644
--- a/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md
+++ b/docs/src/pages/guides/minimizing-bundle-size/minimizing-bundle-size.md
@@ -49,8 +49,8 @@ Head to [Option 2](#option-2) for the approach that yields the best DX and UX.
While importing directly in this manner doesn't use the exports in [`@material-ui/core/index.js`](https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/index.js), this file can serve as a handy reference as to which modules are public.
-Be aware that we only support first and second level imports. Anything below is considered
-private and can cause module duplication in your bundle.
+Be aware that we only support first and second level imports. Anything deeper is considered
+private and can cause issues, like module duplication in your bundle.
Would it remove part of the confusion?
I meant use examples, because this issue depends on a deep knowledge of the source code.
best solution would be to remove material ui styles all together , react-jss 10 is released
@Coglite #17391
I had to change to change all my references from:
import MuiThemeProvider from "@material-ui/core/styles/MuiThemeProvider";
to:
import { MuiThemeProvider } from "@material-ui/core/styles";
when upgrading from @material-ui/[email protected] to @material-ui/[email protected], otherwise the MuiThemeProvide module could not be found.
@shiraze Thanks for the feedback! This looks expected, imports deeper than 2 levels deep are private. @material-ui/core/styles/MuiThemeProvider
is a private module.
Might be good to create a codemod that could be run to catch and fix improper imports.
There is a codemod and and eslint rule https://github.com/mui-org/material-ui/tree/master/packages/eslint-plugin-material-ui#restricted-path-imports, https://github.com/mui-org/material-ui/tree/master/packages/material-ui-codemod#optimal-imports.
Is it problematic to import types from deep import paths? I'd like to remove my explicit dependency on @material-ui/styles
, but I'm not sure where to import e.g. the CSSProperties
type from, if not @material-ui/core/styles/withStyles
.
@pschlette Could you explain what you need this type for? I'd hope everything is covered by *Styles
.
Hi, I have the following.
import { CSSProperties } from '@material-ui/styles`
interface Common {
appbarSpace: CSSProperties;
}
const getAppStyle = (theme: Theme) => {
return {
appbarSpace: 50,
}
}
So after upgrading to 4.5.1
, I shifted most of the component from @material-ui/styles
to @material-ui/core/styles
. But there's no export of CSSProperties
now, so it complains about that.
Any idea?
@josephstgh It looks like your Common
interface is unused. Could you expand your example so that Common
and getAppStyle
is used?
Hi sorry, let me try.
I have a place where Theme
is defined.
interface Theme {
customPalette: ReturnType<typeof getAppThemeColor>;
commonStyle: () => Common;
}
const appTheme = (options: ThemeOptions) => {
const theme = createMuiTheme({ ... // commented off });
theme.commonStyle = () => getAppStyle(theme);
return theme;
}
So in my other component where I can make use of the commonStyle
.
const useStyles = makeStyles((theme: Theme) => ({
smallPadding: {
...theme.commonStyle().appbarSpace,
// some other properties
}
});
Full disclosure, this is actually part of the workaround because jssExtend()
plugin does not work for my case. Hence, the use of spreading
to extend
the style.
@eps1lon Using the CSSProperties
type seemed like a reasonable way to annotate variables holding some shared styles, like this (simplified a bit):
const useStyles = makeStyles(() => {
// Annotate this with `CSSProperties` to avoid type widening and see type errors here
const sharedStyles: CSSProperties = { display: "inline-flex" };
return createStyles({
class1: {
...sharedStyles,
color: "red"
},
class2: {
...sharedStyles,
color: "green"
}
}));
Sorry - I'm not sure what you meant by *Styles
?
I was really in favor to use @material-ui/styles
and make it as a peer dependency of @material-ui/core
.
I think this is something suggested here also: https://www.styled-components.com/docs/faqs#i-am-a-library-author-should-i-bundle-styledcomponents-with-my-library
@oliviertassinari I can rewrite the implementation to use react-jss in a couple hours, but there is 1 thing i am unclear on how to solve, which is the use of the '$(pseudoClass)' variables scattered throughout the styles. For example, the styles on the Button use '$disabled' below:
export const styles = theme => ({
/* Styles applied to the root element. */
root: {
...theme.typography.button,
boxSizing: 'border-box',
minWidth: 64,
padding: '6px 16px',
borderRadius: theme.shape.borderRadius,
color: theme.palette.text.primary,
transition: theme.transitions.create(['background-color', 'box-shadow', 'border'], {
duration: theme.transitions.duration.short,
}),
'&:hover': {
textDecoration: 'none',
backgroundColor: fade(theme.palette.text.primary, theme.palette.action.hoverOpacity),
// Reset on touch devices, it doesn't add specificity
'@media (hover: none)': {
backgroundColor: 'transparent',
},
'&$disabled': {
backgroundColor: 'transparent',
},
},
'&$disabled': {
color: theme.palette.action.disabled,
},
},
//...rest
I understand what it does just not how it interops with jss-nested and its effect on rule generation.
https://github.com/mui-org/material-ui/blob/master/packages/material-ui-styles/src/createGenerateClassName/createGenerateClassName.js
any guidance? or maybe like a pseudo code walk through of the logic.
something like
if stylesheet starts with Mui -> apply pseudoSelector logic
else apply jss-nested logic
Most helpful comment
There is a codemod and and eslint rule https://github.com/mui-org/material-ui/tree/master/packages/eslint-plugin-material-ui#restricted-path-imports, https://github.com/mui-org/material-ui/tree/master/packages/material-ui-codemod#optimal-imports.