feat: 🎸 Enable react-hooks lint rules and fix all stale-closure

.changeset/react-hooks-lint.md

@@ -0,0 +1,16 @@
+---
+"@vega-ui/hooks": minor
+"@vega-ui/react": patch
+---
+
+Enable `react-hooks` lint rules repo-wide; fix all stale-closure findings; add `useEventCallback`
+
+`eslint-plugin-react-hooks` was installed but none of its rules were enabled — the class of bugs behind the recent Slider and Calendar stale closures was invisible to CI. `rules-of-hooks` and `exhaustive-deps` now run as errors (with `rules-of-hooks` off for Storybook CSF `render` functions, which are false positives).
+
+- new `@vega-ui/hooks` export: `useEventCallback` — a stable-identity wrapper that always calls the latest handler (the industry "latest ref" pattern à la MUI/Radix/Floating UI), throws in development if called during render
+- `useScrollSnap`: `onSnapChanging`/`onSnapChange`/`getSnapPoint` go through `useEventCallback`, so inline consumer callbacks no longer resubscribe the `scrollend` listener and observers on every render; the remaining deps are honest
+- `Calendar`: `onSelectDay` no longer freezes the first `onChange` — it reads the latest callback through `useEventCallback` while keeping a stable identity (regression-tested by swapping `onChange` between renders)
+- `NumberField`: `decrement` no longer clamps against a stale `max`; the native non-passive `wheel` listener is attached only while `changeOnWheel` is enabled and no longer resubscribes when `min`/`max`/`step` or the value change
+- `useSelection`: a consumer-provided `resolveRange` no longer cascades identity changes into `select`/`expand`/`toggle`
+- `Collapsible`: `onChangeHidden` fires only when `hidden` actually changes — an inline callback no longer re-triggers the notification effect on every parent render
+- all remaining findings were stable-value dependencies added for free, plus three documented intentional suppressions (`useMutationObserver` per-field options deps, `createContext` shallow-by-value memoization, `SnapScrollerContent` unmount-only unregistration)

eslint.config.mjs

@@ -20,9 +20,18 @@ export default tseslint.config([
       'object-curly-spacing': ['error', 'always'],
       'jsx-quotes': ['error', 'prefer-single'],
       'react/react-in-jsx-scope': 'off',
+      'react-hooks/rules-of-hooks': 'error',
+      'react-hooks/exhaustive-deps': 'error',
       '@typescript-eslint/strict-boolean-expressions': 'off',
       '@typescript-eslint/no-floating-promises': 'off',
       '@typescript-eslint/explicit-function-return-type': 'off'
     },
+  },
+  {
+    // Storybook CSF `render` functions use hooks but are not named like components
+    files: ['**/*.stories.tsx', '**/*.stories.ts'],
+    rules: {
+      'react-hooks/rules-of-hooks': 'off',
+    },
   }
 ]);

packages/hooks/src/index.ts

@@ -1,6 +1,7 @@
 export * from './useResize'
 export * from './useIsomorphicLayoutEffect'
 export * from './useLatest'
+export * from './useEventCallback'
 export * from './useControlledState'
 export * from './useRefMap'
 export * from './useSelection'

packages/hooks/src/useEventCallback.ts

@@ -0,0 +1,32 @@
+import { useCallback, useInsertionEffect, useRef } from 'react';
+
+const renderGuard = () => {
+  throw new Error('useEventCallback: the handler must not be called during render.');
+};
+
+/**
+ * Returns a function with a stable identity that always calls the latest `fn`.
+ *
+ * For event handlers only: the wrapped function is non-reactive by design and
+ * must never be called during render (throws in development if it is).
+ * Functions used while rendering should use `useCallback` with honest deps instead.
+ */
+export function useEventCallback<Args extends unknown[], R>(
+  fn: (...args: Args) => R,
+): (...args: Args) => R;
+export function useEventCallback<Args extends unknown[], R>(
+  fn?: (...args: Args) => R,
+): (...args: Args) => R | undefined;
+export function useEventCallback<Args extends unknown[], R>(
+  fn?: (...args: Args) => R,
+): (...args: Args) => R | undefined {
+  const ref = useRef<((...args: Args) => R) | undefined>(
+    process.env.NODE_ENV === 'production' ? fn : (renderGuard as never),
+  );
+
+  useInsertionEffect(() => {
+    ref.current = fn;
+  });
+
+  return useCallback((...args: Args) => ref.current?.(...args), []);
+}

packages/hooks/src/useMutationObserver.ts

@@ -21,6 +21,9 @@ export function useMutationObserver<T extends Node>(
     observer.observe(target, options ?? { childList: true });
 
     return () => observer.disconnect();
+    // options itself is intentionally not a dependency: its fields are listed one by one,
+    // so inline options objects don't reconnect the observer on every render
+    // eslint-disable-next-line react-hooks/exhaustive-deps
   }, [
     ref,
     options?.attributes,

packages/hooks/src/useScrollSnap.ts

@@ -1,6 +1,7 @@
 import { RefObject, UIEvent, useCallback, useEffect, useLayoutEffect, useRef } from 'react';
 import { useMutationObserver } from './useMutationObserver';
 import { useResizeObserver } from './useResizeObserver';
+import { useEventCallback } from './useEventCallback';
 import { nearest } from '@vega-ui/utils';
 import { useBiMap } from './useBiMap';
 
@@ -76,20 +77,25 @@ export const useScrollSnap = <K extends string | number, E extends HTMLElement>(
   align = 'start',
   getSnapPoint,
   respectScrollPadding = true,
-  onSnapChanging,
-  onSnapChange,
+  onSnapChanging: _onSnapChanging,
+  onSnapChange: _onSnapChange,
   scrollEndDebounceMs = 80,
 }: UseScrollSnapOptions<K, E>) => {
   const points = useBiMap<K, number>()
   const items = useBiMap<K, HTMLElement>()
 
   const setItemRef = useCallback((key: K) => (element: HTMLElement) => {
     items.set(key, element);
-  }, [])
-  
+  }, [items])
+
   const removeItemRef = useCallback((key: K) => {
     items?.deleteByKey(key)
-  }, [])
+  }, [items])
+
+  const onSnapChanging = useEventCallback(_onSnapChanging)
+  const onSnapChange = useEventCallback(_onSnapChange)
+  const customSnapPoint = useEventCallback(getSnapPoint)
+  const hasCustomSnapPoint = getSnapPoint != null
 
   const committedKey = useRef<K | null>(null);
   const pendingKey = useRef<K | null>(null);
@@ -104,10 +110,10 @@ export const useScrollSnap = <K extends string | number, E extends HTMLElement>(
     const scroller = scrollerRef.current;
     if (!scroller) return;
 
-    return getSnapPoint
-      ? getSnapPoint(scroller, element, axis, align)
+    return hasCustomSnapPoint
+      ? customSnapPoint(scroller, element, axis, align)
       : defaultGetSnapPoint(scroller, element, axis, align, respectScrollPadding);
-  }, [respectScrollPadding, axis, align])
+  }, [respectScrollPadding, axis, align, hasCustomSnapPoint, customSnapPoint, scrollerRef])
 
   const getPointed = useCallback((scroller?: HTMLElement) => {
     const s = scroller ?? scrollerRef.current;
@@ -125,7 +131,7 @@ export const useScrollSnap = <K extends string | number, E extends HTMLElement>(
     const element = (key !== null ? items.getByKey(key) : null) ?? null
 
     return { key, element }
-  }, [axis])
+  }, [axis, points, items, scrollerRef])
 
   const measure = useCallback(() => {
     points.clear();
@@ -182,7 +188,7 @@ export const useScrollSnap = <K extends string | number, E extends HTMLElement>(
 
     committedKey.current = key;
     pendingKey.current = key;
-    if (element && key !== null) onSnapChange?.(element, key);
+    if (element && key !== null) onSnapChange(element, key);
   }, [onSnapChange, getPointed])
 
   const scheduleCommitFallback = useCallback(() => {
@@ -206,13 +212,13 @@ export const useScrollSnap = <K extends string | number, E extends HTMLElement>(
 
       if (key !== pendingKey.current) {
         pendingKey.current = key;
-        if (element && key !== null) onSnapChanging?.(element, key);
+        if (element && key !== null) onSnapChanging(element, key);
       }
 
       // Native-like: commit only on real scroll end when possible
       if (!hasNativeScrollEndRef.current) scheduleCommitFallback();
     });
-  }, [onSnapChanging, scheduleMeasure, scheduleCommitFallback]);
+  }, [onSnapChanging, scheduleMeasure, scheduleCommitFallback, getPointed, points]);
 
   const scrollToElement = useCallback((el: HTMLElement, behavior: ScrollBehavior = 'smooth') => {
     const scroller = scrollerRef.current;
@@ -231,14 +237,14 @@ export const useScrollSnap = <K extends string | number, E extends HTMLElement>(
     }
 
     scroller.scrollTo({ left: target, behavior });
-  }, [axis, scheduleMeasure, calculateSnapPoint])
+  }, [axis, scheduleMeasure, calculateSnapPoint, items, points, scrollerRef])
 
   const scrollToElementByKey = useCallback((key: K, behavior: ScrollBehavior = 'smooth') => {
     const item = items.getByKey(key as K)
     if (!item) return
 
     scrollToElement(item, behavior)
-  }, [scrollToElement])
+  }, [scrollToElement, items])
 
   const getCommited = useCallback(() => committedKey.current, [])
   const getPending = useCallback(() => pendingKey.current, [])

packages/hooks/src/useSelection.ts

@@ -1,6 +1,7 @@
 import { useCallback, useMemo } from 'react';
 import { compare as defaultCompare } from '@vega-ui/utils';
 import { useControlledState } from './useControlledState';
+import { useEventCallback } from './useEventCallback';
 
 export type Selection = 'single' | 'multiple' | 'range'
 export type SelectedValue<M extends Selection, K> = M extends 'single' ? K : K[]
@@ -36,6 +37,7 @@ export const useSelection = <K, const M extends Selection>({
 
   const eq = useMemo(() => equals ?? ((a: K, b: K) => Object.is(a, b)), [equals])
   const comparator = useMemo(() => compare ?? (defaultCompare as unknown as (a: K, b: K) => -1 | 0 | 1), [compare])
+  const resolveRangeOrDefault = useEventCallback(resolveRange ?? ((start: K, end: K): K[] => [start, end]))
 
   const edges = useCallback((): [K, K] | [] => {
     if (selection !== 'range') return []
@@ -99,7 +101,7 @@ export const useSelection = <K, const M extends Selection>({
 
       setSelected([key] as SelectedValue<M, K>)
     }
-  }, [eq, selected])
+  }, [eq, selected, selection, setSelected])
 
   const select = useCallback((key: K) => {
     if (isDisabled(key)) return
@@ -116,9 +118,7 @@ export const useSelection = <K, const M extends Selection>({
       if (eq(start, key)) return
 
       if (array.length === 1) {
-        const range = resolveRange
-          ? resolveRange(start, key)
-          : [start, key]
+        const range = resolveRangeOrDefault(start, key)
         setSelected(range as SelectedValue<M, K>)
         return
       }
@@ -136,12 +136,11 @@ export const useSelection = <K, const M extends Selection>({
     }
 
     setSelected(key as SelectedValue<M, K>)
-  }, [selection, selected, eq, resolveRange, isDisabled])
+  }, [selection, selected, eq, resolveRangeOrDefault, isDisabled, setSelected])
 
   const resolvedRange = useCallback((start: K, end: K): SelectedValue<M, K> => {
-    const value = resolveRange ? resolveRange(start, end) : [start, end]
-    return value as SelectedValue<M, K>
-  }, [resolveRange])
+    return resolveRangeOrDefault(start, end) as SelectedValue<M, K>
+  }, [resolveRangeOrDefault])
 
   const expand = useCallback((key: K, edge?: 0 | 1) => {
     if (selection !== 'range') return
@@ -178,7 +177,7 @@ export const useSelection = <K, const M extends Selection>({
       setSelected(resolvedRange(start, key))
       return
     }
-  }, [selection, comparator, eq, resolvedRange, edges])
+  }, [selection, comparator, eq, resolvedRange, edges, setSelected])
 
   const toggle = useCallback((key: K) => {
     if (isSelected(key)) unselect(key)

packages/react-context/src/createContext.tsx

@@ -5,6 +5,9 @@ export const createContext = <D extends object | null>(name: string, defaultCont
   Context.displayName = name + 'Context';
 
   const Provider: FC<PropsWithChildren<D>> = ({ children, ...props }) => {
+    // Intentional shallow-by-value memoization: the context value keeps its identity
+    // as long as every prop is referentially equal. The dep list can't be a literal here.
+    // eslint-disable-next-line react-hooks/exhaustive-deps
     const value = useMemo(() => props, Object.values(props)) as D;
     return <Context.Provider value={value}>{children}</Context.Provider>;
   };

packages/ui/src/Calendar/Calendar.tsx

@@ -1,7 +1,7 @@
 import { useCallback, useRef } from 'react';
 import { IndexedSnapScrollerApiRef } from '../IndexedSnapScroller';
 import { CalendarProvider } from './contexts';
-import { useControlledState } from '@vega-ui/hooks';
+import { useControlledState, useEventCallback } from '@vega-ui/hooks';
 import { CalendarDatesDisabled, CalendarPicker, CalendarSelection, CalendarValue } from './types';
 import { DataGridApiRef } from '../DataGrid';
 import { CalendarBase, CalendarBaseProps } from '../CalendarBase';
@@ -202,7 +202,7 @@ export const Calendar = <S extends CalendarSelection>({
 
   const openDayPicker = useCallback(() => {
     setActivePicker('day')
-  }, [])
+  }, [setActivePicker])
 
   const closePicker = useCallback(() => {
     openDayPicker()
@@ -252,10 +252,10 @@ export const Calendar = <S extends CalendarSelection>({
     focusAvailable(value, clampedMonth)
   }
 
-  const onSelectDay = useCallback((day: number | number[]) => {
+  const onSelectDay = useEventCallback((day: number | number[]) => {
     const value = Array.isArray(day) ? day.map(d => new Date(d)) : new Date(day)
     onChange?.(value as CalendarValue<S>)
-  }, [])
+  })
 
   const toggleMonthPicker = useCallback(() => {
     if (activePicker === 'month') {
@@ -265,7 +265,7 @@ export const Calendar = <S extends CalendarSelection>({
 
     setActivePicker('month')
     requestAnimationFrame(() => focusPickerValue(monthPickerApiRef.current, date.getMonth()))
-  }, [activePicker, date, openDayPicker])
+  }, [activePicker, date, openDayPicker, setActivePicker])
 
   const toggleYearPicker = useCallback(() => {
     if (activePicker === 'year') {
@@ -275,7 +275,7 @@ export const Calendar = <S extends CalendarSelection>({
 
     setActivePicker('year')
     requestAnimationFrame(() => focusPickerValue(yearPickerApiRef.current, date.getFullYear()))
-  }, [activePicker, date, openDayPicker])
+  }, [activePicker, date, openDayPicker, setActivePicker])
 
   const nextPeriod = useCallback(() => scrollerApiRef.current?.next(), [])
   const nextYearGroup = useCallback(() => yearScrollerApiRef.current?.next(), [])

packages/ui/src/Calendar/__tests__/Calendar.browser.test.tsx

@@ -420,6 +420,24 @@ describe('Calendar', () => {
   });
 
   describe('Edge Cases', () => {
+    it('calls the latest onChange after it changes between renders', async () => {
+      const initialOnChange = vi.fn();
+      const nextOnChange = vi.fn();
+
+      const r = render(<CalendarTest onChange={initialOnChange} />);
+      r.rerender(<CalendarTest onChange={nextOnChange} />);
+
+      const day = r
+        .getAllByText('15')
+        .find((el) => el.closest('[role="gridcell"]')?.getAttribute('aria-disabled') === 'false');
+
+      await userEvent.click(day!);
+
+      expect(nextOnChange).toHaveBeenCalledTimes(1);
+      expect(nextOnChange).toHaveBeenCalledWith(expect.any(Date));
+      expect(initialOnChange).not.toHaveBeenCalled();
+    });
+
     it('supports multiple calendars rendered together: renders two month picker buttons', async () => {
       const r = render(
         <>

packages/ui/src/Collapsible/Collapsible.tsx

@@ -1,7 +1,7 @@
 'use client';
 import { FC, ReactNode, useCallback, useEffect, useId, useState } from 'react';
 import { CollapsibleProvider } from './contexts';
-import { useControlledState } from '@vega-ui/hooks';
+import { useControlledState, useEventCallback } from '@vega-ui/hooks';
 
 export interface CollapsibleProps {
   /**
@@ -51,7 +51,7 @@ export const Collapsible: FC<CollapsibleProps> = ({
   open: controlledOpen,
   defaultOpen = false,
   onChangeOpen: onControlledChangeOpen,
-  onChangeHidden,
+  onChangeHidden: _onChangeHidden,
   contentId,
   children
 }) => {
@@ -68,8 +68,10 @@ export const Collapsible: FC<CollapsibleProps> = ({
     onChangeOpen?.(false)
   }, [onChangeOpen])
 
+  const onChangeHidden = useEventCallback(_onChangeHidden)
+
   useEffect(() => {
-    onChangeHidden?.(hidden)
+    onChangeHidden(hidden)
   }, [hidden, onChangeHidden])
 
   const onOpenContent = useCallback(() => {

packages/ui/src/DataGrid/DataGrid.tsx

@@ -104,22 +104,22 @@ export const DataGrid = <K extends DataGridCellKey = DataGridCellKey>({
 
   const [active, setActive] = useControlledState<K>(_active, defaultActive ?? '' as K, onChangeActive)
 
-  useImperativeHandle(apiRef, () => ({ grid, scopes }), [])
+  useImperativeHandle(apiRef, () => ({ grid, scopes }), [grid, scopes])
 
   const setItemRef = useCallback((coordinates: DataGridCoordinates, key: K, scope: DataGridScope) => (element: HTMLDivElement) => {
     grid.addNode(coordinates, key, element)
 
     const currentScope = scopes.get(scope) ?? []
     scopes.set(scope, [...currentScope, key])
-  }, [])
+  }, [grid, scopes])
 
   const removeItemRef = useCallback((coordinates: DataGridCoordinates, key: K, scope: DataGridScope) => {
     grid.removeNode(coordinates)
 
     const currentScope = scopes.get(scope) ?? []
     scopes.set(scope, currentScope.filter(v => v !== key))
     if (scopes.get(scope)?.length === 0) scopes.delete(scope)
-  }, [])
+  }, [grid, scopes])
 
   const changeFocus = (node: MatrixNode<HTMLElement, K>) => {
     node.payload?.focus()