Skip to content

Commit 0e94d34

Browse files
Hyperkid123Hyperkid123
authored
Merge pull request #1410 from victoria-dos/load-options-bug
Add compareValues prop to useSelect's reducer
2 parents d0bf771 + ef9d48f commit 0e94d34

File tree

12 files changed

+325
-40
lines changed

12 files changed

+325
-40
lines changed

‎packages/common/src/select/deep-equal.d.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,3 @@
1+
declare const deepEqual: (valueA: any, valueB: any) => boolean;
2+
3+
export default deepEqual;

‎packages/common/src/select/deep-equal.js‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
const deepEqual = (a, b, depth = 0) => {
2+
if (depth > 4) {
3+
console.error('Recursion limit of 5 has been exceeded.');
4+
return false;
5+
}
6+
7+
if (a === b) {
8+
return true;
9+
}
10+
11+
if (Array.isArray(a) && Array.isArray(b) && a.length === b.length) {
12+
return a.every((item, index) => deepEqual(item, b[index]));
13+
}
14+
15+
if (typeof a === 'object' && typeof b === 'object' && a !== null && b !== null) {
16+
const keysA = Object.keys(a);
17+
const keysB = Object.keys(b);
18+
19+
if (keysA.length !== keysB.length) {
20+
return false;
21+
}
22+
23+
for (const key of keysA) {
24+
if (!keysB.includes(key) || !deepEqual(a[key], b[key], depth + 1)) {
25+
return false;
26+
}
27+
}
28+
29+
return true;
30+
}
31+
32+
return false;
33+
};
34+
35+
export default deepEqual;

‎packages/common/src/select/select.d.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -29,6 +29,7 @@ export interface SelectProps {
2929
SelectComponent?: React.ComponentType;
3030
noValueUpdates?: boolean;
3131
optionsTransformer?: (options: AnyObject[]) => option[];
32+
compareValues?: (valueA: any, valueB: any) => boolean;
3233
}
3334

3435
declare const Select: React.ComponentType<SelectProps>;

‎packages/common/src/select/select.js‎

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import React, { useMemo } from 'react';
44
import PropTypes from 'prop-types';
55
import clsx from 'clsx';
66
import useSelect from '../use-select/use-select';
7+
import deepEqual from './deep-equal';
78

89
const Select = ({
910
invalid,
@@ -24,6 +25,7 @@ const Select = ({
2425
SelectComponent,
2526
noValueUpdates,
2627
optionsTransformer,
28+
compareValues = deepEqual,
2729
...props
2830
}) => {
2931
const {
@@ -44,6 +46,7 @@ const Select = ({
4446
pluckSingleValue,
4547
isMulti,
4648
simpleValue,
49+
compareValues,
4750
});
4851

4952
const renderNoOptionsMessage = () => (Object.values(state.promises).some((value) => value) ? () => updatingMessage : () => noOptionsMessage);
@@ -119,6 +122,7 @@ Select.propTypes = {
119122
SelectComponent: PropTypes.elementType.isRequired,
120123
noValueUpdates: PropTypes.bool,
121124
optionsTransformer: PropTypes.func,
125+
compareValues: PropTypes.func,
122126
};
123127

124128
Select.defaultProps = {
@@ -129,6 +133,7 @@ Select.defaultProps = {
129133
placeholder: 'Choose...',
130134
isSearchable: false,
131135
isClearable: false,
136+
compareValues: deepEqual,
132137
};
133138

134139
export default Select;

‎packages/common/src/tests/select/deep-equal.test.js‎

Lines changed: 78 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,78 @@
1+
import cloneDeep from 'lodash/cloneDeep';
2+
import deepEqual from '../../select/deep-equal';
3+
4+
describe('deepEqual', () => {
5+
describe('primitives', () => {
6+
const tuplesBase = [1, 'a', false, undefined, null];
7+
const positiveCases = tuplesBase.map((value) => ({
8+
input: [value, value],
9+
output: true,
10+
}));
11+
const negativeCases = tuplesBase.map((value) => ({
12+
input: [value, value + 1],
13+
output: false,
14+
}));
15+
positiveCases.forEach(({ input, output }) => {
16+
it(`should return ${output} for a ${input} input`, () => {
17+
expect(deepEqual(...input)).toEqual(output);
18+
});
19+
});
20+
21+
negativeCases.forEach(({ input, output }) => {
22+
it(`should return ${output} for a ${input} input`, () => {
23+
expect(deepEqual(...input)).toEqual(output);
24+
});
25+
});
26+
});
27+
28+
describe('objects', () => {
29+
it('should return true for equal object structure', () => {
30+
const valueA = {
31+
value: {
32+
foo: 'bar',
33+
baz: [1, 2, { quaz: { 2: 3 } }],
34+
},
35+
};
36+
37+
expect(deepEqual(valueA, cloneDeep(valueA))).toEqual(true);
38+
});
39+
40+
it('should return false for non equal object structure', () => {
41+
const valueA = {
42+
value: {
43+
foo: 'bar',
44+
baz: [1, 2, { quaz: { 2: 3 } }],
45+
},
46+
};
47+
48+
const valueB = {
49+
...valueA,
50+
value: {
51+
...valueA.value,
52+
baz: [1, 2, { quaz: {} }],
53+
},
54+
};
55+
56+
expect(deepEqual(valueA, valueB)).toEqual(false);
57+
});
58+
59+
it('should log an error if an object is too deeply nested', () => {
60+
const errorSpy = jest.spyOn(console, 'error');
61+
const valueA = {
62+
value: {
63+
value: {
64+
value: {
65+
value: {
66+
value: {},
67+
},
68+
},
69+
},
70+
},
71+
};
72+
73+
expect(deepEqual(valueA, cloneDeep(valueA))).toEqual(false);
74+
expect(errorSpy).toHaveBeenCalledWith('Recursion limit of 5 has been exceeded.');
75+
errorSpy.mockReset();
76+
});
77+
});
78+
});

‎packages/common/src/tests/select/select.test.js‎

Lines changed: 141 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
import React from 'react';
44
import { render, screen, act, fireEvent } from '@testing-library/react';
55
import userEvent from '@testing-library/user-event';
6+
import cloneDeep from 'lodash/cloneDeep';
67

78
import { useFieldApi, FormRenderer, componentTypes } from '@data-driven-forms/react-form-renderer';
89

@@ -501,7 +502,146 @@ describe('Select test', () => {
501502
jest.useRealTimers();
502503
});
503504

504-
it('initial values is not in options', async () => {
505+
it('initial value is object', async () => {
506+
const loadOptions = jest.fn().mockImplementation(() =>
507+
Promise.resolve([
508+
{ value: { value: '111' }, label: 'first' },
509+
{ value: { value: '1111' }, label: 'second' },
510+
])
511+
);
512+
field = { ...field, loadOptions, options: [] };
513+
514+
await act(async () => {
515+
render(
516+
<FormRenderer
517+
{...rendererProps}
518+
schema={{
519+
fields: [
520+
{
521+
...field,
522+
isMulti: true,
523+
isSearchable: true,
524+
isClearable: true,
525+
initialValue: [{ value: { value: '111' }, label: 'first' }],
526+
component: componentTypes.SELECT,
527+
name: 'select',
528+
},
529+
],
530+
}}
531+
/>
532+
);
533+
});
534+
535+
expect(inputValue).toEqual([{ value: { value: '111' }, label: 'first' }]);
536+
});
537+
538+
describe('options compareValues', () => {
539+
const loadOptions = jest.fn().mockImplementation(() =>
540+
Promise.resolve([
541+
{ value: { value: '111' }, label: 'first' },
542+
{ value: { value: '1111' }, label: 'second' },
543+
{ value: { value: '3' }, label: 'third' },
544+
])
545+
);
546+
547+
const customCompareField = {
548+
...field,
549+
component: componentTypes.SELECT,
550+
name: 'select',
551+
loadOptions,
552+
options: [],
553+
isMulti: true,
554+
isSearchable: true,
555+
isClearable: true,
556+
};
557+
558+
it('custom function compares options with object value', async () => {
559+
await act(async () => {
560+
render(
561+
<FormRenderer
562+
{...rendererProps}
563+
schema={{
564+
fields: [
565+
{
566+
...customCompareField,
567+
compareValues: (a, b) => {
568+
return a.value.value === b.value.value;
569+
},
570+
initialValue: [{ value: { value: '111' }, label: 'first' }],
571+
},
572+
],
573+
}}
574+
/>
575+
);
576+
});
577+
578+
expect(inputValue).toEqual([{ value: { value: '111' }, label: 'first' }]);
579+
});
580+
581+
it('default function compares options with object value', async () => {
582+
await act(async () => {
583+
render(
584+
<FormRenderer
585+
{...rendererProps}
586+
schema={{
587+
fields: [
588+
{
589+
...customCompareField,
590+
initialValue: [{ value: { value: '111' }, label: 'first' }],
591+
},
592+
],
593+
}}
594+
/>
595+
);
596+
});
597+
598+
expect(inputValue).toEqual([{ value: { value: '111' }, label: 'first' }]);
599+
});
600+
601+
it('default function logs an error for highly nested values ', async () => {
602+
const consoleSpy = jest.spyOn(console, 'error').mockImplementationOnce(() => undefined);
603+
const nestedOptions = {
604+
label: 'Nested',
605+
value: {
606+
value: {
607+
value: {
608+
value: {
609+
value: {
610+
value: 'foo',
611+
},
612+
},
613+
},
614+
},
615+
},
616+
};
617+
const loadOptions = jest.fn().mockImplementation(() => Promise.resolve([{ ...nestedOptions }]));
618+
await act(async () => {
619+
render(
620+
<FormRenderer
621+
{...rendererProps}
622+
schema={{
623+
fields: [
624+
{
625+
...customCompareField,
626+
loadOptions,
627+
initialValue: [cloneDeep(nestedOptions)],
628+
},
629+
],
630+
}}
631+
/>
632+
);
633+
});
634+
635+
// Console error should be triggered for large object depth
636+
expect(consoleSpy).toHaveBeenCalledWith('Recursion limit of 5 has been exceeded.');
637+
// Should have no value because he options are not matched if the depth limit was exceeded
638+
expect(inputValue).toEqual('');
639+
640+
consoleSpy.mockReset();
641+
});
642+
});
643+
644+
it('initial values are not in options', async () => {
505645
const loadOptions = jest.fn().mockImplementation(() =>
506646
Promise.resolve([
507647
{ value: '111', label: 'first' },

‎packages/common/src/use-select/reducer.js‎

Lines changed: 9 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,10 +40,16 @@ const reducer = (state, { type, payload, options = [], optionsTransformer }) =>
4040
...payload,
4141
},
4242
options: optionsTransformer
43-
? optionsTransformer([...state.options, ...options.filter(({ value }) => !state.options.find((option) => option.value === value))])
44-
: [...state.options, ...options.filter(({ value }) => !state.options.find((option) => option.value === value))],
43+
? optionsTransformer([
44+
...state.options,
45+
...options.filter(({ value }) => !state.options.find((option) => payload.compareValues(option.value, value))),
46+
])
47+
: [...state.options, ...options.filter(({ value }) => !state.options.find((option) => payload.compareValues(option.value, value)))],
4548
...(optionsTransformer && {
46-
originalOptions: [...state.options, ...options.filter(({ value }) => !state.options.find((option) => option.value === value))],
49+
originalOptions: [
50+
...state.options,
51+
...options.filter(({ value }) => !state.options.find((option) => payload.compareValues(option.value, value))),
52+
],
4753
}),
4854
};
4955
default:

‎packages/common/src/use-select/use-select.d.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@ interface UseSelectProps {
1313
pluckSingleValue?: boolean;
1414
isMulti?: boolean;
1515
simpleValue?: boolean;
16+
compareValues?: (value1: any, value2: any) => any;
1617
}
1718

1819
interface SelectState {

‎packages/common/src/use-select/use-select.js‎

Lines changed: 7 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -67,6 +67,7 @@ const useSelect = ({
6767
pluckSingleValue,
6868
isMulti,
6969
simpleValue,
70+
compareValues,
7071
}) => {
7172
const [state, originalDispatch] = useReducer(reducer, { optionsTransformer, propsOptions }, init);
7273
const dispatch = (action) => originalDispatch({ ...action, optionsTransformer });
@@ -81,10 +82,12 @@ const useSelect = ({
8182
if (!noValueUpdates) {
8283
if (value && Array.isArray(value)) {
8384
const selectValue = value.filter((value) =>
84-
typeof value === 'object' ? data.find((option) => value.value === option.value) : data.find((option) => value === option.value)
85+
typeof value === 'object'
86+
? data.find((option) => compareValues(value.value, option.value))
87+
: data.find((option) => compareValues(value, option.value))
8588
);
8689
onChange(selectValue.length === 0 ? undefined : selectValue);
87-
} else if (value && !data.find(({ value: internalValue }) => internalValue === value)) {
90+
} else if (value && !data.find(({ value: internalValue }) => compareValues(internalValue, value))) {
8891
onChange(undefined);
8992
}
9093
}
@@ -122,14 +125,14 @@ const useSelect = ({
122125

123126
const onInputChange = (inputValue) => {
124127
if (inputValue && loadOptions && state.promises[inputValue] === undefined && isSearchable) {
125-
dispatch({ type: 'setPromises', payload: { [inputValue]: true } });
128+
dispatch({ type: 'setPromises', payload: { [inputValue]: true, compareValues } });
126129

127130
loadOptions(inputValue)
128131
.then((options) => {
129132
if (isMounted.current) {
130133
dispatch({
131134
type: 'setPromises',
132-
payload: { [inputValue]: false },
135+
payload: { [inputValue]: false, compareValues },
133136
options,
134137
});
135138
}

0 commit comments

Comments
 (0)