Skip to content

Commit

Permalink
allow to ignore value attribute for option (facebook#5362)
Browse files Browse the repository at this point in the history
  • Loading branch information
yiminghe authored and sophiebits committed May 9, 2016
1 parent 982e096 commit 904ee9a
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 32 deletions.
52 changes: 33 additions & 19 deletions src/renderers/dom/client/wrappers/ReactDOMOption.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,30 @@ var ReactDOMComponentTree = require('ReactDOMComponentTree');
var ReactDOMSelect = require('ReactDOMSelect');

var warning = require('warning');
var didWarnInvalidOptionChildren = false;

function flattenChildren(children) {
var content = '';

// Flatten children and warn if they aren't strings or numbers;
// invalid types are ignored.
ReactChildren.forEach(children, function(child) {
if (child == null) {
return;
}
if (typeof child === 'string' || typeof child === 'number') {
content += child;
} else if (!didWarnInvalidOptionChildren) {
didWarnInvalidOptionChildren = true;
warning(
false,
'Only strings and numbers are supported as <option> children.'
);
}
});

return content;
}

/**
* Implements an <option> native component that warns when `selected` is set.
Expand Down Expand Up @@ -49,17 +73,23 @@ var ReactDOMOption = {
// or missing (e.g., for <datalist>), we don't change props.selected
var selected = null;
if (selectValue != null) {
var value;
if (props.value != null) {
value = props.value + '';
} else {
value = flattenChildren(props.children);
}
selected = false;
if (Array.isArray(selectValue)) {
// multiple
for (var i = 0; i < selectValue.length; i++) {
if ('' + selectValue[i] === '' + props.value) {
if ('' + selectValue[i] === value) {
selected = true;
break;
}
}
} else {
selected = ('' + selectValue === '' + props.value);
selected = ('' + selectValue === value);
}
}

Expand All @@ -84,23 +114,7 @@ var ReactDOMOption = {
nativeProps.selected = inst._wrapperState.selected;
}

var content = '';

// Flatten children and warn if they aren't strings or numbers;
// invalid types are ignored.
ReactChildren.forEach(props.children, function(child) {
if (child == null) {
return;
}
if (typeof child === 'string' || typeof child === 'number') {
content += child;
} else {
warning(
false,
'Only strings and numbers are supported as <option> children.'
);
}
});
var content = flattenChildren(props.children);

if (content) {
nativeProps.children = content;
Expand Down
38 changes: 25 additions & 13 deletions src/renderers/dom/client/wrappers/__tests__/ReactDOMOption-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,28 +31,18 @@ describe('ReactDOMOption', function() {
expect(node.innerHTML).toBe('1 foo');
});

it('should ignore invalid children types', function() {
it('should ignore and warn invalid children types', function() {
spyOn(console, 'error');
var stub = <option>{1} <div /> {2}</option>;
stub = ReactTestUtils.renderIntoDocument(stub);
var node = ReactDOM.findDOMNode(stub);

expect(node.innerHTML).toBe('1 2');
ReactTestUtils.renderIntoDocument(<option>{1} <div /> {2}</option>);
// only warn once
expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain('Only strings and numbers are supported as <option> children.');
});

it('should warn when passing invalid children', function() {
var stub = <option>{1} <div /></option>;
spyOn(console, 'error');
stub = ReactTestUtils.renderIntoDocument(stub);

expect(console.error.calls.length).toBe(1);
expect(console.error.argsForCall[0][0]).toContain(
'Only strings and numbers are supported as <option> children.'
);
});

it('should ignore null/undefined/false children without warning', function() {
var stub = <option>{1} {false}{true}{null}{undefined} {2}</option>;
spyOn(console, 'error');
Expand Down Expand Up @@ -82,4 +72,26 @@ describe('ReactDOMOption', function() {
expect(option.hasAttribute('value')).toBe(true);
expect(option.getAttribute('value')).toBe('lava');
});

it('should allow ignoring `value` on option', function() {
var a = 'a';
var stub =
<select value="giraffe" onChange={() => {}}>
<option>monkey</option>
<option>gir{a}ffe</option>
<option>gorill{a}</option>
</select>;
var options = stub.props.children;
var container = document.createElement('div');
stub = ReactDOM.render(stub, container);
var node = ReactDOM.findDOMNode(stub);

expect(node.selectedIndex).toBe(1);

ReactDOM.render(
<select value="gorilla">{options}</select>,
container
);
expect(node.selectedIndex).toEqual(2);
});
});

0 comments on commit 904ee9a

Please sign in to comment.