fix: use-controllable issue with should-update

.eslintrc

@@ -43,6 +43,7 @@
     "react/no-unescaped-entities": "off",
     "operator-assignment": "off",
     "prefer-destructuring": "off",
+    "react/no-children-prop": "off",
     "react/state-in-constructor": "off",
     "no-restricted-syntax": "off",
     "no-continue": "off",

packages/accordion/src/use-accordion.ts

@@ -102,6 +102,7 @@ export function useAccordion(props: UseAccordionProps) {
     propsMap: {
       value: "index",
       defaultValue: "defaultIndex",
+      onChange: "onChange",
     },
   })
 

packages/editable/src/use-editable.ts

@@ -96,7 +96,6 @@ export function useEditable(props: UseEditableProps = {}) {
     defaultValue: defaultValue || "",
     value: valueProp,
     onChange: onChangeProp,
-    shouldUpdate: (prev, next) => prev !== next,
   })
 
   /**

packages/hooks/src/use-controllable.ts

@@ -20,11 +20,7 @@ export interface UseControllableStateProps<T> {
   /**
    * The callback fired when the value changes
    */
-  onChange?: (nextValue: T) => void
-  /**
-   * The condition to update the state
-   */
-  shouldUpdate?: (prevState: T, state: T) => boolean
+  onChange?: (value: T) => void
   /**
    * The component name (for warnings)
    */
@@ -37,9 +33,9 @@ export interface UseControllableStateProps<T> {
    * contextual warning messages
    */
   propsMap?: {
-    value?: string
-    defaultValue?: string
-    onChange?: string
+    value: string
+    defaultValue: string
+    onChange: string
   }
 }
 
@@ -58,7 +54,6 @@ export function useControllableState<T>(props: UseControllableStateProps<T>) {
     value: valueProp,
     defaultValue,
     onChange,
-    shouldUpdate = () => true,
     name = "Component",
     propsMap = defaultPropsMap,
   } = props
@@ -84,11 +79,11 @@ export function useControllableState<T>(props: UseControllableStateProps<T>) {
     })
   }, [valueProp, isControlled, name])
 
-  const { current: initialValue } = React.useRef(defaultValue)
+  const { current: initialDefaultValue } = React.useRef(defaultValue)
 
   React.useEffect(() => {
     warn({
-      condition: initialValue !== defaultValue,
+      condition: initialDefaultValue !== defaultValue,
       message:
         `Warning: A component is changing the default value of an uncontrolled ${name} after being initialized. ` +
         `To suppress this warning opt to use a controlled ${name}.`,
@@ -97,21 +92,11 @@ export function useControllableState<T>(props: UseControllableStateProps<T>) {
 
   const value = isControlled ? (valueProp as T) : valueState
 
-  const updateValue = React.useCallback(
-    (next: React.SetStateAction<T>) => {
-      const nextValue = runIfFn(next, value)
-      const shouldUpdateState = shouldUpdate(value, nextValue)
-
-      if (!shouldUpdateState) return
-
-      if (!isControlled) {
-        setValue(next)
-      }
-
-      onChange?.(nextValue)
-    },
-    [onChange, shouldUpdate, isControlled, value],
-  )
+  const updateValue = React.useCallback((next: React.SetStateAction<T>) => {
+    const nextValue = runIfFn(next, value)
+    if (!isControlled) setValue(nextValue)
+    onChange?.(nextValue)
+  }, [])
 
   return [value, updateValue] as [T, React.Dispatch<React.SetStateAction<T>>]
 }

packages/slider/src/use-slider.ts

@@ -5,6 +5,7 @@ import {
   useEventCallback,
   useEventListener,
   useIds,
+  useUnmountEffect,
   useUpdateEffect,
 } from "@chakra-ui/hooks"
 import {
@@ -25,14 +26,7 @@ import {
   roundValueToStep,
   valueToPercent,
 } from "@chakra-ui/utils"
-import {
-  CSSProperties,
-  useCallback,
-  useEffect,
-  useMemo,
-  useRef,
-  useState,
-} from "react"
+import { CSSProperties, useCallback, useMemo, useRef, useState } from "react"
 
 export interface UseSliderProps {
   /**
@@ -171,7 +165,6 @@ export function useSlider(props: UseSliderProps) {
     value: valueProp,
     defaultValue: defaultValue ?? getDefaultValue(min, max),
     onChange,
-    shouldUpdate: (prev, next) => prev !== next,
   })
 
   /**
@@ -425,24 +418,26 @@ export function useSlider(props: UseSliderProps) {
 
     const run = (event: MouseEvent) => {
       const nextValue = getValueFromPointer(event)
-      if (nextValue != null && nextValue !== value) {
+
+      if (nextValue != null) {
         setEventSource("mouse")
         setValue(nextValue)
       }
     }
 
     run(event)
 
-    doc.addEventListener("mousemove", run)
+    doc?.addEventListener("mousemove", run)
 
     const clean = () => {
-      doc.removeEventListener("mousemove", run)
+      doc?.removeEventListener("mousemove", run)
       setDragging.off()
     }
 
-    doc.addEventListener("mouseup", clean)
+    doc?.addEventListener("mouseup", clean)
+
     cleanUpRef.current.mouseup = () => {
-      doc.removeEventListener("mouseup", clean)
+      doc?.removeEventListener("mouseup", clean)
     }
   })
 
@@ -461,30 +456,30 @@ export function useSlider(props: UseSliderProps) {
     const run = (event: TouchEvent) => {
       const nextValue = getValueFromPointer(event)
 
-      if (nextValue != null && nextValue !== value) {
+      if (nextValue != null) {
         setEventSource("touch")
         setValue(nextValue)
       }
     }
 
     run(event)
 
-    doc.addEventListener("touchmove", run)
+    doc?.addEventListener("touchmove", run)
 
     const clean = () => {
-      doc.removeEventListener("touchmove", run)
+      doc?.removeEventListener("touchmove", run)
       setDragging.off()
     }
 
-    doc.addEventListener("touchend", clean)
-    doc.addEventListener("touchcancel", clean)
+    doc?.addEventListener("touchend", clean)
+    doc?.addEventListener("touchcancel", clean)
 
     cleanUpRef.current.touchend = () => {
-      doc.removeEventListener("touchend", clean)
+      doc?.removeEventListener("touchend", clean)
     }
 
     cleanUpRef.current.touchcancel = () => {
-      doc.removeEventListener("touchcancel", clean)
+      doc?.removeEventListener("touchcancel", clean)
     }
   })
 
@@ -501,14 +496,10 @@ export function useSlider(props: UseSliderProps) {
   /**
    * Ensure we clean up listeners when slider unmounts
    */
-  useEffect(() => {
-    return () => detach()
-  }, [])
+  useUnmountEffect(detach)
 
   useUpdateEffect(() => {
-    if (!isDragging) {
-      detach()
-    }
+    if (!isDragging) detach()
   }, [isDragging])
 
   cleanUpRef.current.mousedown = useEventListener(

packages/slider/stories/slider.stories.tsx

@@ -19,14 +19,27 @@ export default {
   ],
 }
 
+export const SliderBug = () => {
+  return (
+    <Slider defaultValue={10} min={0} max={20} step={5}>
+      <SliderTrack bg="red.100">
+        <SliderFilledTrack bg="tomato" />
+      </SliderTrack>
+      <SliderThumb boxSize={6} />
+    </Slider>
+  )
+}
+
 export function HorizontalSlider() {
   return (
     <Slider colorScheme="red" onChangeEnd={console.log}>
       <SliderTrack>
         <SliderFilledTrack />
       </SliderTrack>
       <SliderThumb />
-      <SliderMark value={90} children="90%" top="20px" />
+      <SliderMark value={90} top="20px">
+        "90%"
+      </SliderMark>
     </Slider>
   )
 }

packages/tabs/src/use-tabs.ts

@@ -104,10 +104,10 @@ export function useTabs(props: UseTabsProps) {
     defaultValue: defaultIndex ?? 0,
     value: index,
     onChange,
-    shouldUpdate: (prevIndex, nextIndex) => prevIndex !== nextIndex,
     propsMap: {
       value: "index",
       defaultValue: "defaultIndex",
+      onChange: "onChange",
     },
   })