Skip to content

Commit

Permalink
Merge pull request #563 from airbnb/fix_zero_propvalue
Browse files Browse the repository at this point in the history
Fix valid + falsy propvalues
  • Loading branch information
ljharb authored Aug 28, 2016
2 parents ae75be8 + 0314e2f commit b566033
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 40 deletions.
22 changes: 3 additions & 19 deletions src/MountedTraversal.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@ import isEmpty from 'lodash/isEmpty';
import isSubset from 'is-subset';
import {
internalInstance,
coercePropValue,
nodeEqual,
propsOfNode,
isFunctionalComponent,
Expand All @@ -12,6 +11,7 @@ import {
AND,
SELECTOR,
nodeHasType,
nodeHasProperty,
} from './Utils';
import {
isDOMComponent,
Expand Down Expand Up @@ -85,26 +85,10 @@ export function instHasType(inst, type) {

export function instHasProperty(inst, propKey, stringifiedPropValue) {
if (!isDOMComponent(inst)) return false;
const node = getNode(inst);
const nodeProps = propsOfNode(node);
const descriptor = Object.getOwnPropertyDescriptor(nodeProps, propKey);
if (descriptor && descriptor.get) {
return false;
}
const nodePropValue = nodeProps[propKey];

const propValue = coercePropValue(propKey, stringifiedPropValue);

// intentionally not matching node props that are undefined
if (nodePropValue === undefined) {
return false;
}

if (propValue) {
return nodePropValue === propValue;
}
const node = getNode(inst);

return Object.prototype.hasOwnProperty.call(nodeProps, propKey);
return nodeHasProperty(node, propKey, stringifiedPropValue);
}

// called with private inst
Expand Down
22 changes: 2 additions & 20 deletions src/ShallowTraversal.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,14 +2,14 @@ import React from 'react';
import isEmpty from 'lodash/isEmpty';
import isSubset from 'is-subset';
import {
coercePropValue,
propsOfNode,
splitSelector,
isCompoundSelector,
selectorType,
AND,
SELECTOR,
nodeHasType,
nodeHasProperty,
} from './Utils';


Expand Down Expand Up @@ -84,25 +84,7 @@ export function nodeHasId(node, id) {
}


export function nodeHasProperty(node, propKey, stringifiedPropValue) {
const nodeProps = propsOfNode(node);
const propValue = coercePropValue(propKey, stringifiedPropValue);
const descriptor = Object.getOwnPropertyDescriptor(nodeProps, propKey);
if (descriptor && descriptor.get) {
return false;
}
const nodePropValue = nodeProps[propKey];

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

if (propValue) {
return nodePropValue === propValue;
}

return Object.prototype.hasOwnProperty.call(nodeProps, propKey);
}
export { nodeHasProperty };

export function nodeMatchesObjectProps(node, props) {
return isSubset(propsOfNode(node), props);
Expand Down
36 changes: 35 additions & 1 deletion src/Utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -213,6 +213,19 @@ export function coercePropValue(propName, propValue) {
return propValue;
}

// can be the empty string
if (propValue === '') {
return propValue;
}

if (propValue === 'NaN') {
return NaN;
}

if (propValue === 'null') {
return null;
}

const trimmedValue = propValue.trim();

// if propValue includes quotes, it should be
Expand All @@ -224,7 +237,7 @@ export function coercePropValue(propName, propValue) {
const numericPropValue = +trimmedValue;

// if parseInt is not NaN, then we've wanted a number
if (!isNaN(numericPropValue)) {
if (!is(NaN, numericPropValue)) {
return numericPropValue;
}

Expand All @@ -239,6 +252,27 @@ export function coercePropValue(propName, propValue) {
);
}

export function nodeHasProperty(node, propKey, stringifiedPropValue) {
const nodeProps = propsOfNode(node);
const descriptor = Object.getOwnPropertyDescriptor(nodeProps, propKey);
if (descriptor && descriptor.get) {
return false;
}
const nodePropValue = nodeProps[propKey];

const propValue = coercePropValue(propKey, stringifiedPropValue);

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

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

return Object.prototype.hasOwnProperty.call(nodeProps, propKey);
}

export function mapNativeEventNames(event) {
const nativeToReactEventMap = {
compositionend: 'compositionEnd',
Expand Down
36 changes: 36 additions & 0 deletions test/ShallowTraversal-spec.jsx
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,42 @@ describe('ShallowTraversal', () => {
expect(nodeHasProperty(<div foo={-Infinity} />, 'foo', '-Infinity')).to.equal(true);
});

it('should parse zeroes properly', () => {
expect(nodeHasProperty(<div foo={0} />, 'foo', '0')).to.equal(true);
expect(nodeHasProperty(<div foo={-0} />, 'foo', '-0')).to.equal(true);
expect(nodeHasProperty(<div foo={1} />, 'foo', '0')).to.equal(false);
expect(nodeHasProperty(<div foo={2} />, 'foo', '-0')).to.equal(false);
});

it('should work with empty strings', () => {
expect(nodeHasProperty(<div foo={''} />, 'foo', '')).to.equal(true);
expect(nodeHasProperty(<div foo={''} />, 'foo', '""')).to.equal(true);
expect(nodeHasProperty(<div foo={'bar'} />, 'foo', '')).to.equal(false);
expect(nodeHasProperty(<div foo={'bar'} />, 'foo', '""')).to.equal(false);
});

it('should work with NaN', () => {
expect(nodeHasProperty(<div foo={NaN} />, 'foo', 'NaN')).to.equal(true);
expect(nodeHasProperty(<div foo={0} />, 'foo', 'NaN')).to.equal(false);
});

it('should work with null', () => {
expect(nodeHasProperty(<div foo={null} />, 'foo', 'null')).to.equal(true);
expect(nodeHasProperty(<div foo={0} />, 'foo', 'null')).to.equal(false);
});

it('should work with false', () => {
expect(nodeHasProperty(<div foo={false} />, 'foo', 'false')).to.equal(true);
expect(nodeHasProperty(<div foo={0} />, 'foo', 'false')).to.equal(false);
});

it('should work with ±Infinity', () => {
expect(nodeHasProperty(<div foo={Infinity} />, 'foo', 'Infinity')).to.equal(true);
expect(nodeHasProperty(<div foo={0} />, 'foo', 'Infinity')).to.equal(false);
expect(nodeHasProperty(<div foo={-Infinity} />, 'foo', '-Infinity')).to.equal(true);
expect(nodeHasProperty(<div foo={0} />, 'foo', '-Infinity')).to.equal(false);
});

it('should throw when un unquoted string is passed in', () => {
const node = (<div title="foo" />);

Expand Down

0 comments on commit b566033

Please sign in to comment.