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

MUI 5 styled() pass props along with theme object #29207

Open
1 task done
lougroshek opened this issue Oct 21, 2021 · 6 comments
Open
1 task done

MUI 5 styled() pass props along with theme object #29207

lougroshek opened this issue Oct 21, 2021 · 6 comments

Comments

@lougroshek
Copy link

@lougroshek lougroshek commented Oct 21, 2021

Based on MUI's documentation of the styled() utility, the theme is provided for styling directly attached to the component, but props are supposed to be handled only within the overridesResolver option. If this is used for large applications, it means that any theming based on conditional logic has to be lifted up into the theme. Potential disadvantages/deleterious effects on developer experience:

  • Extremely large theme files
  • Theming split between custom components and theme

The legacy makeStyles utility allowed props to be passed along with the theme, and I don't understand why that is not also offered here. It is possible to create an intermediary component as a workaround, but that is extra work and code. Why is there a forwardProps option but no way to get props available to the local theming rules? If this is what we should be doing in this case it should probably be documented because at present, given how opinionated MUI is, it feels like a hack.

Workaround

const MyStyledComponent = ({myProp, children}) => {

  const StyledEl = styled('div')(({ theme }) => ({
    backgroundColor: myProp ? 'aliceblue' : 'red',
    padding: theme.spacing(1),
  }));

  return StyledEl;
}
  • I have searched the issues of this repository and believe that this is not a duplicate.

Summary 💡

Ability to pass props in with theme. Allows to keep all styles in one place for a custom component, rather than lifting up to theme and potentially creating an unwieldy large theme file or splitting styling rules for a single component into multiple places.

const StyledComp = styled('div', {themeProps: props})(({ theme, props }) => ({
    backgroundColor: props.myProp ? 'aliceblue' : 'red',
    padding: theme.spacing(1),
 }));

Motivation 🔦

Use MUI components and v5 theming solution without splitting styling rules into multiple places or creating a huge theme file.

@siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Oct 22, 2021

You don't need to create an intermediate component to pass your custom props. You can do:

JS

const StyledComp = styled("div", {
  shouldForwardProp: (prop) => prop !== "color"
})(({ theme, myProp, color }) => ({
  backgroundColor: myProp ? "aliceblue" : "red",
  color,
  padding: theme.spacing(1)
}));

<StyledComp myProp color="red" />

TS

const StyledComp = styled("div", {
  shouldForwardProp: (prop) => prop !== "color"
})<{ myProp?: boolean; color?: string }>(({ theme, myProp, color }) => ({
  backgroundColor: myProp ? "aliceblue" : "red",
  color,
  padding: theme.spacing(1)
}));

<StyledComp myProp color="red" /> // typed safe

https://codesandbox.io/s/basicbuttons-material-demo-forked-7vs5v?file=/demo.tsx

Note: for some props, you have to define shouldForwardProps to the styled options so that the custom props are not spread to DOM.

@lougroshek
Copy link
Author

@lougroshek lougroshek commented Oct 22, 2021

@siriwatknp Your solution is in typescript. My original example that I linked to, as well as my code sample, is not in typescript. Is the actual solution you are proposing "switch your entire project to typescript"? Not everyone is using typescript. If you're telling me to do that you haven't addressed my initial question, much less triaged. If MUI should only be used with typescript it should say so in the docs.

@siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Oct 22, 2021

@siriwatknp Your solution is in typescript. My original example that I linked to, as well as my code sample, is not in typescript. Is the actual solution you are proposing "switch your entire project to typescript"? Not everyone is using typescript. If you're telling me to do that you haven't addressed my initial question, much less triaged. If MUI should only be used with typescript it should say so in the docs.

I have included JS example in my response. Please check it again if that works for you.

@lougroshek
Copy link
Author

@lougroshek lougroshek commented Oct 22, 2021

@siriwatknp Much appreciated! I had found the ts demo you linked to previously but wasn't clear how that looked in JS, and in fact I was getting eslint errors to do with type definition when I tried. Is there any possibility of making this clearer in the docs? I am happy to pursue that myself if that's an option.

@RookTKO
Copy link

@RookTKO RookTKO commented Oct 26, 2021

@siriwatknp Much appreciated! I had found the ts demo you linked to previously but wasn't clear how that looked in JS, and in fact I was getting eslint errors to do with type definition when I tried. Is there any possibility of making this clearer in the docs? I am happy to pursue that myself if that's an option.

I second this. It really wasn't very clear in the documentation to me as how to get that to work but the example above produced a-lot of clarity. If @lougroshek doesn't have time I would be more then happy to submit the update.

@siriwatknp
Copy link
Member

@siriwatknp siriwatknp commented Oct 27, 2021

@lougroshek @RookTKO What do you think about adding the info to this page?

Feel free to submit a PR if you'd like.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
3 participants