[react-jss] Merge classes instead of overwriting (#946)

* Add forwardRef support * Add support for merging the classes * Fix path of test file * Remove jsdoc comment * Implement custom memoize-one * Convert function to arrow function
cssinjs · Dec 31, 2018 · 70a05ad · 70a05ad
1 parent 9784b93
commit 70a05ad
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 74 deletions.
diff --git a/packages/react-jss/.size-snapshot.json b/packages/react-jss/.size-snapshot.json
@@ -1,30 +1,30 @@
 {
   "dist/react-jss.js": {
-    "bundled": 146196,
-    "minified": 48865,
-    "gzipped": 14339
+    "bundled": 109837,
+    "minified": 36886,
+    "gzipped": 10571
   },
   "dist/react-jss.min.js": {
-    "bundled": 111372,
-    "minified": 38169,
-    "gzipped": 11622
+    "bundled": 76380,
+    "minified": 26772,
+    "gzipped": 7995
   },
   "dist/react-jss.cjs.js": {
-    "bundled": 15227,
-    "minified": 6630,
-    "gzipped": 2259
+    "bundled": 14336,
+    "minified": 6506,
+    "gzipped": 2151
   },
   "dist/react-jss.esm.js": {
-    "bundled": 14671,
-    "minified": 6180,
-    "gzipped": 2158,
+    "bundled": 13685,
+    "minified": 5954,
+    "gzipped": 2046,
     "treeshaked": {
       "rollup": {
-        "code": 1712,
-        "import_statements": 492
+        "code": 1647,
+        "import_statements": 472
       },
       "webpack": {
-        "code": 3130
+        "code": 3029
       }
     }
   }

diff --git a/packages/react-jss/src/compose.js b/packages/react-jss/src/compose.js
@@ -1,32 +0,0 @@
-// @flow
-import type {Classes} from 'jss'
-/**
- * Adds `composes` property to each top level rule
- * in order to have a composed class name for dynamic style sheets.
- *
- * It relies on jss-plugin-compose and jss-plugin-extend plugins.
- *
- * Example:
- * classes:  {left: 'a', button: 'b'}
- * styles:   {button: {height: () => { ... }}}
- * composed: {
- *   button: {
- *     composes: 'b',
- *     height: () => { ... }
- *   },
- *   left: {
- *     composes: 'a'
- *   }
- * }
- */
-export default (staticClasses: Classes, dynamicClasses: Classes) => {
-  const combinedClasses = {...staticClasses}
-
-  for (const name in dynamicClasses) {
-    combinedClasses[name] = staticClasses[name]
-      ? `${staticClasses[name]} ${dynamicClasses[name]}`
-      : dynamicClasses[name]
-  }
-
-  return combinedClasses
-}

diff --git a/packages/react-jss/src/createHoc.js b/packages/react-jss/src/createHoc.js
@@ -3,12 +3,13 @@
 import React, {Component, type ComponentType} from 'react'
 import PropTypes from 'prop-types'
 import {ThemeContext} from 'theming'
-import {getDynamicStyles, SheetsManager, type StyleSheet, type Classes} from 'jss'
+import {getDynamicStyles, SheetsManager, type StyleSheet} from 'jss'
 import defaultJss from './jss'
-import compose from './compose'
+import mergeClasses from './merge-classes'
 import getDisplayName from './getDisplayName'
 import JssContext from './JssContext'
 import type {Options, Theme, StylesOrCreator, InnerProps, OuterProps} from './types'
+import memoize from './memoize-one'
 
 // Like a Symbol
 const dynamicStylesNs = Math.random()
@@ -66,9 +67,6 @@ export default function createHOC<
   const manager = new SheetsManager()
   const ThemeConsumer = (theming && theming.context.Consumer) || ThemeContext.Consumer
 
-  // $FlowFixMe: DefaultProps is missing in type definitions
-  const {classes: defaultClasses = {}, ...defaultProps} = {...InnerComponent.defaultProps}
-
   const getTheme = props => (isThemingEnabled && props.theme ? props.theme : noTheme)
 
   class Jss extends Component<OuterPropsType, State> {
@@ -78,7 +76,14 @@ export default function createHOC<
       innerRef: PropTypes.func
     }
 
-    static defaultProps = defaultProps
+    // $FlowFixMe
+    static defaultProps = {...InnerComponent.defaultProps}
+
+    mergeClassesProp = memoize(classesProp => {
+      const {classes} = this.state
+
+      return classesProp ? mergeClasses(classes, classesProp) : classes
+    })
 
     constructor(props: OuterPropsType) {
       super(props)
@@ -196,20 +201,6 @@ export default function createHOC<
       }
     }
 
-    computeClasses(staticSheet: StyleSheet, dynamicSheet?: StyleSheet): Classes {
-      const jssClasses = dynamicSheet
-        ? compose(
-            staticSheet.classes,
-            dynamicSheet.classes
-          )
-        : staticSheet.classes
-      return {
-        ...defaultClasses,
-        ...jssClasses,
-        ...this.props.classes
-      }
-    }
-
     createState(): State {
       if (this.props.jssContext.disableStylesGeneration) {
         return {classes: {}}
@@ -221,22 +212,22 @@ export default function createHOC<
       return {
         staticSheet,
         dynamicSheet,
-        classes: this.computeClasses(staticSheet, dynamicSheet)
+        classes: mergeClasses(staticSheet.classes, dynamicSheet ? dynamicSheet.classes : {})
       }
     }
 
     render() {
-      const {classes} = this.state
       const {
         innerRef,
         jssContext,
         theme,
+        classes,
         // $FlowFixMe: Flow complains for no reason...
         ...props
       } = this.props
 
-      // We have merged classes already.
-      props.classes = classes
+      // Merge the class names for the user into the sheet classes
+      props.classes = this.mergeClassesProp(classes)
 
       if (innerRef) props.ref = innerRef
       if (injectTheme) props.theme = theme

diff --git a/packages/react-jss/src/memoize-one.js b/packages/react-jss/src/memoize-one.js
@@ -0,0 +1,17 @@
+// @flow
+
+const memoize = <Arg, Return>(fn: (arg: Arg) => Return) => {
+  let lastArg
+  let lastResult
+
+  return (arg: Arg): Return => {
+    if (typeof lastArg === 'undefined' || lastArg !== arg) {
+      lastArg = arg
+      lastResult = fn(arg)
+    }
+
+    return lastResult
+  }
+}
+
+export default memoize
diff --git a/packages/react-jss/src/merge-classes.js b/packages/react-jss/src/merge-classes.js
@@ -0,0 +1,17 @@
+// @flow
+import type {Classes} from 'jss'
+
+function mergeClasses(baseClasses: Classes, additionalClasses: Classes) {
+  const combinedClasses = {...baseClasses}
+
+  for (const name in additionalClasses) {
+    combinedClasses[name] =
+      name in combinedClasses
+        ? `${combinedClasses[name]} ${additionalClasses[name]}`
+        : additionalClasses[name]
+  }
+
+  return combinedClasses
+}
+
+export default mergeClasses
diff --git a/packages/react-jss/src/compose.test.js → packages/react-jss/src/merge-classes.test.js b/packages/react-jss/src/compose.test.js → packages/react-jss/src/merge-classes.test.js
@@ -1,8 +1,8 @@
 import expect from 'expect.js'
-import compose from './compose'
+import compose from './merge-classes'
 
-describe('compose', () => {
-  it('should compose two class objects', () => {
+describe('react-jss: merge-classes', () => {
+  it('should merge two class objects', () => {
     const staticClasses = {
       a: 'a',
       b: 'b'