diff --git a/src/__tests__/createTheming.test.js b/src/__tests__/createTheming.test.js index ac91d60..cb8f8f4 100644 --- a/src/__tests__/createTheming.test.js +++ b/src/__tests__/createTheming.test.js @@ -1,5 +1,6 @@ /* eslint-disable react/no-multi-comp */ -import React, { Fragment } from 'react'; + +import * as React from 'react'; import ReactDOM from 'react-dom'; import createTheming from '../createTheming'; @@ -26,8 +27,6 @@ describe('createTheming', () => { secondaryColor: '#ffffff', }; - const originalTheme = { ...lightTheme }; - const { ThemeProvider, withTheme, useTheme } = createTheming(darkTheme); it('provides theme prop with HOC', () => { @@ -115,7 +114,7 @@ describe('createTheming', () => { ); }); - it('allows to use ref on wrapped component', () => { + it('sets correct ref on wrapped component', () => { class Component extends React.Component { foo() { return 'bar'; @@ -125,12 +124,11 @@ describe('createTheming', () => { return null; } } + const WithThemeComponent = withTheme(Component); class Wrapper extends React.Component { componentDidMount() { - expect(typeof this.comp).toBe('object'); - expect(typeof this.comp.foo).toEqual('function'); expect(this.comp.foo()).toEqual('bar'); } @@ -153,71 +151,6 @@ describe('createTheming', () => { ); }); - it('Should set properly getWrappedInstance method', () => { - class Component extends React.Component { - foo() { - return 'bar'; - } - - getWrappedInstance() { - return this; - } - - render() { - return null; - } - } - class ComponentWithoutGWI extends React.Component { - foo() { - return 'bar'; - } - - render() { - return null; - } - } - const WithThemeComponent = withTheme(Component); - const WithThemeComponentWithoutGWI = withTheme(ComponentWithoutGWI); - - class Wrapper extends React.Component { - componentDidMount() { - expect(typeof this.comp).toBe('object'); - expect(typeof this.comp.getWrappedInstance).toEqual('function'); - expect(this.comp.getWrappedInstance().foo()).toEqual('bar'); - - expect(typeof this.compWithoutGWI).toBe('object'); - expect(typeof this.compWithoutGWI.getWrappedInstance).toEqual( - 'function' - ); - expect(this.compWithoutGWI.getWrappedInstance().foo()).toEqual('bar'); - } - - render() { - return ( - - { - this.comp = component; - }} - /> - { - this.compWithoutGWI = component; - }} - /> - - ); - } - } - - ReactDOM.render( - - - , - node - ); - }); - it('merge theme from provider and prop', () => { const PropsChecker = withTheme(({ theme }) => { expect(theme).not.toBe(lightTheme); @@ -290,18 +223,17 @@ describe('createTheming', () => { }); it('doesnt mutate existing theme', () => { + const overrides = { primaryColor: 'red' }; const Checker1 = withTheme(({ theme }) => { - expect(theme).toEqual({ - ...lightTheme, - primaryColor: 'red', - }); + expect(theme).not.toBe(lightTheme); + expect(theme).not.toBe(overrides); return null; }); const Checker1WithTheme = withTheme(Checker1); const Checker2 = withTheme(({ theme }) => { - expect(theme).toEqual(originalTheme); + expect(theme).toBe(lightTheme); return null; }); @@ -309,7 +241,7 @@ describe('createTheming', () => { ReactDOM.render( - + , node diff --git a/src/createWithTheme.js b/src/createWithTheme.js index 900079f..13cd668 100644 --- a/src/createWithTheme.js +++ b/src/createWithTheme.js @@ -4,14 +4,9 @@ import * as React from 'react'; import deepmerge from 'deepmerge'; import hoistNonReactStatics from 'hoist-non-react-statics'; -import { copyRefs } from './utils'; - import type { ThemeProviderType } from './createThemeProvider'; import type { $DeepShape } from './types'; -const isClassComponent = (Component: any) => - Boolean(Component.prototype && Component.prototype.isReactComponent); - export type WithThemeType = >( Comp: C ) => C & @@ -25,8 +20,6 @@ const createWithTheme = >( ) => function withTheme(Comp: *) { class ThemedComponent extends React.Component<*> { - static displayName = `withTheme(${Comp.displayName || Comp.name})`; - _previous: ?{ a: T, b: ?S, result: T }; _merge = (a: T, b: ?S) => { @@ -36,39 +29,29 @@ const createWithTheme = >( return previous.result; } - const result = a && b ? deepmerge(a, b) : a || b; + const result = a && b && a !== b ? deepmerge(a, b) : a || b; this._previous = { a, b, result }; return result; }; - _root: any; - render() { + const { _reactThemeProviderForwardedRef, ...rest } = this.props; + return ( {theme => { - const merged = this._merge(theme, this.props.theme); - - let element; - if (isClassComponent(Comp)) { - // Only add refs for class components as function components don't support them - // It's needed to support use cases which need access to the underlying node - element = ( - { - this._root = c; - }} - theme={merged} - /> - ); - } else { - element = ; - } - - if (merged !== this.props.theme) { + const merged = this._merge(theme, rest.theme); + const element = ( + + ); + + if (rest.theme && merged !== rest.theme) { // If a theme prop was passed, expose it to the children return {element}; } @@ -80,23 +63,15 @@ const createWithTheme = >( } } - if (isClassComponent(Comp)) { - // getWrappedInstance is exposed by some HOCs like react-redux's connect - // Use it to get the ref to the underlying element - // Also expose it to access the underlying element after wrapping - // $FlowFixMe - ThemedComponent.prototype.getWrappedInstance = function getWrappedInstance() { - return this._root && this._root.getWrappedInstance - ? this._root.getWrappedInstance() - : this._root; - }; + const ResultComponent = React.forwardRef((props, ref) => ( + + )); - ThemedComponent = copyRefs(ThemedComponent, Comp); - } + ResultComponent.displayName = `withTheme(${Comp.displayName || Comp.name})`; - hoistNonReactStatics(ThemedComponent, Comp); + hoistNonReactStatics(ResultComponent, Comp); - return (ThemedComponent: any); + return (ResultComponent: any); }; export default createWithTheme; diff --git a/src/utils.js b/src/utils.js deleted file mode 100644 index 302f550..0000000 --- a/src/utils.js +++ /dev/null @@ -1,81 +0,0 @@ -/* @flow */ - -import * as React from 'react'; - -const REACT_METHODS = [ - 'autobind', - 'childContextTypes', - 'componentDidMount', - 'componentDidUpdate', - 'componentWillMount', - 'componentWillReceiveProps', - 'componentWillUnmount', - 'componentWillUpdate', - 'contextTypes', - 'displayName', - 'getChildContext', - 'getDefaultProps', - 'getDOMNode', - 'getInitialState', - 'mixins', - 'propTypes', - 'render', - 'replaceProps', - 'setProps', - 'shouldComponentUpdate', - 'statics', - 'updateComponent', -]; - -export function copyRefs( - TargetComponent: React.ComponentType, - SourceComponent: React.ComponentType -): React.ComponentType { - // $FlowFixMe - if (!SourceComponent.prototype) { - return TargetComponent; - } - - // $FlowFixMe - Object.getOwnPropertyNames(SourceComponent.prototype) - .filter( - prop => - !( - REACT_METHODS.includes(prop) || // React specific methods and properties - prop in React.Component.prototype || // Properties from React's prototype such as `setState` - // $FlowFixMe - prop in TargetComponent.prototype || // Properties from enhanced component's prototype - // Private methods - prop.startsWith('_') - ) - ) - .forEach(prop => { - // $FlowFixMe - if (typeof SourceComponent.prototype[prop] === 'function') { - /* eslint-disable func-names, no-param-reassign */ - // $FlowFixMe - TargetComponent.prototype[prop] = function(...args) { - // Make sure the function is called with correct context - // $FlowFixMe - return SourceComponent.prototype[prop].apply( - this.getWrappedInstance(), - args - ); - }; - } else { - // Copy properties as getters and setters - // This make sure dynamic properties always stay up-to-date - // $FlowFixMe - Object.defineProperty(TargetComponent.prototype, prop, { - get() { - return this.getWrappedInstance()[prop]; - }, - set(value) { - this.getWrappedInstance()[prop] = value; - }, - }); - } - }); - - return TargetComponent; -}