Feature/theme spreads (javivelasco#47)

raveclassic · javivelasco · commit 15869495b52e · 2017-02-20T12:46:00.000+01:00
* refactoring * theme spreads * typings * fix themr typing * fix themr typing * fix undefined mixin value cases * fix state type incompatibility in javivelasco#39 * revert typing changes * accept sfc as component * typo * also accept symbols and numbers as identifier * merge sfc fix
diff --git a/index.d.ts b/index.d.ts
@@ -1,37 +1,35 @@
 import * as React from "react";
 
-declare module "react-css-themr"
-{
-	export interface IThemrOptions
-	{
-		/** @default "deeply" */
-		composeTheme?: "deeply" | "softly" | false,
-	}
-
-	export interface ThemeProviderProps
-	{
-    innerRef?: Function,
-		theme: {}
-	}
+declare module "react-css-themr" {
+  type TReactCSSThemrTheme = {
+    [key: string]: string | TReactCSSThemrTheme
+  }
+
+  export function themeable(...themes: Array<TReactCSSThemrTheme>): TReactCSSThemrTheme;
 
-	export class ThemeProvider extends React.Component<ThemeProviderProps, any>
-	{
+  export interface IThemrOptions {
+    /** @default "deeply" */
+    composeTheme?: "deeply" | "softly" | false,
+  }
 
-	}
+  export interface ThemeProviderProps {
+    innerRef?: Function,
+    theme: {}
+  }
 
-	interface ThemedComponent<P, S> extends React.Component<P, S>
-	{
+  export class ThemeProvider extends React.Component<ThemeProviderProps, any> {
+  }
 
-	}
+  interface ThemedComponent<P, S> extends React.Component<P, S> {
+  }
 
-	interface ThemedComponentClass<P, S> extends React.ComponentClass<P>
-	{
-		new(props?: P, context?: any): ThemedComponent<P, S>;
-	}
+  interface ThemedComponentClass<P, S> extends React.ComponentClass<P> {
+    new(props?: P, context?: any): ThemedComponent<P, S>;
+  }
 
-	export function themr(
-		identifier: string,
-		defaultTheme?: {},
-		options?: IThemrOptions
-	): <P, S>(component: new(props?: P, context?: any) => React.Component<P, S>) => ThemedComponentClass<P, S>;
+  export function themr(
+    identifier: string | number | symbol,
+    defaultTheme?: {},
+    options?: IThemrOptions
+  ): <P, S>(component: (new(props?: P, context?: any) => React.Component<P, S>) | React.SFC<P>) => ThemedComponentClass<P, S>;
 }
diff --git a/package.json b/package.json
@@ -27,6 +27,7 @@
     "invariant": "^2.2.1"
   },
   "devDependencies": {
+    "@types/react": "~15.0.4",
     "babel-cli": "^6.7.7",
     "babel-core": "^6.18.0",
     "babel-eslint": "^7.1.1",
diff --git a/src/components/themr.js b/src/components/themr.js
@@ -84,7 +84,7 @@ export default (componentName, localTheme, options = {}) => (ThemedComponent) =>
     getNamespacedTheme(props) {
       const { themeNamespace, theme } = props
       if (!themeNamespace) return theme
-      if (themeNamespace &&  !theme) throw new Error('Invalid themeNamespace use in react-css-themr. ' +
+      if (themeNamespace && !theme) throw new Error('Invalid themeNamespace use in react-css-themr. ' +
         'themeNamespace prop should be used only with theme prop.')
 
       return Object.keys(theme)
@@ -153,57 +153,99 @@ export default (componentName, localTheme, options = {}) => (ThemedComponent) =>
 }
 
 /**
- * Merges two themes by concatenating values with the same keys
- * @param {TReactCSSThemrTheme} [original] - Original theme object
- * @param {TReactCSSThemrTheme} [mixin] - Mixing theme object
- * @returns {TReactCSSThemrTheme} - Merged resulting theme
+ * Merges passed themes by concatenating string keys and processing nested themes
+ * @param {...TReactCSSThemrTheme} themes - Themes
+ * @returns {TReactCSSThemrTheme} - Resulting theme
  */
-export function themeable(original = {}, mixin) {
-  //don't merge if no mixin is passed
-  if (!mixin) return original
-
-  //merge themes by concatenating values with the same keys
-  return Object.keys(mixin).reduce(
-
-    //merging reducer
-    (result, key) => {
-
-      const originalValue = typeof original[key] !== 'function'
-        ? (original[key] || '')
-        : ''
-      const mixinValue = typeof mixin[key] !== 'function'
-        ? (mixin[key] || '')
-        : ''
-
-      let newValue
+export function themeable(...themes) {
+  return themes.reduce((acc, theme) => merge(acc, theme), {})
+}
 
-      //when you are mixing an string with a object it should fail
-      invariant(!(typeof originalValue === 'string' && typeof mixinValue === 'object'),
-        `You are merging a string "${originalValue}" with an Object,` +
-        'Make sure you are passing the proper theme descriptors.'
-      )
+/**
+ * @param {TReactCSSThemrTheme} [original] - Original theme
+ * @param {TReactCSSThemrTheme} [mixin] - Mixin theme
+ * @returns {TReactCSSThemrTheme} - resulting theme
+ */
+function merge(original = {}, mixin = {}) {
+  //make a copy to avoid mutations of nested objects
+  //also strip all functions injected by isomorphic-style-loader
+  const result = Object.keys(original).reduce((acc, key) => {
+    const value = original[key]
+    if (typeof value !== 'function') {
+      acc[key] = value
+    }
+    return acc
+  }, {})
+
+  //traverse mixin keys and merge them to resulting theme
+  Object.keys(mixin).forEach(key => {
+    //there's no need to set any defaults here
+    const originalValue = result[key]
+    const mixinValue = mixin[key]
+
+    switch (typeof mixinValue) {
+      case 'object': {
+        //possibly nested theme object
+        switch (typeof originalValue) {
+          case 'object': {
+            //exactly nested theme object - go recursive
+            result[key] = merge(originalValue, mixinValue)
+            break
+          }
+
+          case 'undefined': {
+            //original does not contain this nested key - just take it as is
+            result[key] = mixinValue
+            break
+          }
+
+          default: {
+            //can't merge an object with a non-object
+            throw new Error(`You are merging object ${key} with a non-object ${originalValue}`)
+          }
+        }
+        break
+      }
 
-      //check if values are nested objects
-      if (typeof originalValue === 'object' && typeof mixinValue === 'object') {
-        //go recursive
-        newValue = themeable(originalValue, mixinValue)
-      } else {
-        //either concat or take mixin value
-        newValue = originalValue.split(' ')
-          .concat(mixinValue.split(' '))
-          .filter((item, pos, self) => self.indexOf(item) === pos && item !== '')
-          .join(' ')
+      case 'undefined': //fallthrough - handles accidentally unset values which may come from props
+      case 'function': {
+        //this handles issue when isomorphic-style-loader addes helper functions to css-module
+        break //just skip
       }
 
-      return {
-        ...result,
-        [key]: newValue
+      default: {
+        //plain values
+        switch (typeof originalValue) {
+          case 'object': {
+            //can't merge a non-object with an object
+            throw new Error(`You are merging non-object ${mixinValue} with an object ${key}`)
+          }
+
+          case 'undefined': {
+            //mixin key is new to original theme - take it as is
+            result[key] = mixinValue
+            break
+          }
+          case 'function': {
+            //this handles issue when isomorphic-style-loader addes helper functions to css-module
+            break //just skip
+          }
+
+          default: {
+            //finally we can merge
+            result[key] = originalValue.split(' ')
+              .concat(mixinValue.split(' '))
+              .filter((item, pos, self) => self.indexOf(item) === pos && item !== '')
+              .join(' ')
+            break
+          }
+        }
+        break
       }
-    },
+    }
+  })
 
-    //use original theme as an acc
-    original
-  )
+  return result
 }
 
 /**
diff --git a/test/components/themr.spec.js b/test/components/themr.spec.js
@@ -510,17 +510,113 @@ describe('themeable function', () => {
     expect(result).toEqual(expected)
   })
 
-  it('should skip dupplicated keys classNames', () => {
+  it('should skip duplicated keys classNames', () => {
     const themeA = { test: 'test' }
     const themeB = { test: 'test test2' }
     const expected = { test: 'test test2' }
     const result = themeable(themeA, themeB)
     expect(result).toEqual(expected)
   })
 
-  it('throws an exception when its called mixing a string with an object', () => {
-    expect(() => {
-      themeable('fail', { test: { foo: 'baz' } })
-    }).toThrow(/sure you are passing the proper theme descriptors/)
+  it('should take mixin value if original does not contain one', () => {
+    const themeA = {}
+    const themeB = {
+      test: 'test',
+      nested: {
+        bar: 'bar'
+      }
+    }
+    const expected = themeB
+    const result = themeable(themeA, themeB)
+    expect(result).toEqual(expected)
+  })
+
+  it('should take original value if mixin does not contain one', () => {
+    const themeA = {
+      test: 'test',
+      nested: {
+        bar: 'bar'
+      }
+    }
+    const themeB = {}
+    const expected = themeA
+    const result = themeable(themeA, themeB)
+    expect(result).toEqual(expected)
+  })
+
+  it('should skip function values for usage with isomorphic-style-loader', () => {
+    const themeA = {
+      test: 'test',
+      foo() {
+      }
+    }
+
+    const themeB = {
+      test: 'test2',
+      bar() {
+      }
+    }
+
+    const expected = {
+      test: [
+        themeA.test, themeB.test
+      ].join(' ')
+    }
+
+    const result = themeable(themeA, themeB)
+    expect(result).toEqual(expected)
+  })
+
+  it('should throw when merging objects with non-objects', () => {
+    const themeA = {
+      test: 'test'
+    }
+    const themeB = {
+      test: {
+      }
+    }
+    expect(() => themeable(themeA, themeB)).toThrow()
+  })
+
+  it('should throw when merging non-objects with objects', () => {
+    const themeA = {
+      test: {
+      }
+    }
+    const themeB = {
+      test: 'test'
+    }
+    expect(() => themeable(themeA, themeB)).toThrow()
+  })
+
+  it('should support theme spreads', () => {
+    const a = {
+      test: 'a'
+    }
+    const b = {
+      test: 'b'
+    }
+    const c = {
+      test: 'foo',
+      foo: 'foo'
+    }
+    const expected = {
+      test: 'a b foo',
+      foo: 'foo'
+    }
+    const result = themeable(a, b, c)
+    expect(result).toEqual(expected)
+  })
+
+  it('should skip undefined mixin values', () => {
+    const a = {
+      test: 'a'
+    }
+    const b = {
+      test: undefined
+    }
+    const expected = a
+    const result = themeable(a, b)
+    expect(result).toEqual(expected)
   })
 })
