Merge pull request #662 from vadimdemedes/skip-null-or-undefined-props

ljharb · web-flow · commit efb3913fb47a · 2017-09-25T20:47:54.000-07:00
[Breaking] Skip undefined props when comparing nodes
diff --git a/docs/api/ShallowWrapper/equals.md b/docs/api/ShallowWrapper/equals.md
@@ -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`.
diff --git a/docs/api/selector.md b/docs/api/selector.md
@@ -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
@@ -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).
diff --git a/package.json b/package.json
@@ -104,5 +104,6 @@
     "sinon": "^2.4.1",
     "webpack": "^1.13.3"
   },
-  "dependencies": {}
+  "dependencies": {
+  }
 }
diff --git a/packages/enzyme-adapter-react-13/src/ReactThirteenElementToTree.js b/packages/enzyme-adapter-react-13/src/ReactThirteenElementToTree.js
@@ -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 {
diff --git a/packages/enzyme-adapter-utils/src/Utils.js b/packages/enzyme-adapter-utils/src/Utils.js
@@ -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 {
diff --git a/packages/enzyme-test-suite/test/ReactWrapper-spec.jsx b/packages/enzyme-test-suite/test/ReactWrapper-spec.jsx
@@ -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>
@@ -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);
diff --git a/packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx b/packages/enzyme-test-suite/test/ShallowWrapper-spec.jsx
@@ -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>
@@ -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);
diff --git a/packages/enzyme-test-suite/test/Utils-spec.jsx b/packages/enzyme-test-suite/test/Utils-spec.jsx
@@ -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>
diff --git a/packages/enzyme/package.json b/packages/enzyme/package.json
@@ -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"
   },
diff --git a/packages/enzyme/src/RSTTraversal.js b/packages/enzyme/src/RSTTraversal.js
@@ -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';
@@ -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(
@@ -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 '';
   }
 
diff --git a/packages/enzyme/src/ReactWrapper.js b/packages/enzyme/src/ReactWrapper.js
@@ -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;
@@ -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;
diff --git a/packages/enzyme/src/ShallowWrapper.js b/packages/enzyme/src/ShallowWrapper.js
@@ -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;
diff --git a/packages/enzyme/src/Utils.js b/packages/enzyme/src/Utils.js
@@ -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) {
@@ -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);
   };
 
@@ -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);
@@ -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);
   }
 
diff --git a/packages/enzyme/src/selectors.js b/packages/enzyme/src/selectors.js
@@ -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';
@@ -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(

Original file line number	Diff line number	Diff line change
`@@ -104,5 +104,6 @@`
`104`	`104`	`"sinon": "^2.4.1",`
`105`	`105`	`"webpack": "^1.13.3"`
`106`	`106`	`},`
`107`		`- "dependencies": {}`
	`107`	`+ "dependencies": {`
	`108`	`+ }`
`108`	`109`	`}`
Original file line number	Diff line number	Diff line change
`@@ -23,7 +23,7 @@ export default function elementToTree(el) {`
`23`	`23`	`let rendered = null;`
`24`	`24`	`if (Array.isArray(children)) {`
`25`	`25`	`rendered = flatten(children, true).map(elementToTree);`
`26`		`- } else if (children !== undefined) {`
	`26`	`+ } else if (typeof children !== 'undefined') {`
`27`	`27`	`rendered = elementToTree(children);`
`28`	`28`	`}`
`29`	`29`	`return {`
Original file line number	Diff line number	Diff line change
`@@ -104,7 +104,7 @@ export function elementToTree(el) {`
`104`	`104`	`let rendered = null;`
`105`	`105`	`if (Array.isArray(children)) {`
`106`	`106`	`rendered = flatten(children, true).map(elementToTree);`
`107`		`- } else if (children !== undefined) {`
	`107`	`+ } else if (typeof children !== 'undefined') {`
`108`	`108`	`rendered = elementToTree(children);`
`109`	`109`	`}`
`110`	`110`	`return {`