From cc4c7a471eb481d11057ac78f421e86a525fd4f6 Mon Sep 17 00:00:00 2001 From: Wei Zhu Date: Wed, 27 Sep 2017 15:46:03 +0800 Subject: [PATCH] Allow any option value in single or multiple mode --- examples/single.js | 2 +- package.json | 1 + src/Option.jsx | 2 +- src/PropTypes.js | 6 +++--- src/Select.jsx | 9 +++++++-- src/util.js | 16 ++++++++++++++++ tests/Select.combobox.spec.js | 2 ++ tests/Select.multiple.spec.js | 16 ++++++++++++++++ tests/Select.spec.js | 20 ++++++++++++++++++++ tests/Select.tags.spec.js | 2 ++ tests/shared/throwOptionValue.js | 19 +++++++++++++++++++ 11 files changed, 88 insertions(+), 7 deletions(-) create mode 100644 tests/shared/throwOptionValue.js diff --git a/examples/single.js b/examples/single.js index f2452a964..ee04bb931 100644 --- a/examples/single.js +++ b/examples/single.js @@ -72,7 +72,7 @@ class Test extends React.Component { jack - + {[0, 1, 2, 3, 4, 5, 6, 7, 8, 9].map((i) => { diff --git a/package.json b/package.json index 5620f9d6f..ecd13daf2 100644 --- a/package.json +++ b/package.json @@ -69,6 +69,7 @@ "classnames": "2.x", "component-classes": "1.x", "dom-scroll-into-view": "1.x", + "lodash.isequal": "^4.5.0", "prop-types": "^15.5.8", "rc-animate": "2.x", "rc-menu": "^5.0.11", diff --git a/src/Option.jsx b/src/Option.jsx index 7b7151323..546a42628 100644 --- a/src/Option.jsx +++ b/src/Option.jsx @@ -3,7 +3,7 @@ import PropTypes from 'prop-types'; export default class Option extends React.Component { static propTypes = { - value: PropTypes.string, + value: PropTypes.any, }; static isSelectOption = true; diff --git a/src/PropTypes.js b/src/PropTypes.js index 2beb509e0..0801aa05a 100644 --- a/src/PropTypes.js +++ b/src/PropTypes.js @@ -2,7 +2,7 @@ import PropTypes from 'prop-types'; function valueType(props, propName, componentName) { const labelInValueShape = PropTypes.shape({ - key: PropTypes.string.isRequired, + key: PropTypes.any.isRequired, label: PropTypes.string, }); if (props.labelInValue) { @@ -25,8 +25,8 @@ function valueType(props, propName, componentName) { ); } else { const validate = PropTypes.oneOfType([ - PropTypes.arrayOf(PropTypes.string), - PropTypes.string, + PropTypes.arrayOf(PropTypes.any), + PropTypes.any, ]); return validate(...arguments); } diff --git a/src/Select.jsx b/src/Select.jsx index ce0a7191c..f7b363a35 100644 --- a/src/Select.jsx +++ b/src/Select.jsx @@ -22,11 +22,13 @@ import { splitBySeparators, findIndexInValueByLabel, defaultFilterFn, + validateOptionValue, } from './util'; import SelectTrigger from './SelectTrigger'; import { SelectPropTypes } from './PropTypes'; import { Item as MenuItem, ItemGroup as MenuItemGroup } from 'rc-menu'; import warning from 'warning'; +import isEqual from 'lodash.isequal'; function noop() {} @@ -424,7 +426,7 @@ export default class Select extends React.Component { if (maybe !== null) { label = maybe; } - } else if (getValuePropValue(child) === value) { + } else if (isEqual(getValuePropValue(child), value)) { label = this.getLabelFromOption(child); } }); @@ -895,6 +897,9 @@ export default class Select extends React.Component { ); const childValue = getValuePropValue(child); + + validateOptionValue(childValue, this.props); + if (this.filterOption(inputValue, child)) { sel.push( - {value[0].label} + {String(value[0].label)} ); } diff --git a/src/util.js b/src/util.js index f64ec4a5c..0bcbb386f 100644 --- a/src/util.js +++ b/src/util.js @@ -23,6 +23,10 @@ export function getPropValue(child, prop) { return child.props[prop]; } +export function isMultiple(props) { + return props.multiple; +} + export function isCombobox(props) { return props.combobox; } @@ -146,3 +150,15 @@ export function defaultFilterFn(input, child) { String(getPropValue(child, this.props.optionFilterProp)).indexOf(input) > -1 ); } + +export function validateOptionValue(value, props) { + if (isSingleMode(props) || isMultiple(props)) { + return; + } + if (typeof value !== 'string') { + throw new Error( + `Invalid \`value\` of type \`${typeof value}\` supplied to Option, ` + + `expected \`string\` when \`tags/combobox\` is \`true\`.` + ); + } +} diff --git a/tests/Select.combobox.spec.js b/tests/Select.combobox.spec.js index 9633f993a..f9f56ac26 100644 --- a/tests/Select.combobox.spec.js +++ b/tests/Select.combobox.spec.js @@ -4,11 +4,13 @@ import Select, { Option } from '../src'; import { mount, render } from 'enzyme'; import KeyCode from 'rc-util/lib/KeyCode'; import allowClearTest from './shared/allowClearTest'; +import throwOptionValue from './shared/throwOptionValue'; const delay = timeout => new Promise(resolve => setTimeout(resolve, timeout)); describe('Select.combobox', () => { allowClearTest('combobox'); + throwOptionValue('combobox'); it('renders correctly', () => { const wrapper = render( diff --git a/tests/Select.multiple.spec.js b/tests/Select.multiple.spec.js index 1641ab3a9..917781d65 100644 --- a/tests/Select.multiple.spec.js +++ b/tests/Select.multiple.spec.js @@ -85,4 +85,20 @@ describe('Select.multiple', () => { ); }).not.toThrow(); }); + + it('allow non-string value', () => { + const handleChange = jest.fn(); + + const wrapper = mount( + + ); + + wrapper.find('.rc-select').simulate('click'); + wrapper.find('MenuItem').at(1).simulate('click'); + + expect(handleChange).toBeCalledWith([1, 2]); + }); }); diff --git a/tests/Select.spec.js b/tests/Select.spec.js index 898a7a915..cd537fbb9 100644 --- a/tests/Select.spec.js +++ b/tests/Select.spec.js @@ -707,4 +707,24 @@ describe('Select', () => { expect(handleChange).toBeCalledWith('2'); expect(handleSelect).toBeCalledWith('2', expect.anything()); }); + + it('allow non-string value', () => { + const handleChange = jest.fn(); + + const wrapper = mount( + + ); + + wrapper.find('.rc-select').simulate('click'); + wrapper.find('MenuItem').at(1).simulate('click'); + expect(handleChange).toBeCalledWith(true); + + wrapper.find('.rc-select').simulate('click'); + wrapper.find('MenuItem').at(2).simulate('click'); + expect(handleChange).toBeCalledWith({ value: 3 }); + }); }); diff --git a/tests/Select.tags.spec.js b/tests/Select.tags.spec.js index 20cc8c79b..b1bb197ca 100644 --- a/tests/Select.tags.spec.js +++ b/tests/Select.tags.spec.js @@ -7,6 +7,7 @@ import allowClearTest from './shared/allowClearTest'; import blurTest from './shared/blurTest'; import renderTest from './shared/renderTest'; import removeSelectedTest from './shared/removeSelectedTest'; +import throwOptionValue from './shared/throwOptionValue'; jest.unmock('react-dom'); @@ -15,6 +16,7 @@ describe('Select.tags', () => { blurTest('tags'); renderTest('tags'); removeSelectedTest('tags'); + throwOptionValue('tags'); it('allow user input tags', () => { const wrapper = mount( diff --git a/tests/shared/throwOptionValue.js b/tests/shared/throwOptionValue.js new file mode 100644 index 000000000..0d0728b84 --- /dev/null +++ b/tests/shared/throwOptionValue.js @@ -0,0 +1,19 @@ +import React from 'react'; +import Select from '../../src/Select'; +import Option from '../../src/Option'; +import { mount } from 'enzyme'; + +export default function throwOptionValue(mode) { + it('warn option value type', () => { + const render = () => mount( + + ); + + expect(render).toThrow( + 'Invalid `value` of type `number` supplied to Option, ' + + 'expected `string` when `tags/combobox` is `true`.' + ); + }); +}