Skip to content

Commit

Permalink
Deduplication of warn when selected is set on <option> (#11821)
Browse files Browse the repository at this point in the history
* Deduplication of warn selected on option

- Wrote a failing test
- Deduplication when selected is set on option

* Ran yarn preitter

* Fixed PR request

- Moved dedupe test to above
- Moved && case to seperate if to seperate static and dynamic things
- Render'd component twice

* Actually check for deduplication

* Minor nits
  • Loading branch information
watadarkstar authored and gaearon committed Dec 10, 2017
1 parent f23dd71 commit 51e3f49
Show file tree
Hide file tree
Showing 2 changed files with 39 additions and 5 deletions.
29 changes: 29 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMSelect-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -540,6 +540,7 @@ describe('ReactDOMSelect', () => {
</select>,
);
if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'`value` prop on `select` should not be null. ' +
'Consider using an empty string to clear the component or `undefined` ' +
Expand All @@ -557,6 +558,34 @@ describe('ReactDOMSelect', () => {
}
});

it('should warn if selected is set on <option>', () => {
spyOnDev(console, 'error');

ReactTestUtils.renderIntoDocument(
<select>
<option selected={true} />
<option selected={true} />
</select>,
);
if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
}

ReactTestUtils.renderIntoDocument(
<select>
<option selected={true} />
<option selected={true} />
</select>,
);
if (__DEV__) {
expect(console.error.calls.count()).toBe(1);
expect(console.error.calls.argsFor(0)[0]).toContain(
'Use the `defaultValue` or `value` props on <select> instead of ' +
'setting `selected` on <option>.',
);
}
});

it('should warn if value is null and multiple is true', () => {
spyOnDev(console, 'error');
ReactTestUtils.renderIntoDocument(
Expand Down
15 changes: 10 additions & 5 deletions packages/react-dom/src/client/ReactDOMFiberOption.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,8 @@
import React from 'react';
import warning from 'fbjs/lib/warning';

let didWarnSelectedSetOnOption = false;

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

Expand All @@ -36,11 +38,14 @@ function flattenChildren(children) {
export function validateProps(element: Element, props: Object) {
// TODO (yungsters): Remove support for `selected` in <option>.
if (__DEV__) {
warning(
props.selected == null,
'Use the `defaultValue` or `value` props on <select> instead of ' +
'setting `selected` on <option>.',
);
if (!didWarnSelectedSetOnOption) {
warning(
props.selected == null,
'Use the `defaultValue` or `value` props on <select> instead of ' +
'setting `selected` on <option>.',
);
didWarnSelectedSetOnOption = true;
}
}
}

Expand Down

0 comments on commit 51e3f49

Please sign in to comment.