feat(hooks): itemToKey prop (#1573)

* change for useCombobox reducer

* useCombo docs

* finish useCombo tests

* types

* docs

* finish changes

* correct docs

Author: silviuaavram

src/hooks/useCombobox/README.md

@@ -81,6 +81,7 @@ and update if necessary.
   - [defaultIsOpen](#defaultisopen)
   - [defaultHighlightedIndex](#defaulthighlightedindex)
   - [defaultInputValue](#defaultinputvalue)
+  - [itemToKey](#itemtokey)
   - [selectedItemChanged](#selecteditemchanged)
   - [getA11yStatusMessage](#geta11ystatusmessage)
   - [getA11ySelectionMessage](#geta11yselectionmessage)
@@ -393,8 +394,55 @@ reset or when an item is selected.
 Pass a string that sets the content of the input when downshift is reset or when
 an item is selected.
 
+### itemToKey
+
+> `function(item: any)` | defaults to: `item => item`
+
+Used to determine the uniqueness of an item when searching for the item or
+comparing the item with another. Returns the item itself, by default, so the
+comparing/searching is done internally via referential equality.
+
+If using items as objects and their reference will change during use, you can
+use the function to generate a unique key for each item, such as an `id` prop.
+
+```js
+function itemToKey(item) {
+  return item.id
+}
+```
+
+> This deprecates the "selectedItemChanged" prop. If you are using the prop
+> already, make sure you change to "itemToKey" as the former will be removed in
+> the next Breaking Change update. A migration example:
+
+```js
+// initial items.
+const items = [
+  {id: 1, value: 'Apples'},
+  {id: 2, value: 'Oranges'},
+]
+// the same items but with different references, for any reason.
+const newItems = [
+  {id: 1, value: 'Apples'},
+  {id: 2, value: 'Oranges'},
+]
+
+// previously, if you probably had something like this.
+function selectedItemChanged(item1, item2) {
+  return item1.id === item2.id
+}
+
+// moving forward, switch to this one.
+function itemToKey(item) {
+  return item.id
+  // and we will do the comparison like: const isChanged = itemToKey(prevSelectedItem) !== itemToKey(nextSelectedItem)
+}
+```
+
 ### selectedItemChanged
 
+> DEPRECATED. Please use "itemToKey".
+
 > `function(prevItem: any, item: any)` | defaults to:
 > `(prevItem, item) => (prevItem !== item)`
 

src/hooks/useCombobox/__tests__/props.test.js

@@ -165,6 +165,9 @@ describe('props', () => {
     })
 
     test('props update of selectedItem will not update inputValue state if selectedItemChanged returns false', () => {
+      const consoleWarnSpy = jest
+        .spyOn(console, 'warn')
+        .mockImplementation(() => {})
       const initialSelectedItem = {id: 1, value: 'hmm'}
       const selectedItem = {id: 1, value: 'wow'}
       function itemToString(item) {
@@ -197,6 +200,116 @@ describe('props', () => {
         initialSelectedItem,
         selectedItem,
       )
+      expect(consoleWarnSpy).toHaveBeenCalledTimes(1)
+      expect(consoleWarnSpy).toHaveBeenCalledWith(
+        `The "selectedItemChanged" is deprecated. Please use "itemToKey instead". https://github.com/downshift-js/downshift/blob/master/src/hooks/useCombobox/README.md#selecteditemchanged`,
+      )
+      consoleWarnSpy.mockRestore()
+    })
+  })
+
+  describe('itemToKey', () => {
+    test('props update of selectedItem will update inputValue state with default itemToKey referential equality check', () => {
+      const initialSelectedItem = {id: 3, value: 'init'}
+      const selectedItem = {id: 1, value: 'wow'}
+      const newSelectedItem = {id: 1, value: 'not wow'}
+      function itemToString(item) {
+        return item.value
+      }
+      const stateReducer = jest
+        .fn()
+        .mockImplementation((_state, {changes}) => changes)
+
+      const {rerender} = renderCombobox({
+        stateReducer,
+        itemToString,
+        selectedItem: initialSelectedItem,
+      })
+
+      expect(stateReducer).not.toHaveBeenCalled() // won't get called on first render
+
+      rerender({
+        stateReducer,
+        itemToString,
+        selectedItem,
+      })
+
+      expect(stateReducer).toHaveBeenCalledTimes(1)
+      expect(stateReducer).toHaveBeenCalledWith(
+        {
+          inputValue: itemToString(initialSelectedItem),
+          selectedItem,
+          highlightedIndex: -1,
+          isOpen: false,
+        },
+        expect.objectContaining({
+          type: useCombobox.stateChangeTypes.ControlledPropUpdatedSelectedItem,
+          changes: {
+            inputValue: itemToString(selectedItem),
+            selectedItem,
+            highlightedIndex: -1,
+            isOpen: false,
+          },
+        }),
+      )
+
+      stateReducer.mockClear()
+      rerender({
+        stateReducer,
+        selectedItem: newSelectedItem,
+        itemToString,
+      })
+
+      expect(stateReducer).toHaveBeenCalledTimes(1)
+      expect(stateReducer).toHaveBeenCalledWith(
+        {
+          inputValue: itemToString(selectedItem),
+          selectedItem: newSelectedItem,
+          highlightedIndex: -1,
+          isOpen: false,
+        },
+        expect.objectContaining({
+          changes: {
+            inputValue: itemToString(newSelectedItem),
+            selectedItem: newSelectedItem,
+            highlightedIndex: -1,
+            isOpen: false,
+          },
+          type: useCombobox.stateChangeTypes.ControlledPropUpdatedSelectedItem,
+        }),
+      )
+      expect(getInput()).toHaveValue(itemToString(newSelectedItem))
+    })
+
+    test('props update of selectedItem will not update inputValue state if itemToKey returns equal values', () => {
+      const initialSelectedItem = {id: 1, value: 'hmm'}
+      const selectedItem = {id: 1, value: 'wow'}
+      function itemToString(item) {
+        return item.value
+      }
+      const itemToKey = jest.fn().mockImplementation(item => item.id)
+      const stateReducer = jest
+        .fn()
+        .mockImplementation((_state, {changes}) => changes)
+
+      const {rerender} = renderCombobox({
+        itemToKey,
+        stateReducer,
+        selectedItem: initialSelectedItem,
+        itemToString,
+      })
+
+      rerender({
+        itemToKey,
+        stateReducer,
+        selectedItem,
+        itemToString,
+      })
+
+      expect(getInput()).toHaveValue(itemToString(initialSelectedItem))
+      expect(itemToKey).toHaveBeenCalledTimes(2)
+      expect(itemToKey).toHaveBeenNthCalledWith(1, selectedItem)
+      expect(itemToKey).toHaveBeenNthCalledWith(2, initialSelectedItem)
     })
   })
 
@@ -601,6 +714,67 @@ describe('props', () => {
     expect(input).toHaveValue(selectedItem)
   })
 
+  test('selectedItem change updates the input value', async () => {
+    const selectedItem = items[2]
+    const newSelectedItem = items[4]
+    const nullSelectedItem = null
+    const lastSelectedItem = items[1]
+    const stateReducer = jest.fn().mockImplementation((s, a) => a.changes)
+
+    const {rerender} = renderCombobox({
+      selectedItem,
+      stateReducer,
+    })
+    const input = getInput()
+
+    expect(input).toHaveValue(selectedItem)
+    expect(stateReducer).not.toHaveBeenCalled() // don't call on first render.
+
+    rerender({
+      selectedItem: newSelectedItem,
+      stateReducer,
+    })
+
+    expect(stateReducer).toHaveBeenCalledTimes(1)
+    expect(stateReducer).toHaveBeenCalledWith(
+      expect.anything(),
+      expect.objectContaining({
+        type: useCombobox.stateChangeTypes.ControlledPropUpdatedSelectedItem,
+      }),
+    )
+    expect(input).toHaveValue(newSelectedItem)
+
+    stateReducer.mockClear()
+    rerender({
+      selectedItem: nullSelectedItem,
+      stateReducer,
+    })
+
+    expect(stateReducer).toHaveBeenCalledTimes(1)
+    expect(stateReducer).toHaveBeenCalledWith(
+      expect.anything(),
+      expect.objectContaining({
+        type: useCombobox.stateChangeTypes.ControlledPropUpdatedSelectedItem,
+      }),
+    )
+    expect(input).toHaveValue('')
+
+    stateReducer.mockClear()
+    rerender({
+      selectedItem: lastSelectedItem,
+      stateReducer,
+    })
+
+    expect(stateReducer).toHaveBeenCalledTimes(1)
+    expect(stateReducer).toHaveBeenCalledWith(
+      expect.anything(),
+      expect.objectContaining({
+        type: useCombobox.stateChangeTypes.ControlledPropUpdatedSelectedItem,
+      }),
+    )
+    expect(input).toHaveValue(lastSelectedItem)
+  })
+
   describe('stateReducer', () => {
     beforeEach(() => jest.useFakeTimers())
     afterEach(() => {

src/hooks/useCombobox/utils.js

@@ -83,16 +83,31 @@ export function useControlledReducer(
     }
 
     if (
-      !isInitialMount && // on first mount we already have the proper inputValue for a initial selected item.
-      props.selectedItemChanged(
-        previousSelectedItemRef.current,
-        props.selectedItem,
-      )
+      !isInitialMount // on first mount we already have the proper inputValue for a initial selected item.
     ) {
-      dispatch({
-        type: ControlledPropUpdatedSelectedItem,
-        inputValue: props.itemToString(props.selectedItem),
-      })
+      let shouldCallDispatch
+
+      if (props.selectedItemChanged === undefined) {
+        shouldCallDispatch =
+          props.itemToKey(props.selectedItem) !==
+          props.itemToKey(previousSelectedItemRef.current)
+      } else {
+        console.warn(
+          `The "selectedItemChanged" is deprecated. Please use "itemToKey instead". https://github.com/downshift-js/downshift/blob/master/src/hooks/useCombobox/README.md#selecteditemchanged`,
+        )
+
+        shouldCallDispatch = props.selectedItemChanged(
+          previousSelectedItemRef.current,
+          props.selectedItem,
+        )
+      }
+
+      if (shouldCallDispatch) {
+        dispatch({
+          type: ControlledPropUpdatedSelectedItem,
+          inputValue: props.itemToString(props.selectedItem),
+        })
+      }
     }
 
     previousSelectedItemRef.current =
@@ -116,7 +131,6 @@ if (process.env.NODE_ENV !== 'production') {
 
 export const defaultProps = {
   ...defaultPropsCommon,
-  selectedItemChanged: (prevItem, item) => prevItem !== item,
   getA11yStatusMessage,
   isItemDisabled() {
     return false

src/hooks/useMultipleSelection/README.md

@@ -44,6 +44,7 @@ such as when an item has been removed from selection.
   - [initialActiveIndex](#initialactiveindex)
   - [defaultSelectedItems](#defaultselecteditems)
   - [defaultActiveIndex](#defaultactiveindex)
+  - [itemToKey](#itemtokey)
   - [getA11yRemovalMessage](#geta11yremovalmessage)
   - [onActiveIndexChange](#onactiveindexchange)
   - [onStateChange](#onstatechange)
@@ -423,6 +424,35 @@ Pass an array of items that are going to be used when downshift is reset.
 Pass a number that sets the index of the focused / active selected item when
 downshift is reset.
 
+### itemToKey
+
+> `function(item: any)` | defaults to: `item => item`
+
+Used to determine the uniqueness of an item when searching for the item or
+comparing the item with another. Returns the item itself, by default, so the
+comparing/searching is done internally via referential equality.
+
+If using items as objects and their reference will change during use, you can
+use the function to generate a unique key for each item, such as an `id` prop.
+
+```js
+// initial items.
+const selectedItems = [
+  {id: 1, value: 'Apples'},
+  {id: 2, value: 'Oranges'},
+]
+// the same items but with different references, for any reason.
+const newSelectedItems = [
+  {id: 1, value: 'Apples'},
+  {id: 2, value: 'Oranges'},
+]
+
+function itemToKey(item) {
+  return item.id
+  // and we will do the comparison like: const isChanged = itemToKey(prevSelectedItem) !== itemToKey(nextSelectedItem)
+}
+```
+
 ### getA11yRemovalMessage
 
 > `function({/* see below */})` | default messages provided in English

src/hooks/useMultipleSelection/index.js

@@ -63,7 +63,10 @@ function useMultipleSelection(userProps = {}) {
 
     if (selectedItems.length < previousSelectedItemsRef.current.length) {
       const removedSelectedItem = previousSelectedItemsRef.current.find(
-        item => selectedItems.indexOf(item) < 0,
+        selectedItem =>
+          selectedItems.findIndex(
+            item => props.itemToKey(item) === props.itemToKey(selectedItem),
+          ) < 0,
       )
 
       setStatus(

src/hooks/useMultipleSelection/reducer.js

@@ -74,7 +74,9 @@ export default function downshiftMultipleSelectionReducer(state, action) {
       break
     case stateChangeTypes.FunctionRemoveSelectedItem: {
       let newActiveIndex = activeIndex
-      const selectedItemIndex = selectedItems.indexOf(selectedItem)
+      const selectedItemIndex = selectedItems.findIndex(
+        item => props.itemToKey(item) === props.itemToKey(selectedItem),
+      )
 
       if (selectedItemIndex < 0) {
         break

src/hooks/useMultipleSelection/utils.js

@@ -127,6 +127,7 @@ const propTypes = {
 
 export const defaultProps = {
   itemToString: defaultPropsCommon.itemToString,
+  itemToKey: defaultPropsCommon.itemToKey,
   stateReducer: defaultPropsCommon.stateReducer,
   environment: defaultPropsCommon.environment,
   getA11yRemovalMessage,