Skip to content

Commit

Permalink
Merge pull request #662 from vadimdemedes/skip-null-or-undefined-props
Browse files Browse the repository at this point in the history
[Breaking] Skip undefined props when comparing nodes
  • Loading branch information
ljharb committed Sep 26, 2017
2 parents b355eed + 9bb8a91 commit efb3913
Show file tree
Hide file tree
Showing 14 changed files with 82 additions and 21 deletions.
2 changes: 1 addition & 1 deletion docs/api/ShallowWrapper/equals.md
Original file line number Diff line number Diff line change
Expand Up @@ -32,4 +32,4 @@ expect(wrapper.equals(<div className="foo bar" />)).to.equal(true);
when you are calling it you are calling it with a ReactElement or a JSX expression.
- Keep in mind that this method determines equality based on the equality of the node's children as
well.

- Following React's behavior, `.equals()` ignores properties whose values are `undefined`.
15 changes: 13 additions & 2 deletions docs/api/selector.md
Original file line number Diff line number Diff line change
Expand Up @@ -85,10 +85,10 @@ a string can be used to find it:
function MyComponent() {
return <div />;
}
MyComponent.displayName = 'MyComponent!';
MyComponent.displayName = 'My Component';

// find instances of MyComponent
const myComponents = wrapper.find('MyComponent!');
const myComponents = wrapper.find('My Component');
```

NOTE: This will *only* work if the selector (and thus the component's `displayName`) is a string
Expand All @@ -113,3 +113,14 @@ wrapper.find({ foo: 3 });
wrapper.find({ bar: false });
wrapper.find({ title: 'baz' });
```

**Note - undefined properties**
are not allowed in the object property selector and will cause an error:


```jsx
wrapper.find({ foo: 3, bar: undefined });
// => TypeError: Enzyme::Props can't have 'undefined' values. Try using 'findWhere()' instead.
```

If you have to search by `undefined` property value, use [.findWhere()](ShallowWrapper/findWhere.md).
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -104,5 +104,6 @@
"sinon": "^2.4.1",
"webpack": "^1.13.3"
},
"dependencies": {}
"dependencies": {
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ export default function elementToTree(el) {
let rendered = null;
if (Array.isArray(children)) {
rendered = flatten(children, true).map(elementToTree);
} else if (children !== undefined) {
} else if (typeof children !== 'undefined') {
rendered = elementToTree(children);
}
return {
Expand Down
2 changes: 1 addition & 1 deletion packages/enzyme-adapter-utils/src/Utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,7 @@ export function elementToTree(el) {
let rendered = null;
if (Array.isArray(children)) {
rendered = flatten(children, true).map(elementToTree);
} else if (children !== undefined) {
} else if (typeof children !== 'undefined') {
rendered = elementToTree(children);
}
return {
Expand Down
14 changes: 13 additions & 1 deletion packages/enzyme-test-suite/test/ReactWrapper-spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -469,6 +469,19 @@ describeWithDOM('mount', () => {
expect(wrapper.find('[htmlFor]')).to.have.length(2);
});

it('should error sensibly if any of the search props are undefined', () => {
const wrapper = mount((
<div>
<input type={undefined} />
</div>
));

expect(() => wrapper.find({ type: undefined })).to.throw(
TypeError,
'Enzyme::Props can’t have `undefined` values. Try using ‘findWhere()’ instead.',
);
});

it('should compound tag and prop selector', () => {
const wrapper = mount(
<div>
Expand Down Expand Up @@ -655,7 +668,6 @@ describeWithDOM('mount', () => {
expect(wrapper.find({ a: 1 })).to.have.length(0);
expect(wrapper.find({ 'data-test': 'ref' })).to.have.length(7);
expect(wrapper.find({ className: 'foo' })).to.have.length(1);
expect(wrapper.find({ 'data-prop': undefined })).to.have.length(1);
expect(wrapper.find({ 'data-prop': null })).to.have.length(1);
expect(wrapper.find({ 'data-prop': 123 })).to.have.length(1);
expect(wrapper.find({ 'data-prop': false })).to.have.length(1);
Expand Down
14 changes: 13 additions & 1 deletion packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -603,6 +603,19 @@ describe('shallow', () => {
);
});

it('should error sensibly if any of the search props are undefined', () => {
const wrapper = shallow(
<div>
<input type={undefined} />
</div>,
);

expect(() => wrapper.find({ type: undefined })).to.throw(
TypeError,
'Enzyme::Props can’t have `undefined` values. Try using ‘findWhere()’ instead.',
);
});

it('should compound tag and prop selector', () => {
const wrapper = shallow(
<div>
Expand Down Expand Up @@ -695,7 +708,6 @@ describe('shallow', () => {
expect(wrapper.find({ a: 1 })).to.have.length(0);
expect(wrapper.find({ 'data-test': 'ref' })).to.have.length(7);
expect(wrapper.find({ className: 'foo' })).to.have.length(1);
expect(wrapper.find({ prop: undefined })).to.have.length(1);
expect(wrapper.find({ prop: null })).to.have.length(1);
expect(wrapper.find({ prop: 123 })).to.have.length(1);
expect(wrapper.find({ prop: false })).to.have.length(1);
Expand Down
7 changes: 7 additions & 0 deletions packages/enzyme-test-suite/test/Utils-spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,13 @@ describe('Utils', () => {
)).to.equal(false);
});

it('should skip undefined props', () => {
expect(nodeEqual(
<div id="foo" className={undefined} />,
<div id="foo" />,
)).to.equal(true);
});

it('should check children as well', () => {
expect(nodeEqual(
<div>
Expand Down
1 change: 1 addition & 0 deletions packages/enzyme/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"object-is": "^1.0.1",
"object.assign": "^4.0.4",
"object.entries": "^1.0.4",
"object.values": "^1.0.4",
"raf": "^3.3.2",
"rst-selector-parser": "^2.2.1"
},
Expand Down
20 changes: 15 additions & 5 deletions packages/enzyme/src/RSTTraversal.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import flatten from 'lodash/flatten';
import entries from 'object.entries';
import isSubset from 'is-subset';
import functionName from 'function.prototype.name';
import { nodeHasProperty } from './Utils';
Expand Down Expand Up @@ -39,11 +40,11 @@ export function treeFilter(tree, fn) {
* To support sibling selectors we need to be able to find
* the siblings of a node. The easiest way to do that is find
* the parent of the node and access its children.
*
*
* This would be unneeded if the RST spec included sibling pointers
* such as node.nextSibling and node.prevSibling
* @param {*} root
* @param {*} targetNode
* @param {*} root
* @param {*} targetNode
*/
export function findParentNode(root, targetNode) {
const results = treeFilter(
Expand Down Expand Up @@ -98,12 +99,21 @@ export function nodeHasId(node, id) {

export { nodeHasProperty };

const CAN_NEVER_MATCH = {};
function replaceUndefined(v) {
return typeof v !== 'undefined' ? v : CAN_NEVER_MATCH;
}
function replaceUndefinedValues(obj) {
return entries(obj)
.reduce((acc, [k, v]) => ({ ...acc, [k]: replaceUndefined(v) }), {});
}

export function nodeMatchesObjectProps(node, props) {
return isSubset(propsOfNode(node), props);
return isSubset(propsOfNode(node), replaceUndefinedValues(props));
}

export function getTextFromNode(node) {
if (node === null || node === undefined) {
if (node == null) {
return '';
}

Expand Down
4 changes: 2 additions & 2 deletions packages/enzyme/src/ReactWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -611,7 +611,7 @@ class ReactWrapper {
throw new Error('ReactWrapper::state() can only be called on the root');
}
const _state = this.single('state', () => this.instance().state);
if (name !== undefined) {
if (typeof name !== 'undefined') {
return _state[name];
}
return _state;
Expand All @@ -635,7 +635,7 @@ class ReactWrapper {
throw new Error('ReactWrapper::context() can only be called on components with instances');
}
const _context = instance.context;
if (name !== undefined) {
if (typeof name !== 'undefined') {
return _context[name];
}
return _context;
Expand Down
2 changes: 1 addition & 1 deletion packages/enzyme/src/ShallowWrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -699,7 +699,7 @@ class ShallowWrapper {
throw new Error('ShallowWrapper::state() can only be called on class components');
}
const _state = this.single('state', () => this.instance().state);
if (name !== undefined) {
if (typeof name !== 'undefined') {
return _state[name];
}
return _state;
Expand Down
12 changes: 7 additions & 5 deletions packages/enzyme/src/Utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,9 @@ export function isCustomComponentElement(inst, adapter) {
}

function propsOfNode(node) {
return (node && node.props) || {};
return entries((node && node.props) || {})
.filter(([, value]) => typeof value !== 'undefined')
.reduce((acc, [key, value]) => Object.assign(acc, { [key]: value }), {});
}

export function typeOfNode(node) {
Expand Down Expand Up @@ -139,7 +141,7 @@ function childrenToArray(children) {
const result = [];

const push = (el) => {
if (el === null || el === false || el === undefined) return;
if (el === null || el === false || typeof el === 'undefined') return;
result.push(el);
};

Expand All @@ -159,7 +161,7 @@ export function childrenToSimplifiedArray(nodeChildren) {
const child = childrenArray[i];
const previousChild = simplifiedArray.pop();

if (previousChild === undefined) {
if (typeof previousChild === 'undefined') {
simplifiedArray.push(child);
} else if (isTextualNode(child) && isTextualNode(previousChild)) {
simplifiedArray.push(previousChild + child);
Expand Down Expand Up @@ -218,11 +220,11 @@ export function nodeHasProperty(node, propKey, propValue) {
}
const nodePropValue = nodeProps[propKey];

if (nodePropValue === undefined) {
if (typeof nodePropValue === 'undefined') {
return false;
}

if (propValue !== undefined) {
if (typeof propValue !== 'undefined') {
return is(nodePropValue, propValue);
}

Expand Down
5 changes: 5 additions & 0 deletions packages/enzyme/src/selectors.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
import { createParser } from 'rst-selector-parser';
import values from 'object.values';
import isEmpty from 'lodash/isEmpty';
import flatten from 'lodash/flatten';
import unique from 'lodash/uniq';
Expand Down Expand Up @@ -141,6 +142,10 @@ export function buildPredicate(selector) {
// If the selector is an non-empty object, treat the keys/values as props
if (typeof selector === 'object') {
if (!Array.isArray(selector) && selector !== null && !isEmpty(selector)) {
const hasUndefinedValues = values(selector).some(value => typeof value === 'undefined');
if (hasUndefinedValues) {
throw new TypeError('Enzyme::Props can’t have `undefined` values. Try using ‘findWhere()’ instead.');
}
return node => nodeMatchesObjectProps(node, selector);
}
throw new TypeError(
Expand Down

0 comments on commit efb3913

Please sign in to comment.