Skip to content

Commit

Permalink
fix(hooks): check state chage via deep compare (#1558)
Browse files Browse the repository at this point in the history
* fix(hooks): check state chage via deep compare

* fix function

* update prev state from props

* remove the props update

* fix the test
  • Loading branch information
silviuaavram authored Dec 28, 2023
1 parent a8f2699 commit 4edfc30
Show file tree
Hide file tree
Showing 8 changed files with 95 additions and 7 deletions.
2 changes: 1 addition & 1 deletion docusaurus/pages/useMultipleCombobox.js
Original file line number Diff line number Diff line change
Expand Up @@ -125,7 +125,7 @@ export default function DropdownMultipleCombobox() {
style={{padding: '4px', cursor: 'pointer'}}
onClick={e => {
e.stopPropagation()
removeSelectedItem(null)
removeSelectedItem(selectedItemForRender)
}}
>
✕
Expand Down
34 changes: 34 additions & 0 deletions src/hooks/__tests__/utils.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import {
getDefaultValue,
useMouseAndTouchTracker,
getItemAndIndex,
isDropdownsStateEqual,
} from '../utils'

describe('utils', () => {
Expand Down Expand Up @@ -152,4 +153,37 @@ describe('utils', () => {
})
})
})

describe('isDropdownsStateEqual', () => {
test('is true when each property is equal', () => {
const selectedItem = 'hello'
const prevState = {
highlightedIndex: 2,
isOpen: true,
selectedItem,
inputValue: selectedItem,
}
const newState = {
...prevState,
}

expect(isDropdownsStateEqual(prevState, newState)).toBe(true)
})

test('is false when at least one property is not equal', () => {
const selectedItem = {value: 'hello'}
const prevState = {
highlightedIndex: 2,
isOpen: true,
selectedItem,
inputValue: selectedItem,
}
const newState = {
...prevState,
selectedItem: {...selectedItem},
}

expect(isDropdownsStateEqual(prevState, newState)).toBe(false)
})
})
})
2 changes: 2 additions & 0 deletions src/hooks/useCombobox/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import {
useElementIds,
getItemAndIndex,
getInitialValue,
isDropdownsStateEqual
} from '../utils'
import {
getInitialState,
Expand Down Expand Up @@ -43,6 +44,7 @@ function useCombobox(userProps = {}) {
downshiftUseComboboxReducer,
props,
getInitialState,
isDropdownsStateEqual
)
const {isOpen, highlightedIndex, selectedItem, inputValue} = state

Expand Down
4 changes: 3 additions & 1 deletion src/hooks/useCombobox/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -58,14 +58,16 @@ const propTypes = {
* @param {Function} reducer Reducer function from downshift.
* @param {Object} props The hook props, also passed to createInitialState.
* @param {Function} createInitialState Function that returns the initial state.
* @param {Function} isStateEqual Function that checks if a previous state is equal to the next.
* @returns {Array} An array with the state and an action dispatcher.
*/
export function useControlledReducer(reducer, props, createInitialState) {
export function useControlledReducer(reducer, props, createInitialState, isStateEqual) {
const previousSelectedItemRef = useRef()
const [state, dispatch] = useEnhancedReducer(
reducer,
props,
createInitialState,
isStateEqual
)

// ToDo: if needed, make same approach as selectedItemChanged from Downshift.
Expand Down
2 changes: 2 additions & 0 deletions src/hooks/useMultipleSelection/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
defaultProps,
isKeyDownOperationPermitted,
validatePropTypes,
isStateEqual
} from './utils'
import downshiftMultipleSelectionReducer from './reducer'
import * as stateChangeTypes from './stateChangeTypes'
Expand All @@ -40,6 +41,7 @@ function useMultipleSelection(userProps = {}) {
downshiftMultipleSelectionReducer,
props,
getInitialState,
isStateEqual
)
const {activeIndex, selectedItems} = state

Expand Down
16 changes: 16 additions & 0 deletions src/hooks/useMultipleSelection/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,21 @@ function getA11yRemovalMessage(selectionParameters) {
return `${itemToStringLocal(removedSelectedItem)} has been removed.`
}

/**
* Check if a state is equal for taglist, by comparing active index and selected items.
* Used by useSelect and useCombobox.
*
* @param {Object} prevState
* @param {Object} newState
* @returns {boolean} Wheather the states are deeply equal.
*/
function isStateEqual(prevState, newState) {
return (
prevState.selectedItems === newState.selectedItems &&
prevState.activeIndex === newState.activeIndex
)
}

const propTypes = {
...commonPropTypes,
selectedItems: PropTypes.array,
Expand Down Expand Up @@ -133,4 +148,5 @@ export {
getDefaultValue,
getInitialState,
isKeyDownOperationPermitted,
isStateEqual,
}
3 changes: 2 additions & 1 deletion src/hooks/useSelect/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import {
useMouseAndTouchTracker,
getItemAndIndex,
getInitialValue,
isDropdownsStateEqual,
} from '../utils'
import {
callAllEventHandlers,
Expand Down Expand Up @@ -46,9 +47,9 @@ function useSelect(userProps = {}) {
downshiftSelectReducer,
props,
getInitialState,
isDropdownsStateEqual,
)
const {isOpen, highlightedIndex, selectedItem, inputValue} = state

// Element efs.
const toggleButtonRef = useRef(null)
const menuRef = useRef(null)
Expand Down
39 changes: 35 additions & 4 deletions src/hooks/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -189,9 +189,10 @@ function useLatestRef(val) {
* @param {Function} reducer Reducer function from downshift.
* @param {Object} props The hook props, also passed to createInitialState.
* @param {Function} createInitialState Function that returns the initial state.
* @param {Function} isStateEqual Function that checks if a previous state is equal to the next.
* @returns {Array} An array with the state and an action dispatcher.
*/
function useEnhancedReducer(reducer, props, createInitialState) {
function useEnhancedReducer(reducer, props, createInitialState, isStateEqual) {
const prevStateRef = useRef()
const actionRef = useRef()
const enhancedReducer = useCallback(
Expand Down Expand Up @@ -219,7 +220,12 @@ function useEnhancedReducer(reducer, props, createInitialState) {
const action = actionRef.current

useEffect(() => {
if (action && prevStateRef.current && prevStateRef.current !== state) {
const shouldCallOnChangeProps =
action &&
prevStateRef.current &&
!isStateEqual(prevStateRef.current, state)

if (shouldCallOnChangeProps) {
callOnChangeProps(
action,
getState(prevStateRef.current, action.props),
Expand All @@ -228,7 +234,7 @@ function useEnhancedReducer(reducer, props, createInitialState) {
}

prevStateRef.current = state
}, [state, props, action])
}, [state, action, isStateEqual])

return [state, dispatchWithProps]
}
Expand All @@ -240,13 +246,20 @@ function useEnhancedReducer(reducer, props, createInitialState) {
* @param {Function} reducer Reducer function from downshift.
* @param {Object} props The hook props, also passed to createInitialState.
* @param {Function} createInitialState Function that returns the initial state.
* @param {Function} isStateEqual Function that checks if a previous state is equal to the next.
* @returns {Array} An array with the state and an action dispatcher.
*/
function useControlledReducer(reducer, props, createInitialState) {
function useControlledReducer(
reducer,
props,
createInitialState,
isStateEqual,
) {
const [state, dispatch] = useEnhancedReducer(
reducer,
props,
createInitialState,
isStateEqual,
)

return [getState(state, props), dispatch]
Expand Down Expand Up @@ -585,6 +598,23 @@ function getChangesOnSelection(props, highlightedIndex, inputValue = true) {
}
}

/**
* Check if a state is equal for dropdowns, by comparing isOpen, inputValue, highlightedIndex and selected item.
* Used by useSelect and useCombobox.
*
* @param {Object} prevState
* @param {Object} newState
* @returns {boolean} Wheather the states are deeply equal.
*/
function isDropdownsStateEqual(prevState, newState) {
return (
prevState.isOpen === newState.isOpen &&
prevState.inputValue === newState.inputValue &&
prevState.highlightedIndex === newState.highlightedIndex &&
prevState.selectedItem === newState.selectedItem
)
}

// Shared between all exports.
const commonPropTypes = {
environment: PropTypes.shape({
Expand Down Expand Up @@ -646,6 +676,7 @@ export {
getItemAndIndex,
useElementIds,
getChangesOnSelection,
isDropdownsStateEqual,
commonDropdownPropTypes,
commonPropTypes,
}

0 comments on commit 4edfc30

Please sign in to comment.