Skip to content

Commit b4ebb52

Browse files
zombieJzombieJ
authored
feat: Support legacy props param with warning (#424)
* add warning * add warning if use `props`
1 parent d484a9f commit b4ebb52

File tree

5 files changed

+133
-41
lines changed

5 files changed

+133
-41
lines changed

‎src/utils/valueUtil.ts‎

Lines changed: 23 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,26 @@ export function flattenOptions(options: SelectOptionsType): FlattenOptionData[]
6767
return flattenList;
6868
}
6969

70+
/**
71+
* Inject `props` into `option` for legacy usage
72+
*/
73+
function injectPropsWithOption<T>(option: T): T {
74+
const newOption = { ...option };
75+
if (!('props' in newOption)) {
76+
Object.defineProperty(newOption, 'props', {
77+
get() {
78+
warning(
79+
false,
80+
'Return type is option instead of Option instance. Please read value directly instead of reading from `props`.',
81+
);
82+
return newOption;
83+
},
84+
});
85+
}
86+
87+
return newOption;
88+
}
89+
7090
export function findValueOption(
7191
values: RawValueType[],
7292
options: FlattenOptionData[],
@@ -81,7 +101,7 @@ export function findValueOption(
81101
}
82102
});
83103

84-
return values.map(val => optionMap.get(val));
104+
return values.map(val => injectPropsWithOption(optionMap.get(val)));
85105
}
86106

87107
export const getLabeledValue: GetLabeledValue<FlattenOptionData[]> = (
@@ -193,7 +213,7 @@ export function filterOptions(
193213
return;
194214
}
195215

196-
if (filterFunc(searchValue, item)) {
216+
if (filterFunc(searchValue, injectPropsWithOption(item))) {
197217
filteredOptions.push(item);
198218
}
199219
});
@@ -227,11 +247,7 @@ export function getSeparatedContent(text: string, tokens: string[]): string[] {
227247

228248
export function isValueDisabled(value: RawValueType, options: FlattenOptionData[]): boolean {
229249
const option = findValueOption([value], options)[0];
230-
if (option) {
231-
return option.disabled;
232-
}
233-
234-
return false;
250+
return option.disabled;
235251
}
236252

237253
/**

‎tests/Combobox.test.tsx‎

Lines changed: 16 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -6,12 +6,7 @@ import Select, { Option, SelectProps } from '../src';
66
import focusTest from './shared/focusTest';
77
import keyDownTest from './shared/keyDownTest';
88
import openControlledTest from './shared/openControlledTest';
9-
import {
10-
expectOpen,
11-
toggleOpen,
12-
selectItem,
13-
injectRunAllTimers,
14-
} from './utils/common';
9+
import { expectOpen, toggleOpen, selectItem, injectRunAllTimers } from './utils/common';
1510
import allowClearTest from './shared/allowClearTest';
1611
import throwOptionValue from './shared/throwOptionValue';
1712

@@ -71,10 +66,10 @@ describe('Select.Combobox', () => {
7166
);
7267

7368
wrapper.find('input').simulate('change', { target: { value: '1' } });
74-
expect(handleChange).toBeCalledWith('1', undefined);
69+
expect(handleChange).toHaveBeenCalledWith('1', {});
7570

7671
wrapper.find('input').simulate('change', { target: { value: '22' } });
77-
expect(handleChange).toBeCalledWith(
72+
expect(handleChange).toHaveBeenCalledWith(
7873
'22',
7974
expect.objectContaining({
8075
children: '22',
@@ -134,13 +129,15 @@ describe('Select.Combobox', () => {
134129
public state = {
135130
data: [],
136131
};
132+
137133
public handleChange = () => {
138134
setTimeout(() => {
139135
this.setState({
140136
data: [{ key: '1', label: '1' }, { key: '2', label: '2' }],
141137
});
142138
}, 500);
143139
};
140+
144141
public render() {
145142
const options = this.state.data.map(item => <Option key={item.key}>{item.label}</Option>);
146143
return (
@@ -171,13 +168,15 @@ describe('Select.Combobox', () => {
171168
public state = {
172169
data: [{ key: '1', label: '1' }, { key: '2', label: '2' }],
173170
};
171+
174172
public onSelect = () => {
175173
setTimeout(() => {
176174
this.setState({
177175
data: [{ key: '3', label: '3' }, { key: '4', label: '4' }],
178176
});
179177
}, 500);
180178
};
179+
181180
public render() {
182181
const options = this.state.data.map(item => <Option key={item.key}>{item.label}</Option>);
183182
return (
@@ -208,27 +207,21 @@ describe('Select.Combobox', () => {
208207
const handleChange = jest.fn();
209208
const handleSelect = jest.fn();
210209
const wrapper = mount(
211-
<Select
212-
mode="combobox"
213-
backfill={true}
214-
open={true}
215-
onChange={handleChange}
216-
onSelect={handleSelect}
217-
>
210+
<Select mode="combobox" backfill open onChange={handleChange} onSelect={handleSelect}>
218211
<Option value="One">One</Option>
219212
<Option value="Two">Two</Option>
220213
</Select>,
221214
);
222215
const input = wrapper.find('input');
223216
input.simulate('keyDown', { which: KeyCode.DOWN });
224217
expect(wrapper.find('input').props().value).toEqual('One');
225-
expect(handleChange).not.toBeCalled();
226-
expect(handleSelect).not.toBeCalled();
218+
expect(handleChange).not.toHaveBeenCalled();
219+
expect(handleSelect).not.toHaveBeenCalled();
227220

228221
input.simulate('keyDown', { which: KeyCode.ENTER });
229222
expect(wrapper.find('input').props().value).toEqual('One');
230-
expect(handleChange).toBeCalledWith('One', expect.objectContaining({ value: 'One' }));
231-
expect(handleSelect).toBeCalledWith('One', expect.objectContaining({ value: 'One' }));
223+
expect(handleChange).toHaveBeenCalledWith('One', expect.objectContaining({ value: 'One' }));
224+
expect(handleSelect).toHaveBeenCalledWith('One', expect.objectContaining({ value: 'One' }));
232225
});
233226

234227
it("should hide clear icon when value is ''", () => {
@@ -283,9 +276,9 @@ describe('Select.Combobox', () => {
283276
public render() {
284277
return (
285278
<Select mode="combobox" onChange={this.updateOptions}>
286-
{this.state.options.map(opt => {
287-
return <Option key={opt}>{opt}</Option>;
288-
})}
279+
{this.state.options.map(opt => (
280+
<Option key={opt}>{opt}</Option>
281+
))}
289282
</Select>
290283
);
291284
}
@@ -328,7 +321,7 @@ describe('Select.Combobox', () => {
328321
});
329322
jest.runAllTimers();
330323
wrapper.update();
331-
expect(onDropdownVisibleChange).toBeCalledWith(false);
324+
expect(onDropdownVisibleChange).toHaveBeenCalledWith(false);
332325
jest.useRealTimers();
333326
});
334327
});

‎tests/Select.test.tsx‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1217,4 +1217,87 @@ describe('Select.Basic', () => {
12171217
);
12181218
errorSpy.mockRestore();
12191219
});
1220+
1221+
describe('warning if use `props` to read data', () => {
1222+
it('filterOption', () => {
1223+
resetWarned();
1224+
const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
1225+
1226+
const wrapper = mount(
1227+
<Select
1228+
filterOption={(input, option) =>
1229+
option.props.children.toLowerCase().indexOf(input.toLowerCase()) >= 0
1230+
}
1231+
showSearch
1232+
>
1233+
<Option value="light">Light</Option>
1234+
<Option value="bamboo">Bamboo</Option>
1235+
</Select>,
1236+
);
1237+
1238+
wrapper.find('input').simulate('change', { target: { value: 'l' } });
1239+
expect(wrapper.find('List').props().data).toHaveLength(1);
1240+
expect(wrapper.find('div.rc-select-item-option-content').text()).toBe('Light');
1241+
1242+
expect(errorSpy).toHaveBeenCalledWith(
1243+
'Warning: Return type is option instead of Option instance. Please read value directly instead of reading from `props`.',
1244+
);
1245+
errorSpy.mockRestore();
1246+
});
1247+
1248+
it('Select & Deselect', () => {
1249+
resetWarned();
1250+
const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
1251+
1252+
const readPropsFunc = (_, opt) => {
1253+
expect(opt.props).toBeTruthy();
1254+
};
1255+
1256+
const wrapper = mount(
1257+
<Select mode="multiple" onSelect={readPropsFunc} onDeselect={readPropsFunc}>
1258+
<Option value="light">Light</Option>
1259+
<Option value="bamboo">Bamboo</Option>
1260+
</Select>,
1261+
);
1262+
1263+
toggleOpen(wrapper);
1264+
selectItem(wrapper);
1265+
expect(errorSpy).toHaveBeenCalledWith(
1266+
'Warning: Return type is option instead of Option instance. Please read value directly instead of reading from `props`.',
1267+
);
1268+
1269+
errorSpy.mockReset();
1270+
resetWarned();
1271+
selectItem(wrapper);
1272+
expect(errorSpy).toHaveBeenCalledWith(
1273+
'Warning: Return type is option instead of Option instance. Please read value directly instead of reading from `props`.',
1274+
);
1275+
1276+
errorSpy.mockRestore();
1277+
});
1278+
1279+
it('onChange', () => {
1280+
resetWarned();
1281+
const errorSpy = jest.spyOn(console, 'error').mockImplementation(() => {});
1282+
1283+
const readPropsFunc = (_, opt) => {
1284+
expect(opt.props).toBeTruthy();
1285+
};
1286+
1287+
const wrapper = mount(
1288+
<Select onChange={readPropsFunc}>
1289+
<Option value="light">Light</Option>
1290+
<Option value="bamboo">Bamboo</Option>
1291+
</Select>,
1292+
);
1293+
1294+
toggleOpen(wrapper);
1295+
selectItem(wrapper);
1296+
expect(errorSpy).toHaveBeenCalledWith(
1297+
'Warning: Return type is option instead of Option instance. Please read value directly instead of reading from `props`.',
1298+
);
1299+
1300+
errorSpy.mockRestore();
1301+
});
1302+
});
12201303
});

‎tests/Tags.test.tsx‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,7 @@ describe('Select.Tags', () => {
5858

5959
jest.runAllTimers();
6060
expect(findSelection(wrapper).text()).toBe('foo');
61-
expect(onChange).toHaveBeenCalledWith(['foo'], [undefined]);
61+
expect(onChange).toHaveBeenCalledWith(['foo'], [{}]);
6262
});
6363

6464
it('tokenize input', () => {
@@ -76,9 +76,9 @@ describe('Select.Tags', () => {
7676

7777
wrapper.find('input').simulate('change', { target: { value: '2,3,4' } });
7878

79-
expect(handleChange).toBeCalledWith(['2', '3', '4'], expect.anything());
79+
expect(handleChange).toHaveBeenCalledWith(['2', '3', '4'], expect.anything());
8080
expect(handleSelect).toHaveBeenCalledTimes(3);
81-
expect(handleSelect).toHaveBeenLastCalledWith('4', undefined);
81+
expect(handleSelect).toHaveBeenLastCalledWith('4', expect.anything());
8282
expect(findSelection(wrapper).text()).toEqual('2');
8383
expect(findSelection(wrapper, 1).text()).toEqual('3');
8484
expect(findSelection(wrapper, 2).text()).toEqual('4');
@@ -187,7 +187,7 @@ describe('Select.Tags', () => {
187187

188188
it('should work fine when filterOption function exists', () => {
189189
const children = [];
190-
for (let i = 10; i < 36; i++) {
190+
for (let i = 10; i < 36; i += 1) {
191191
children.push(
192192
<Option key={i.toString(36) + i} disabled={!(i % 3)}>
193193
{i.toString(36) + i}
@@ -199,7 +199,7 @@ describe('Select.Tags', () => {
199199
mode="tags"
200200
style={{ width: '100%' }}
201201
placeholder="Tags Mode"
202-
filterOption={(input, { key }) => key.indexOf(input) >= 0}
202+
filterOption={(input, { key }) => String(key).indexOf(input) >= 0}
203203
>
204204
{children}
205205
</Select>,

‎tests/shared/dynamicChildrenTest.tsx‎

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ export default function dynamicChildrenTest(mode: any, props?: Partial<SelectPro
2828
<Option key="2">2-label</Option>,
2929
],
3030
};
31+
3132
public select: any;
3233

3334
public componentDidMount() {
@@ -65,14 +66,11 @@ export default function dynamicChildrenTest(mode: any, props?: Partial<SelectPro
6566
wrapper.update();
6667
toggleOpen(wrapper);
6768
selectItem(wrapper, 1);
68-
expect(onChange).toBeCalledWith(
69+
expect(onChange).toHaveBeenCalledWith(
6970
['1', '3'],
70-
[
71-
mode === 'tags' ? expect.anything() : undefined,
72-
expect.objectContaining({ value: '3', children: '3-label' }),
73-
],
71+
[expect.anything(), expect.objectContaining({ value: '3', children: '3-label' })],
7472
);
75-
expect(onSelect).toBeCalledWith(
73+
expect(onSelect).toHaveBeenCalledWith(
7674
'3',
7775
expect.objectContaining({ value: '3', children: '3-label' }),
7876
);
@@ -91,6 +89,7 @@ export default function dynamicChildrenTest(mode: any, props?: Partial<SelectPro
9189
</Option>,
9290
],
9391
};
92+
9493
public select: any;
9594

9695
public componentDidMount() {
@@ -147,6 +146,7 @@ export default function dynamicChildrenTest(mode: any, props?: Partial<SelectPro
147146
<Option key="2">2-label</Option>,
148147
],
149148
};
149+
150150
public select: any;
151151

152152
public componentDidMount() {

0 commit comments

Comments
 (0)