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

[system] compose with style function 'spacing' throws error #23649

Closed
2 tasks done
onkarj422 opened this issue Nov 22, 2020 · 3 comments
Closed
2 tasks done

[system] compose with style function 'spacing' throws error #23649

onkarj422 opened this issue Nov 22, 2020 · 3 comments
Labels
package: system Specific to @mui/system support: question Community support but can be turned into an improvement

Comments

@onkarj422
Copy link

onkarj422 commented Nov 22, 2020

Considering the general working of the compose function, it returns a style function combining multiple style functions passed to it. And the general working of a style function seems like it returns CSS Object when passed with a props object accepted by the style function.
So, I tried following code to see if it works as expected

import { compose, palette, sizing, spacing } from '@material-ui/system';

const muiStyles = compose(spacing, palette, sizing);

console.log(
    muiStyles({
        padding: '14px',
        bgcolor: 'red',
        height: '300px',
        width: '100%',
    }),
);
// should log CSS object => { padding: '14px', backgroundColor: 'red', height: '300px', width: '300px' }

This code compiles without any type error

  • The issue is present in the latest release.
  • I have searched the issues of this repository and believe that this is not a duplicate.

Current Behavior 😯

At runtime, it throws this error

TypeError: Cannot read property 'spacing' of undefined
    at createUnarySpacing (D:\workspace-prsnl\react-cool-universal\public\server\webpack:\node_modules\@material-ui\system\esm\spacing.js:50:1)
    at spacing (D:\workspace-prsnl\react-cool-universal\public\server\webpack:\node_modules\@material-ui\system\esm\spacing.js:119:1)
    at D:\workspace-prsnl\react-cool-universal\public\server\webpack:\node_modules\@material-ui\system\esm\compose.js:11:1
    at Array.reduce (<anonymous>)
    at muiStyles (D:\workspace-prsnl\react-cool-universal\public\server\webpack:\node_modules\@material-ui\system\esm\compose.js:10:1)
    at Module.<anonymous> (D:\workspace-prsnl\react-cool-universal\public\server\webpack:\src\app\index.tsx:20:5)
    at Module../src/app/index.tsx (D:\workspace-prsnl\react-cool-universal\public\server\main.js:49155:30)
    at __webpack_require__ (D:\workspace-prsnl\react-cool-universal\public\server\webpack:\webpack\bootstrap:19:1)
    at Module.<anonymous> (D:\workspace-prsnl\react-cool-universal\public\server\main.js:49438:61)
    at Module../src/server/ssr.tsx (D:\workspace-prsnl\react-cool-universal\public\server\main.js:49578:30)

Expected Behavior 🤔

Should not throw error and output the expected CSS object

Steps to Reproduce 🕹

Here's the link to reproduce
https://codesandbox.io/s/material-ui-issue-forked-bblik?file=/src/Demo.js

Context 🔦

This would have been useful to style any component using material ui style system. So, i was trying to use styles which style functions return on a Card component, instead of using a Box component everywhere, this would have been quite handy.

Your Environment 🌎

Tech Version
Material-UI 4.11.0
React 16.14.0
Browser Chrome
TypeScript 4.0.5
etc.
@onkarj422 onkarj422 added the status: waiting for maintainer These issues haven't been looked at yet by a maintainer label Nov 22, 2020
@oliviertassinari oliviertassinari added package: system Specific to @mui/system support: question Community support but can be turned into an improvement and removed status: waiting for maintainer These issues haven't been looked at yet by a maintainer labels Nov 22, 2020
@oliviertassinari
Copy link
Member

oliviertassinari commented Nov 22, 2020

@onkarj422 You need to provide a theme if you are going to use it like this:

      muiStyles({
        theme: {
          spacing: 8
        },
        height: "300px",
        width: "100%",
        bgcolor: "red",
        padding: 2
      })

https://codesandbox.io/s/material-ui-issue-forked-8rcpy?file=/src/Demo.js:337-497

@oliviertassinari
Copy link
Member

This would have been useful to style any component using material ui style system. So, i was trying to use styles which style functions return on a Card component, instead of using a Box component everywhere, this would have been quite handy.

@onkarj422 All core components will support the system in v5. We document it in here, a demo with the first component that we have migrated:

    <Slider
      value={value}
      onChange={handleChange}
      sx={{
        bgcolor: "background.default",
        color: "grey.600"
      }}
    />

Capture d’écran 2020-11-22 à 11 25 54

https://codesandbox.io/s/continuousslider-material-demo-forked-n59bx?file=/demo.js

Follow #23220 for the support of any primitives.

@onkarj422
Copy link
Author

@onkarj422 You need to provide a theme if you are going to use it like this:

      muiStyles({
        theme: {
          spacing: 8
        },
        height: "300px",
        width: "100%",
        bgcolor: "red",
        padding: 2
      })

https://codesandbox.io/s/material-ui-issue-forked-8rcpy?file=/src/Demo.js:337-497

@oliviertassinari
Thank you for the quick reply on this!

Although it throws type mismatch error when theme object is passed along, this is still helpful. I suppressed the type error for now, will refactor once v5 is out!

Now that this whole thing is already coming up in v5, looking forward to it!

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
package: system Specific to @mui/system support: question Community support but can be turned into an improvement
Projects
None yet
Development

No branches or pull requests

2 participants