Skip to content

Commit

Permalink
[Fix] SetProps - Children objects should be new instances
Browse files Browse the repository at this point in the history
  • Loading branch information
Tzook committed Aug 16, 2017
1 parent c27eab4 commit 5384bb6
Show file tree
Hide file tree
Showing 3 changed files with 72 additions and 0 deletions.
8 changes: 8 additions & 0 deletions src/ReactWrapper.jsx
Expand Up @@ -14,6 +14,8 @@ import {
nodeEqual,
nodeMatches,
getAdapter,
shouldCloneChildren,
cloneChildren,
} from './Utils';
import {
debugNodes,
Expand Down Expand Up @@ -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();
Expand Down
18 changes: 18 additions & 0 deletions 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';
Expand Down Expand Up @@ -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';
}
Expand All @@ -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):
Expand Down Expand Up @@ -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)));
}
46 changes: 46 additions & 0 deletions test/ReactWrapper-spec.jsx
Expand Up @@ -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 (
<div>{this.props.children}</div>
);
}
}
PureComponent = Foo;
});

it('should not render for nothing when children are null', () => {
const wrapper = mount(<PureComponent />);
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(<PureComponent>String children</PureComponent>);
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(<PureComponent><div>Object children</div></PureComponent>);
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(<PureComponent><div>Child 1</div><div>Child 2</div></PureComponent>);
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() {
Expand Down

0 comments on commit 5384bb6

Please sign in to comment.