search

mui / material-ui

Material UI: Comprehensive React component library that implements Google's Material Design. Free forever.
https://mui.com/material-ui/
MIT License
93.64k stars 32.22k forks source link

Missing units for calculated sizes causes issues @next #9576

Closed botten-ct closed 6 years ago

botten-ct commented 6 years ago

Expected Behavior

Generated sizes include the type of unit, so it's not up to the interpreter to add missing information. 

padding: {
    paddingTop: {theme.spacing.unit}px,
    paddingBottom: {theme.spacing.unit}px,
  },

Current Behavior

Currently, a few calculated sizes in the code base are missing the type of unit. For example in https://github.com/mui-org/material-ui/blob/v1-beta/src/List/List.js

padding: {
    paddingTop: theme.spacing.unit,
    paddingBottom: theme.spacing.unit,
  },

It might be our setup, but at production Firefox and Chrome are complaining about the missing unit type. This is correct as the inspector shows:

.c56 {
    padding-top: 8;
    padding-bottom: 8;
}

Steps to Reproduce (for bugs)

The issue is hard to track down as I am not a developer; however it does not occur at our development environments. This leads me to believe it's going wrong during npm build somewhere in our webpack stack.

Adding the unit types would however bypass the issue all together imho.

Might be related: https://github.com/styled-components/styled-components/issues/825 https://github.com/cssinjs/istf-spec/issues/3

Context

Currently we encounter the issue (and found the culprit code) in a List, but there are more

Multiple development environments image

Multiple prod environments image

Your Environment

Latest Chrome and Firefox, zillion npm packages

├─┬ material-ui@1.0.0-beta.24 ├─┬ material-ui-icons@1.0.0-beta.17 │ ├─┬ jss@9.4.0 │ ├─┬ jss-preset-default@4.0.1 │ │ ├── jss-camel-case@6.0.0 │ │ ├─┬ jss-compose@5.0.0 │ │ ├── jss-default-unit@8.0.2 │ │ ├── jss-expand@5.1.0 │ │ ├─┬ jss-extend@6.1.0 │ │ ├── jss-global@3.0.0 │ │ ├─┬ jss-nested@6.0.1 │ │ ├── jss-props-sort@6.0.0 │ │ ├─┬ jss-template@1.0.0 │ │ └─┬ jss-vendor-prefixer@7.0.0 │ ├─┬ react-jss@8.2.0 │ │ ├── jss@9.4.0 deduped │ │ ├── jss-preset-default@4.0.1 deduped

oliviertassinari commented 6 years ago

Could it be an issue with https://github.com/cssinjs/jss-default-unit cc @kof?

kof commented 6 years ago

@botten-ct please create a demo on codesandbox

botten-ct commented 6 years ago

@kof Found out it's caused by transform-property-literals plugin for babel. With the plugin disabled, all is fine. Unless research shows this is actually caused by jss/materia-ui this ticked can be considered closed.

oliviertassinari commented 6 years ago

@botten-ct Thanks for letting us know :).