fix: update element ids when related props change (#1656)

We used to memoize the element id props on the first render and then use
those throughout the lifecycle of the component. This could cause ids in
the markup to be out of sync from the ids provided via props. This
change updates the internal element id getter references whenever any id
related props change to propagate those props changes to the markup.

Author: juzerzarif

src/hooks/__tests__/utils.test.js

@@ -6,9 +6,61 @@ import {
   useMouseAndTouchTracker,
   getItemAndIndex,
   isDropdownsStateEqual,
+  useElementIds,
 } from '../utils'
 
 describe('utils', () => {
+  describe('useElementIds', () => {
+    test('returns the same reference on re-renders when the props do not change', () => {
+      const getTestItemId = () => 'test-item-id'
+      const {result, rerender} = renderHook(useElementIds, {
+        initialProps: {
+          id: 'test-id',
+          labelId: 'test-label-id',
+          menuId: 'test-menu-id',
+          getItemId: getTestItemId,
+          toggleButtonId: 'test-toggle-button-id',
+          inputId: 'test-input-id',
+        },
+      })
+      const renderOneResult = result.current
+      rerender({
+        id: 'test-id',
+        labelId: 'test-label-id',
+        menuId: 'test-menu-id',
+        getItemId: getTestItemId,
+        toggleButtonId: 'test-toggle-button-id',
+        inputId: 'test-input-id',
+      })
+      const renderTwoResult = result.current
+      expect(renderOneResult).toBe(renderTwoResult)
+    })
+
+    test('returns a new reference on re-renders when the props change', () => {
+      const {result, rerender} = renderHook(useElementIds, {
+        initialProps: {
+          id: 'test-id',
+          labelId: 'test-label-id',
+          menuId: 'test-menu-id',
+          getItemId: () => 'test-item-id',
+          toggleButtonId: 'test-toggle-button-id',
+          inputId: 'test-input-id',
+        },
+      })
+      const renderOneResult = result.current
+      rerender({
+        id: 'test-id',
+        labelId: 'test-label-id',
+        menuId: 'test-menu-id',
+        getItemId: () => 'test-item-id',
+        toggleButtonId: 'test-toggle-button-id',
+        inputId: 'test-input-id',
+      })
+      const renderTwoResult = result.current
+      expect(renderOneResult).not.toBe(renderTwoResult)
+    })
+  })
+
   describe('itemToString', () => {
     test('returns empty string if item is falsy', () => {
       const emptyString = defaultProps.itemToString(null)

src/hooks/utils.js

@@ -4,6 +4,7 @@ import React, {
   useReducer,
   useEffect,
   useLayoutEffect,
+  useMemo,
 } from 'react'
 import PropTypes from 'prop-types'
 import {isReactNative} from '../is.macro'
@@ -97,15 +98,18 @@ const useElementIds =
           id = reactId
         }
 
-        const elementIdsRef = useRef({
-          labelId: labelId || `${id}-label`,
-          menuId: menuId || `${id}-menu`,
-          getItemId: getItemId || (index => `${id}-item-${index}`),
-          toggleButtonId: toggleButtonId || `${id}-toggle-button`,
-          inputId: inputId || `${id}-input`,
-        })
+        const elementIds = useMemo(
+          () => ({
+            labelId: labelId || `${id}-label`,
+            menuId: menuId || `${id}-menu`,
+            getItemId: getItemId || (index => `${id}-item-${index}`),
+            toggleButtonId: toggleButtonId || `${id}-toggle-button`,
+            inputId: inputId || `${id}-input`,
+          }),
+          [getItemId, id, inputId, labelId, menuId, toggleButtonId],
+        )
 
-        return elementIdsRef.current
+        return elementIds
       }
     : function useElementIds({
         id = `downshift-${generateId()}`,
@@ -115,15 +119,18 @@ const useElementIds =
         toggleButtonId,
         inputId,
       }) {
-        const elementIdsRef = useRef({
-          labelId: labelId || `${id}-label`,
-          menuId: menuId || `${id}-menu`,
-          getItemId: getItemId || (index => `${id}-item-${index}`),
-          toggleButtonId: toggleButtonId || `${id}-toggle-button`,
-          inputId: inputId || `${id}-input`,
-        })
-
-        return elementIdsRef.current
+        const elementIds = useMemo(
+          () => ({
+            labelId: labelId || `${id}-label`,
+            menuId: menuId || `${id}-menu`,
+            getItemId: getItemId || (index => `${id}-item-${index}`),
+            toggleButtonId: toggleButtonId || `${id}-toggle-button`,
+            inputId: inputId || `${id}-input`,
+          }),
+          [getItemId, id, inputId, labelId, menuId, toggleButtonId],
+        )
+
+        return elementIds
       }
 
 function getItemAndIndex(itemProp, indexProp, items, errorMessage) {