square / maker

Maker Design System by Square
https://square.github.io/maker/styleguide/latest-stable/
Other
63 stars 14 forks source link

theme profiles should be an object and not a keyed array #364

Closed pretzelhammer closed 2 years ago

pretzelhammer commented 2 years ago

Bug description

merging profiles data in nested themes is kinda convoluted because they're currently defined in a keyed array (i.e. an array of objects with id fields) instead of an object. merge logic implemented in this pr: https://github.com/square/maker/pull/360

aside from merging the data being kinda convoluted, it also allows people to validly define profiles with duplicate ids without triggering an errors from their IDE, example of totally valid JS:

{
  profiles: [
    { id: 'a', value: 1 },
    { id: 'a', value: 2 },
  ],
}

but if it was an object it's a lot easier and faster to notice this error:

{
  profiles: {
    a: 1,
    a: 2, // obviously wrong, and detectable by IDEs
  },
}

Reproduction

no repro steps

Environment

every env

Addressed by

idk, the issue is making this change would be a breaking change and would require a (minor) refactor in website so whoever is willing to also do the refactor in website can do this task at their earliest convenience