search

mui / material-ui

Material UI: Ready-to-use foundational React components, free forever. It includes Material UI, which implements Google's Material Design.
https://mui.com/material-ui/
MIT License
92.52k stars 31.89k forks source link

Not guaranteed class order in Grid class names definition #13261

Closed Mati365 closed 5 years ago

Mati365 commented 5 years ago

Source

https://github.com/mui-org/material-ui/blob/master/packages/material-ui/src/Grid/Grid.js#L52

Expected Behavior

inject class name styles to Map or something else with guaranteed key order

Current Behavior

grid class names are injected info plain object without guaranteed key order

Context

styles in JSS are placed in for in loop: https://github.com/cssinjs/jss/blob/master/packages/jss/src/StyleSheet.js#L42

but for in loop in object does not guaranteed key order https://stackoverflow.com/a/38218582

eps1lon commented 5 years ago

Could you provide a repro where this is an issue?

Mati365 commented 5 years ago

@eps1lon nope, due to most modern browsers preserve insert order for string keys(for now) :)

eps1lon commented 5 years ago

Let me rephrase that: Could you provide a repro where this is an issue if the order would not be preserved?

Overall I think this is more of an issue for jss. for .. in has no order specified at all so I don't think they intended for the order to matter. Looks like you're relying on an implementation detail.

oliviertassinari commented 5 years ago

@Mati365 Don't worry, we are good: http://2ality.com/2015/10/property-traversal-order-es6.html.

The spec: http://www.ecma-international.org/ecma-262/6.0/#sec-ordinary-object-internal-methods-and-internal-slots-ownpropertykeys.

capture d ecran 2018-10-19 a 20 35 32

You can see the browsers support in https://kangax.github.io/compat-table/es6/#own_property_order.