Merge pull request #130 from airbnb/lmr--fix-prop-selector-string-issue

Fixed confusing prop selector behavior. Added helpful warning

Author: lelandrichardson

src/MountedTraversal.js

@@ -77,7 +77,7 @@ export function instHasProperty(inst, propKey, stringifiedPropValue) {
   const nodeProps = propsOfNode(node);
   const nodePropValue = nodeProps[propKey];
 
-  const propValue = coercePropValue(stringifiedPropValue);
+  const propValue = coercePropValue(propKey, stringifiedPropValue);
 
   // intentionally not matching node props that are undefined
   if (nodePropValue === undefined) {

src/ShallowTraversal.js

@@ -75,7 +75,7 @@ export function nodeHasId(node, id) {
 
 export function nodeHasProperty(node, propKey, stringifiedPropValue) {
   const nodeProps = propsOfNode(node);
-  const propValue = coercePropValue(stringifiedPropValue);
+  const propValue = coercePropValue(propKey, stringifiedPropValue);
   const nodePropValue = nodeProps[propKey];
 
   if (nodePropValue === undefined) {

src/Utils.js

@@ -149,27 +149,36 @@ export function AND(fns) {
   };
 }
 
-export function coercePropValue(propValue) {
+export function coercePropValue(propName, propValue) {
   // can be undefined
   if (propValue === undefined) {
     return propValue;
   }
 
+  const trimmedValue = propValue.trim();
+
   // if propValue includes quotes, it should be
   // treated as a string
-  if (propValue.search(/"/) !== -1) {
-    return propValue.replace(/"/g, '');
+  if (/^(['"]).*\1$/.test(trimmedValue)) {
+    return trimmedValue.slice(1, -1);
   }
 
-  const numericPropValue = parseInt(propValue, 10);
+  const numericPropValue = +trimmedValue;
 
   // if parseInt is not NaN, then we've wanted a number
   if (!isNaN(numericPropValue)) {
     return numericPropValue;
   }
 
   // coerce to boolean
-  return propValue === 'true' ? true : false;
+  if (trimmedValue === 'true') return true;
+  if (trimmedValue === 'false') return false;
+
+  // user provided an unquoted string value
+  throw new TypeError(
+    `Enzyme::Unable to parse selector '[${propName}=${propValue}]'. ` +
+    `Perhaps you forgot to escape a string? Try '[${propName}="${trimmedValue}"]' instead.`
+  );
 }
 
 export function mapNativeEventNames(event) {

src/__tests__/ShallowTraversal-spec.js

@@ -64,13 +64,13 @@ describe('ShallowTraversal', () => {
       const node = (<div onChange={noop} title="foo" />);
 
       expect(nodeHasProperty(node, 'onChange')).to.equal(true);
-      expect(nodeHasProperty(node, 'title', 'foo')).to.equal(true);
+      expect(nodeHasProperty(node, 'title', '"foo"')).to.equal(true);
     });
 
     it('should not match on html attributes', () => {
       const node = (<div htmlFor="foo" />);
 
-      expect(nodeHasProperty(node, 'for', 'foo')).to.equal(false);
+      expect(nodeHasProperty(node, 'for', '"foo"')).to.equal(false);
     });
 
     it('should not find undefined properties', () => {
@@ -79,6 +79,35 @@ describe('ShallowTraversal', () => {
       expect(nodeHasProperty(node, 'title')).to.equal(false);
     });
 
+    it('should parse false as a literal', () => {
+      const node = (<div foo={false} />);
+
+      expect(nodeHasProperty(node, 'foo', 'false')).to.equal(true);
+    });
+
+    it('should parse false as a literal', () => {
+      const node = (<div foo />);
+
+      expect(nodeHasProperty(node, 'foo', 'true')).to.equal(true);
+    });
+
+    it('should parse numbers as numeric literals', () => {
+      expect(nodeHasProperty(<div foo={2.3} />, 'foo', '2.3')).to.equal(true);
+      expect(nodeHasProperty(<div foo={2} />, 'foo', '2')).to.equal(true);
+      expect(() => nodeHasProperty(<div foo={2} />, 'foo', '2abc')).to.throw();
+      expect(() => nodeHasProperty(<div foo={2} />, 'foo', 'abc2')).to.throw();
+      expect(nodeHasProperty(<div foo={-2} />, 'foo', '-2')).to.equal(true);
+      expect(nodeHasProperty(<div foo={2e8} />, 'foo', '2e8')).to.equal(true);
+      expect(nodeHasProperty(<div foo={Infinity} />, 'foo', 'Infinity')).to.equal(true);
+      expect(nodeHasProperty(<div foo={-Infinity} />, 'foo', '-Infinity')).to.equal(true);
+    });
+
+    it('should throw when un unquoted string is passed in', () => {
+      const node = (<div title="foo" />);
+
+      expect(() => nodeHasProperty(node, 'title', 'foo')).to.throw();
+    });
+
   });
 
   describe('treeForEach', () => {

src/__tests__/ShallowWrapper-spec.js

@@ -169,6 +169,17 @@ describe('shallow', () => {
       expect(wrapper.find('[title]')).to.have.length(1);
     });
 
+    it('should error sensibly if prop selector without quotes', () => {
+      const wrapper = shallow(
+        <div>
+          <input type="text" />
+          <input type="hidden" />
+        </div>
+      );
+
+      expect(() => wrapper.find('[type=text]')).to.throw();
+    });
+
     it('should compound tag and prop selector', () => {
       const wrapper = shallow(
         <div>

src/__tests__/Utils-spec.js

@@ -227,19 +227,19 @@ describe('Utils', () => {
   });
 
   describe('coercePropValue', () => {
-
+    const key = 'foo';
     it('returns undefined if passed undefined', () => {
-      expect(coercePropValue(undefined)).to.equal(undefined);
+      expect(coercePropValue(key, undefined)).to.equal(undefined);
     });
 
     it('returns number if passed a stringified number', () => {
-      expect(coercePropValue('1')).to.be.equal(1);
-      expect(coercePropValue('0')).to.be.equal(0);
+      expect(coercePropValue(key, '1')).to.be.equal(1);
+      expect(coercePropValue(key, '0')).to.be.equal(0);
     });
 
     it('returns a boolean if passed a stringified bool', () => {
-      expect(coercePropValue('true')).to.equal(true);
-      expect(coercePropValue('false')).to.equal(false);
+      expect(coercePropValue(key, 'true')).to.equal(true);
+      expect(coercePropValue(key, 'false')).to.equal(false);
     });
   });