Repository: styled-system/styled-system (js)

Issue:
Nested themes for local breakpoints not working
I've noticed something weird with nested themes and their respective breakpoints. We have a "base" theme with the default `const breakpoints = ['40em', '52em', '64em']`. Sometimes, we want to use "local" overrides of the base theme, e.g. for a component that might have only one breakpoint which is not in the array above. We do this like this (There's one other `ThemeProvider` further up the component tree with the "default" theme):

```jsx
const Example = () => {
  const localTheme = theme => ({
    ...theme,
    breakpoints: ['11em', '22em', '33em'],
  })

  return (
    <ThemeProvider theme={localTheme}>
      {/* Should be shown above 11em */}
      <Text display={['none', 'block']}>Hola.</Text>
    <ThemeProvider>
  )
}
```

While this works completely fine for one component, it behaves really strangely in one other.

Using the above "local theme" with:
```jsx
<Text
  color={['white', 'hotpink', 'white']}
  fontSize={[1, 2, 3]}
>
  Hola.
</Text>
```

... results in:

![image](https://user-images.githubusercontent.com/6179211/60445898-7c4fc580-9c20-11e9-978d-bf41d7160ed4.png)

So, the `color` prop uses the "local theme" values, but `fontSize` uses the default breakpoints. Am I missing something obvious here?

Pull request description:
Enable dynamically changed breakpoints for nested themes 
## Problem
[Starting with v5, style functions cache `theme.breakpoints`](https://github.com/styled-system/styled-system/blob/master/CHANGELOG.md#v500-2019-06-02), which
breaks nested themes with varying breakpoints.

## Possible Solution
I'm not too sure about this, so here goes a first draft:
* If you nest themes (i.e. `<ThemeProvider>`s`), and the child theme provides a new array of breakpoints, the cache should be updated.
* If you nest one more level and don't provide new values, breakpoints will fall back to the default values.

I'm not sure if this is ideal, because I don't know why the cache was implemented in the first place. If the intention was to avoid an unnecessary `get` call, then this solution won't work because it checks whether new breakpoints have been provided with `get`. I feel like I'm in over my head here, so any help is much appreciated :)

Closes #592

Patch:
diff --git a/packages/core/src/index.js b/packages/core/src/index.js
index c146eb1..1d048bc 100644
--- a/packages/core/src/index.js
+++ b/packages/core/src/index.js
@@ -29,6 +29,8 @@ export const createParser = config => {
   const cache = {}
   const parse = props => {
     let styles = {}
+    const isCacheDisabled = props.theme && props.theme.disableStyledSystemCache
+
     for (const key in props) {
       if (!config[key]) continue
       const sx = config[key]
@@ -37,10 +39,10 @@ export const createParser = config => {
 
       if (typeof raw === 'object') {
         cache.breakpoints =
-          cache.breakpoints ||
+          (!isCacheDisabled && cache.breakpoints) ||
           get(props.theme, 'breakpoints', defaults.breakpoints)
         if (Array.isArray(raw)) {
-          cache.media = cache.media || [
+          cache.media = (!isCacheDisabled && cache.media) || [
             null,
             ...cache.breakpoints.map(createMediaQuery),
           ]
@@ -67,6 +69,14 @@ export const createParser = config => {
   parse.config = config
   parse.propNames = Object.keys(config)
   parse.cache = cache
+
+  const keys = Object.keys(config).filter(k => k !== 'config')
+  if (keys.length > 1) {
+    keys.forEach(key => {
+      parse[key] = createParser({ [key]: config[key] })
+    })
+  }
+
   return parse
 }
 


Test patch:
diff --git a/packages/core/test/parser.js b/packages/core/test/parser.js
new file mode 100644
index 0000000..8de7f39
--- /dev/null
+++ b/packages/core/test/parser.js
@@ -0,0 +1,118 @@
+import { system, compose } from '../src'
+
+const theme = {
+  colors: {
+    primary: 'rebeccapurple',
+    secondary: 'papayawhip',
+  },
+  fontSize: [0, 4, 8, 16],
+}
+
+const parser = system({
+  color: {
+    property: 'color',
+    scale: 'colors',
+  },
+  fontSize: true,
+})
+
+test('uses default breakpoints', () => {
+  const styles = parser({
+    theme: theme,
+    fontSize: [1, 2, 3],
+    color: ['primary', null, 'secondary'],
+  })
+  expect(styles).toEqual({
+    color: 'rebeccapurple',
+    fontSize: 4,
+    '@media screen and (min-width: 40em)': {
+      fontSize: 8,
+    },
+    '@media screen and (min-width: 52em)': {
+      fontSize: 16,
+      color: 'papayawhip',
+    },
+  })
+})
+
+// Per default, we expect it to be impossible to override breakpoints
+test('does *not* use dynamically changed breakpoints', () => {
+  const styles = parser({
+    theme: { ...theme, breakpoints: ['11em', '22em', '33em'] },
+    fontSize: [1, 2, 3],
+    color: ['primary', null, 'secondary'],
+  })
+  expect(styles).toEqual({
+    color: 'rebeccapurple',
+    fontSize: 4,
+    '@media screen and (min-width: 40em)': {
+      fontSize: 8,
+    },
+    '@media screen and (min-width: 52em)': {
+      fontSize: 16,
+      color: 'papayawhip',
+    },
+  })
+})
+
+// With caching disabled, we expect it to be possible to change breakpoints
+test('uses dynamically changed breakpoints', () => {
+  const firstStyles = parser({
+    theme: {
+      ...theme,
+      breakpoints: ['11em', '22em', '33em'],
+      disableStyledSystemCache: true,
+    },
+    fontSize: [1, 2, 3],
+    color: ['primary', null, 'secondary'],
+  })
+  expect(firstStyles).toEqual({
+    color: 'rebeccapurple',
+    fontSize: 4,
+    '@media screen and (min-width: 11em)': {
+      fontSize: 8,
+    },
+    '@media screen and (min-width: 22em)': {
+      fontSize: 16,
+      color: 'papayawhip',
+    },
+  })
+
+  const secondStyles = parser({
+    theme: {
+      ...theme,
+      breakpoints: ['9em', '8em', '7em'],
+      disableStyledSystemCache: true,
+    },
+    fontSize: [1, 2, 3],
+    color: ['primary', null, 'secondary'],
+  })
+  expect(secondStyles).toEqual({
+    color: 'rebeccapurple',
+    fontSize: 4,
+    '@media screen and (min-width: 9em)': {
+      fontSize: 8,
+    },
+    '@media screen and (min-width: 8em)': {
+      fontSize: 16,
+      color: 'papayawhip',
+    },
+  })
+
+  const thirdStyles = parser({
+    theme: theme,
+    fontSize: [1, 2, 3],
+    color: ['primary', null, 'secondary'],
+  })
+  expect(thirdStyles).toEqual({
+    color: 'rebeccapurple',
+    fontSize: 4,
+    '@media screen and (min-width: 9em)': {
+      fontSize: 8,
+    },
+    '@media screen and (min-width: 8em)': {
+      fontSize: 16,
+      color: 'papayawhip',
+    },
+  })
+})