From 70a05ad1f1c2b3456fd847418e565a6d4afd7d99 Mon Sep 17 00:00:00 2001 From: Henri Date: Mon, 31 Dec 2018 13:48:10 +0100 Subject: [PATCH] [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 --- packages/react-jss/.size-snapshot.json | 30 +++++++------- packages/react-jss/src/compose.js | 32 --------------- packages/react-jss/src/createHoc.js | 39 +++++++------------ packages/react-jss/src/memoize-one.js | 17 ++++++++ packages/react-jss/src/merge-classes.js | 17 ++++++++ ...{compose.test.js => merge-classes.test.js} | 6 +-- 6 files changed, 67 insertions(+), 74 deletions(-) create mode 100644 packages/react-jss/src/memoize-one.js create mode 100644 packages/react-jss/src/merge-classes.js rename packages/react-jss/src/{compose.test.js => merge-classes.test.js} (71%) diff --git a/packages/react-jss/.size-snapshot.json b/packages/react-jss/.size-snapshot.json index afdfe3179..ddf2172ce 100644 --- 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 index 06428da59..e69de29bb 100644 --- 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 index 6c8db92ca..6b19b5428 100644 --- 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 { @@ -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 new file mode 100644 index 000000000..19681e131 --- /dev/null +++ b/packages/react-jss/src/memoize-one.js @@ -0,0 +1,17 @@ +// @flow + +const memoize = (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 new file mode 100644 index 000000000..4177e4a3c --- /dev/null +++ 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 b/packages/react-jss/src/merge-classes.test.js similarity index 71% rename from packages/react-jss/src/compose.test.js rename to packages/react-jss/src/merge-classes.test.js index 0571b5cb3..265c4d483 100644 --- a/packages/react-jss/src/compose.test.js +++ b/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'