From 5384bb63c97a197613248ea03672f053efbcb399 Mon Sep 17 00:00:00 2001 From: Tzook Date: Wed, 16 Aug 2017 13:12:56 +0300 Subject: [PATCH] [Fix] SetProps - Children objects should be new instances --- src/ReactWrapper.jsx | 8 +++++++ src/Utils.js | 18 +++++++++++++++ test/ReactWrapper-spec.jsx | 46 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 72 insertions(+) diff --git a/src/ReactWrapper.jsx b/src/ReactWrapper.jsx index af3e22009..47f30baa9 100644 --- a/src/ReactWrapper.jsx +++ b/src/ReactWrapper.jsx @@ -14,6 +14,8 @@ import { nodeEqual, nodeMatches, getAdapter, + shouldCloneChildren, + cloneChildren, } from './Utils'; import { debugNodes, @@ -303,6 +305,12 @@ class ReactWrapper { if (typeof callback !== 'function') { throw new TypeError('ReactWrapper::setProps() expects a function as its second argument'); } + const instance = this.getNodeInternal(); + const adapter = getAdapter(this.options); + if (shouldCloneChildren(instance, props, adapter)) { + props = props || {}; // eslint-disable-line no-param-reassign + props.children = cloneChildren(instance); // eslint-disable-line no-param-reassign + } this.component.setChildProps(props, () => { this.update(); callback(); diff --git a/src/Utils.js b/src/Utils.js index 63079867b..e0fea5414 100644 --- a/src/Utils.js +++ b/src/Utils.js @@ -1,4 +1,5 @@ /* eslint no-use-before-define:0 */ +import React from 'react'; import isEqual from 'lodash/isEqual'; import is from 'object-is'; import uuidv4 from 'uuid/v4'; @@ -185,6 +186,10 @@ function childrenOfNode(node) { return childrenToArray(children); } +function propsHaveChildren(props) { + return !!(props || {}).children; +} + function isTextualNode(node) { return typeof node === 'string' || typeof node === 'number'; } @@ -193,6 +198,10 @@ export function isReactElementAlike(arg, adapter) { return adapter.isValidElement(arg) || isTextualNode(arg) || Array.isArray(arg); } +function isReactObjectAlike(arg, adapter) { + return adapter.isValidElement(arg) || Array.isArray(arg); +} + // TODO(lmr): can we get rid of this outside of the adapter? export function withSetStateAllowed(fn) { // NOTE(lmr): @@ -368,3 +377,12 @@ export function displayNameOfNode(node) { return type.displayName || (typeof type === 'function' ? functionName(type) : type.name || type); } + +export function shouldCloneChildren(node, newProps, adapter) { + // if our new props don't have children and we have an array we should clone the children + return !propsHaveChildren(newProps) && isReactObjectAlike(propsOfNode(node).children, adapter); +} + +export function cloneChildren(node) { + return React.Children.map(node.props.children, (child => React.cloneElement(child))); +} diff --git a/test/ReactWrapper-spec.jsx b/test/ReactWrapper-spec.jsx index 9a5983d17..73f7ba89b 100644 --- a/test/ReactWrapper-spec.jsx +++ b/test/ReactWrapper-spec.jsx @@ -999,6 +999,52 @@ describeWithDOM('mount', () => { expect(wrapper.props().d).to.equal('e'); }); + describe('pure components with children', () => { + let PureComponent; + beforeEach(() => { + class Foo extends React.Component { // pure component was introduced only in react 15 + shouldComponentUpdate(nextProps) { + return this.props.children !== nextProps.children; + } + render() { + this.renderedCount = (this.renderedCount || 0) + 1; + return ( +
{this.props.children}
+ ); + } + } + PureComponent = Foo; + }); + + it('should not render for nothing when children are null', () => { + const wrapper = mount(); + expect(wrapper.instance().renderedCount).to.equal(1); + wrapper.setProps({}); + expect(wrapper.instance().renderedCount).to.equal(1); + }); + + it('should not render for nothing when children are string', () => { + const wrapper = mount(String children); + expect(wrapper.instance().renderedCount).to.equal(1); + wrapper.setProps({}); + expect(wrapper.instance().renderedCount).to.equal(1); + }); + + it('should render for nothing because react sends a new children object', () => { + const wrapper = mount(
Object children
); + expect(wrapper.instance().renderedCount).to.equal(1); + wrapper.setProps({}); + expect(wrapper.instance().renderedCount).to.equal(2); + }); + + it('should render for nothing because react sends a new children array', () => { + const wrapper = mount(
Child 1
Child 2
); + expect(wrapper.instance().renderedCount).to.equal(1); + wrapper.setProps({}); + expect(wrapper.instance().renderedCount).to.equal(2); + }); + }); + itIf(!REACT16, 'should throw if an exception occurs during render', () => { class Trainwreck extends React.Component { render() {