Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

antd v5: Controlled components will treat null as empty value and null can not be used as an option value. #777

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
3 changes: 3 additions & 0 deletions docs/demo/null-controlled.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
## null-controlled

<code src="../examples/null-controlled.tsx">
64 changes: 64 additions & 0 deletions docs/examples/null-controlled.tsx
Original file line number Diff line number Diff line change
@@ -0,0 +1,64 @@
/* eslint-disable no-console */
import Select, { Option } from 'rc-select';
import React from 'react';
import '../../assets/index.less';

interface NullControlledState {
destroy: boolean;
value: string | number;
}

class Controlled extends React.Component<unknown, NullControlledState> {
state = {
destroy: false,
value: null,
};

onChange = (e) => {
let value;
if (e && e.target) {
({ value } = e.target);
} else {
value = e;
}
console.log('onChange', value);
this.setState({
value,
});
};

onDestroy = () => {
this.setState({
destroy: true,
});
};

render() {
const { destroy, value } = this.state;
if (destroy) {
return null;
}
return (
<div style={{ margin: 20 }}>
<h2>controlled Select: value === null</h2>
<div style={{ width: 300 }}>
<Select
id="my-select"
value={value}
placeholder="placeholder"
listHeight={200}
style={{ width: 500 }}
onChange={this.onChange}
allowClear
>
<Option value={null}>null</Option>
<Option value="correct">correct</Option>
</Select>
</div>
</div>
);
}
}

export default Controlled;
/* eslint-enable */
1 change: 1 addition & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,7 @@
"father": "^2.13.2",
"jsonp": "^0.2.1",
"np": "^7.5.0",
"prettier": "^2.6.2",
"rc-dialog": "^8.1.1",
"typescript": "^4.2.3"
}
Expand Down
126 changes: 63 additions & 63 deletions src/Select.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -29,24 +29,29 @@
* - `combobox` mode not support `optionLabelProp`
*/

import * as React from 'react';
import warning from 'rc-util/lib/warning';
import useMergedState from 'rc-util/lib/hooks/useMergedState';
import warning from 'rc-util/lib/warning';
import * as React from 'react';
import type {
BaseSelectProps,
BaseSelectPropsWithoutPrivate,
BaseSelectRef,
DisplayValueType,
RenderNode,
} from './BaseSelect';
import BaseSelect, { isMultiple } from './BaseSelect';
import type { DisplayValueType, RenderNode } from './BaseSelect';
import OptionList from './OptionList';
import Option from './Option';
import OptGroup from './OptGroup';
import type { BaseSelectRef, BaseSelectPropsWithoutPrivate, BaseSelectProps } from './BaseSelect';
import useOptions from './hooks/useOptions';
import SelectContext from './SelectContext';
import useCache from './hooks/useCache';
import useFilterOptions from './hooks/useFilterOptions';
import useId from './hooks/useId';
import useOptions from './hooks/useOptions';
import useRefFunc from './hooks/useRefFunc';
import OptGroup from './OptGroup';
import Option from './Option';
import OptionList from './OptionList';
import SelectContext from './SelectContext';
import { toArray } from './utils/commonUtil';
import { fillFieldNames, flattenOptions, injectPropsWithOption } from './utils/valueUtil';
import warningProps from './utils/warningPropsUtil';
import { toArray } from './utils/commonUtil';
import useFilterOptions from './hooks/useFilterOptions';
import useCache from './hooks/useCache';

const OMIT_DOM_PROPS = ['inputValue'];

Expand Down Expand Up @@ -237,45 +242,48 @@ const Select = React.forwardRef(
const valueList = toArray(draftValues);

// Convert to labelInValue type
return valueList.map((val) => {
let rawValue: RawValueType;
let rawLabel: React.ReactNode;
let rawKey: React.Key;
let rawDisabled: boolean | undefined;

// Fill label & value
if (isRawValue(val)) {
rawValue = val;
} else {
rawKey = val.key;
rawLabel = val.label;
rawValue = val.value ?? rawKey;
}
// We .filter() it with null value first
return valueList
.filter((val) => val !== null)
.map((val) => {
let rawValue: RawValueType;
let rawLabel: React.ReactNode;
let rawKey: React.Key;
let rawDisabled: boolean | undefined;

// Fill label & value
if (isRawValue(val)) {
rawValue = val;
} else {
rawKey = val.key;
rawLabel = val.label;
rawValue = val.value ?? rawKey;
}

const option = valueOptions.get(rawValue);
if (option) {
// Fill missing props
if (rawLabel === undefined)
rawLabel = option?.[optionLabelProp || mergedFieldNames.label];
if (rawKey === undefined) rawKey = option?.key ?? rawValue;
rawDisabled = option?.disabled;

// Warning if label not same as provided
if (process.env.NODE_ENV !== 'production' && !optionLabelProp) {
const optionLabel = option?.[mergedFieldNames.label];
if (optionLabel !== undefined && optionLabel !== rawLabel) {
warning(false, '`label` of `value` is not same as `label` in Select options.');
const option = valueOptions.get(rawValue);
if (option) {
// Fill missing props
if (rawLabel === undefined)
rawLabel = option?.[optionLabelProp || mergedFieldNames.label];
if (rawKey === undefined) rawKey = option?.key ?? rawValue;
rawDisabled = option?.disabled;

// Warning if label not same as provided
if (process.env.NODE_ENV !== 'production' && !optionLabelProp) {
const optionLabel = option?.[mergedFieldNames.label];
if (optionLabel !== undefined && optionLabel !== rawLabel) {
warning(false, '`label` of `value` is not same as `label` in Select options.');
}
}
}
}

return {
label: rawLabel,
value: rawValue,
key: rawKey,
disabled: rawDisabled,
};
});
return {
label: rawLabel,
value: rawValue,
key: rawKey,
disabled: rawDisabled,
};
});
},
[mergedFieldNames, optionLabelProp, valueOptions],
);
Expand All @@ -294,30 +302,20 @@ const Select = React.forwardRef(
return [];
}

// if (values)

return values;
}, [internalValue, convert2LabelValues, mode]);

// Fill label with cache to avoid option remove
const [mergedValues, getMixedOption] = useCache(rawLabeledValues, valueOptions);

const displayValues = React.useMemo(() => {
// `null` need show as placeholder instead
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since when null as value will returns [], this logic code is no looger needed. When value={null}, the placeholder will show up correctly.
因为 null 作为空值已经做了返回 [] 的处理,所以这段特殊处理此 issue 的逻辑已经不需要了,value={null} 时 placeholder 会正确显示。

// https://github.com/ant-design/ant-design/issues/25057
if (!mode && mergedValues.length === 1) {
const firstValue = mergedValues[0];
if (
firstValue.value === null &&
(firstValue.label === null || firstValue.label === undefined)
) {
return [];
}
}

return mergedValues.map((item) => ({
...item,
label: item.label ?? item.value,
}));
}, [mode, mergedValues]);
}, [mergedValues]);

/** Convert `displayValues` to raw value type set */
const rawValues = React.useMemo(
Expand All @@ -333,7 +331,7 @@ const Select = React.forwardRef(
setSearchValue(String(strValue));
}
}
}, [mergedValues]);
}, [mergedValues, mode, setSearchValue]);

// ======================= Display Option =======================
// Create a placeholder item if not exist in `options`
Expand Down Expand Up @@ -423,11 +421,13 @@ const Select = React.forwardRef(
injectPropsWithOption(getMixedOption(v.value)),
);

// Do not pass undefined to onChange function.
// Because it will cause the controlled components to be uncontrolled
onChange(
// Value
multiple ? returnValues : returnValues[0],
multiple ? returnValues : returnValues[0] ?? null,
// Option
multiple ? returnOptions : returnOptions[0],
multiple ? returnOptions : returnOptions[0] ?? null,
);
}
};
Expand Down
39 changes: 32 additions & 7 deletions src/utils/warningPropsUtil.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import * as React from 'react';
import warning, { noteOnce } from 'rc-util/lib/warning';
import toNodeArray from 'rc-util/lib/Children/toArray';
import { convertChildrenToData } from './legacyUtil';
import { toArray } from './commonUtil';
import type { RawValueType, LabelInValueType, BaseOptionType, SelectProps } from '../Select';
import warning, { noteOnce } from 'rc-util/lib/warning';
import * as React from 'react';
import { isMultiple } from '../BaseSelect';
import type { BaseOptionType, LabelInValueType, RawValueType, SelectProps } from '../Select';
import { toArray } from './commonUtil';
import { convertChildrenToData } from './legacyUtil';

function warningProps(props: SelectProps) {
const {
Expand All @@ -29,7 +29,6 @@ function warningProps(props: SelectProps) {
const mergedShowSearch = showSearch !== undefined ? showSearch : multiple || mode === 'combobox';
const mergedOptions = options || convertChildrenToData(children);

// `tags` should not set option as disabled
warning(
mode !== 'tags' || mergedOptions.every((opt: { disabled?: boolean }) => !opt.disabled),
'Please avoid setting option to disabled in tags mode since user can always type text as tag.',
Expand Down Expand Up @@ -97,10 +96,19 @@ function warningProps(props: SelectProps) {
);
}

// value in select option can not be null
const nullValueWarning = () =>
warning(
false,
'`value` in select option can not be null, please use `string | number` instead.',
);

// Syntactic sugar should use correct children type
if (children) {
let invalidateChildType = null;
toNodeArray(children).some((node: React.ReactNode) => {
const childrenArray = toNodeArray(children);

childrenArray.some((node: React.ReactNode) => {
if (!React.isValidElement(node) || !node.type) {
return false;
}
Expand Down Expand Up @@ -147,6 +155,23 @@ function warningProps(props: SelectProps) {
inputValue === undefined,
'`inputValue` is deprecated, please use `searchValue` instead.',
);

for (let i = 0; i < childrenArray.length; i++) {
if (childrenArray[i]?.props?.value === null) {
nullValueWarning();
break;
}
}
}

// value in option can not be null
if (options) {
for (let i = 0; i < options.length; i++) {
if (options[i]?.value === null) {
nullValueWarning();
break;
}
}
}
}

Expand Down