Skip to content

[codex] fix slider lint issues#1070

Merged
zombieJ merged 1 commit into
masterfrom
codex/fix-slider-lint
May 12, 2026
Merged

[codex] fix slider lint issues#1070
zombieJ merged 1 commit into
masterfrom
codex/fix-slider-lint

Conversation

@zombieJ
Copy link
Copy Markdown
Member

@zombieJ zombieJ commented May 12, 2026

Summary

  • Fix React hook exhaustive-deps lint warnings in Slider without changing effect trigger timing by reading the latest values from refs.
  • Fix IDE/strict-null TypeScript diagnostics while keeping the existing public/internal number-shaped APIs intact.
  • Use existing runtime semantics for hidden active handles, merged tracks, nullable demo values, optional marks, optional count, and step={null} handling.

Validation

  • npm run lint
  • npm run tsc
  • npx tsc --noEmit --strictNullChecks --pretty false
  • npm test -- Range.test.tsx --runInBand
  • npm test -- --runInBand

Summary by CodeRabbit

发布说明

  • Refactor

    • 改进了 TypeScript 类型安全性,统一处理 null/undefined 值
    • 优化了组件属性和状态的类型注解
    • 增强了事件处理和引用管理的类型定义
  • Tests

    • 更新了测试中的类型断言以提高类型安全性

Review Change Stack

@vercel
Copy link
Copy Markdown

vercel Bot commented May 12, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
slider Error Error May 12, 2026 9:54am

@coderabbitai
Copy link
Copy Markdown
Contributor

coderabbitai Bot commented May 12, 2026

Caution

Review failed

The pull request is closed.

ℹ️ Recent review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro

Run ID: d34ddd01-43cb-430d-9f24-05e9307a6796

📥 Commits

Reviewing files that changed from the base of the PR and between 0e4e031 and 37e9a29.

📒 Files selected for processing (11)
  • docs/examples/components/TooltipSlider.tsx
  • docs/examples/slider.tsx
  • src/Handles/Handle.tsx
  • src/Handles/index.tsx
  • src/Marks/index.tsx
  • src/Slider.tsx
  • src/Tracks/index.tsx
  • src/hooks/useDrag.ts
  • src/hooks/useOffset.ts
  • src/hooks/useRange.ts
  • tests/Range.test.tsx

概览

该 PR 统一了 React Slider 组件中的空值处理策略。通过用显式的非空断言替代隐式假设、引入类型细化(例如用 undefined 替代 null 作为哨兵值)以及为所有 ref 和状态添加明确的类型注解,提高了整体的 TypeScript 类型安全性和代码可维护性。

变更

类型细化和空值处理

层级 / 文件 概述
Handle 组件键盘和 tabIndex 处理
src/Handles/Handle.tsx
onKeyDown 中的 offset 哨兵值从 null 改为 undefined;渲染的 tabIndex 在禁用或计算值为空时返回 undefined 而不是 null
Slider 主组件 ref 管理和焦点控制
src/Slider.tsx
handlesRefcontainerRef 重新定义为可空 ref,后续的 DOM 交互和命令式 API 调用使用非空断言;autoFocusRef 跟踪自动焦点行为;键盘状态初始化为 null! 并在清理时重置;markList 生成通过 marks || {} 确保安全访问;pointCount 计算改为 count !== undefined 检查。
useDrag hook 事件监听器类型和清理
src/hooks/useDrag.ts
containerRef 参数类型允许 nulluseState 调用使用显式泛型类型;事件处理程序 ref 强制为 EventListener/EventTarget 类型;onMouseMoveonMouseUp 处理程序重写为类型化函数变量,添加 preventDefault() 调用;监听器清理逻辑改进以条件性地移除处理程序。
useOffset 和 useRange hook 类型细化
src/hooks/useOffset.ts, src/hooks/useRange.ts
formatStepValue 返回 null! 而不是 nulloffsetValue 添加显式最终返回路径;useRange 中的布尔值显式强制转换。
Marks、Tracks 和 Handles 组件 prop 处理
src/Marks/index.tsx, src/Tracks/index.tsx, src/Handles/index.tsx
Marks 添加 marks 默认值;Tracks 和 Handles 中的 prop 从 null 改为 null!
示例和测试中的类型调整
docs/examples/components/TooltipSlider.tsx, docs/examples/slider.tsx, tests/Range.test.tsx
ref 和状态初始化使用显式类型注解;DOM 查询结果添加非空断言;tabIndex null 用例使用 as any 转换。

可能相关的 PR

建议审查者

  • yoyo837

诗歌

🐰 我们的指针长了眼睛,
nullundefined 分道扬镳,
非空断言让 TypeScript 欢呼,
ref 初始化如此明确,
滑块更安全了呢!


🎯 3 (中等复杂) | ⏱️ ~20 分钟

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/fix-slider-lint

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@codecov
Copy link
Copy Markdown

codecov Bot commented May 12, 2026

Codecov Report

❌ Patch coverage is 97.82609% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 99.25%. Comparing base (0e4e031) to head (37e9a29).
⚠️ Report is 1 commits behind head on master.

Files with missing lines Patch % Lines
src/hooks/useOffset.ts 66.66% 1 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           master    #1070      +/-   ##
==========================================
- Coverage   99.39%   99.25%   -0.15%     
==========================================
  Files          14       14              
  Lines         661      669       +8     
  Branches      191      206      +15     
==========================================
+ Hits          657      664       +7     
- Misses          4        5       +1     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

Copy link
Copy Markdown

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request refactors src/Slider.tsx to use React refs for tracking rawValues, draggingValue, and autoFocus within effects, and cleans up the markList memoization logic. Feedback indicates that updating ref values during the render phase should be avoided to maintain component purity and ensure compatibility with Concurrent Rendering, recommending the use of useEffect for these updates.

Comment thread src/Slider.tsx Outdated
Comment on lines +292 to +293
const rawValuesRef = React.useRef(rawValues);
rawValuesRef.current = rawValues;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Updating a ref's value during the render phase is discouraged in React as it makes the component impure. This can lead to inconsistent behavior, especially with Concurrent Rendering. It is recommended to sync the ref within an effect hook to ensure the component remains pure during rendering.

Suggested change
const rawValuesRef = React.useRef(rawValues);
rawValuesRef.current = rawValues;
const rawValuesRef = React.useRef(rawValues);
React.useEffect(() => {
rawValuesRef.current = rawValues;
}, [rawValues]);

Comment thread src/Slider.tsx Outdated
Comment on lines +356 to +357
const draggingValueRef = React.useRef(draggingValue);
draggingValueRef.current = draggingValue;
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Similar to the rawValuesRef update, assigning a value to draggingValueRef.current during the render phase should be avoided to maintain component purity. Moving this assignment to an effect ensures it happens after the render is committed.

  const draggingValueRef = React.useRef(draggingValue);
  React.useEffect(() => {
    draggingValueRef.current = draggingValue;
  }, [draggingValue]);

@zombieJ zombieJ force-pushed the codex/fix-slider-lint branch from 61cd367 to 37e9a29 Compare May 12, 2026 09:54
@zombieJ zombieJ marked this pull request as ready for review May 12, 2026 09:56
@zombieJ zombieJ merged commit c455f73 into master May 12, 2026
8 of 12 checks passed
@zombieJ zombieJ deleted the codex/fix-slider-lint branch May 12, 2026 09:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant