instacart / instacart/Snacks

Update styles to always use arrays instead of sometimes object merging

Open
#250 1 comment 0 reactions 0 assignees View on GitHub
bug help wanted
Dominant language
JavaScript
Stars
82
Forks
36
PR merge metrics
No merged PRs in 30d

Description

Radium allows you to use (potentially nested) arrays of style objects that are later flattened and merged correctly. Currently, users of Snacks components can only specify a style object. We then sometimes combine this style with existing Snacks styles via the spread operator.

This can be problematic when the user needs to combine multiple style objects with the built in styles of the Snacks component. Also, if the user is passing in a nested object (e.g. a style object w/ a media query), only the top level object is merged while the nested object properties replace existing properties.

The property `foo` goes missing below:
```js
const styleFromSnacks = { color: 'red', myMediaQuery: { foo: 1 } }
const styleFromUser = { myMediaQuery: { bar: 2 } }
const mergedStyles = {...styleFromSnacks, ...styleFromUser}
console.log(mergedStyles.myMediaQuery) // => {bar: 2}
```

Some places in the codebase [use arrays,](https://github.com/instacart/Snacks/blob/aecbb21cddb260e31e6cd7606482a6b6fa83aa70/src/components/Link/Link.js#L30) but others are [merging objects directly](https://github.com/instacart/Snacks/blob/aecbb21cddb260e31e6cd7606482a6b6fa83aa70/src/base/RadioCheckboxBase.js#L120).

To solve this issue, we could:
- Update all Snacks components' `style` prop to accept either an array or object.
- Stop using object spreading to combine styles. Instead, build an array of style objects.

Contributor guide

Open the contributing guide

Assessment

This issue has not been assessed yet.

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.