chore: 修复 react-hooks lint 错误#16
Open
xPeiPeix wants to merge 1 commit into
Open
Conversation
Reviewer's GuideThis PR fixes existing react-hooks lint violations by making state initialization and ref updates render-pure, refactoring a mounted flag to useSyncExternalStore for hydration-safe behavior, and cleaning up an unused catch binding in Azure TTS utilities. Sequence diagram for ThemeToggle hydration-safe mounted flag using useSyncExternalStoresequenceDiagram
participant ReactServer
participant Browser
participant ThemeToggle
Note over ReactServer,ThemeToggle: Server-side render
ReactServer->>ThemeToggle: render()
ThemeToggle->>ThemeToggle: useSyncExternalStore(subscribe, getClientSnapshot, getServerSnapshot)
ThemeToggle-->>ReactServer: mounted = getServerSnapshot() = false
ReactServer-->>Browser: HTML with mounted = false
Note over Browser,ThemeToggle: Client hydration
Browser->>ThemeToggle: hydrate existing HTML
ThemeToggle->>ThemeToggle: useSyncExternalStore(subscribe, getClientSnapshot, getServerSnapshot)
ThemeToggle-->>Browser: mounted = getClientSnapshot() = true
Browser->>ThemeToggle: render with mounted = true
ThemeToggle->>ThemeToggle: isDark = mounted && resolvedTheme === dark
ThemeToggle-->>Browser: final UI with correct theme state
Flow diagram for VoicePicker initial selected voice without extra renderflowchart TD
A[Component mount] --> B[Initialize voices state to empty array]
B --> C[Initialize selected state via getSelectedVoiceName]
C --> D[Initialize open state to false]
D --> E[Initialize previewing state to false]
E --> F[Run effect to load voices]
F --> G[If window and speechSynthesis available]
G -->|yes| H[Call load to populate voices]
H --> I[Add voiceschanged listener calling load]
G -->|no| J[Skip loading voices]
I --> K[On unmount remove voiceschanged listener]
J --> K[Component lifecycle complete]
File-Level Changes
Tips and commandsInteracting with Sourcery
Customizing Your ExperienceAccess your dashboard to:
Getting Help
|
![sourcery-ai[bot]](https://avatars.githubusercontent.com/in/48477?s=60&v=4){kind=small}
There was a problem hiding this comment.
Hey - I've left some high level feedback:
- In
ThemeToggle, theuseSyncExternalStoreimplementation isn’t actually wired to any external store and always returnstrueon the client, which may still cause a server/client snapshot mismatch and makes the extra subscription indirection unnecessary—consider either wiring it to the actualnext-themesstate or using a documented hydration-safe pattern that ensures server and client snapshots initially match. - In
VocabCard,nowis captured once viauseState(() => Date.now()), so if the same component instance can receive a differententryprop over its lifetime,decorativeLevelwill be computed against a stale timestamp—if entries can change in place, consider keying the component byentry.idor recalculatingnowwhenentry.createdAtchanges.
Prompt for AI Agents
Please address the comments from this code review:
## Overall Comments
- In `ThemeToggle`, the `useSyncExternalStore` implementation isn’t actually wired to any external store and always returns `true` on the client, which may still cause a server/client snapshot mismatch and makes the extra subscription indirection unnecessary—consider either wiring it to the actual `next-themes` state or using a documented hydration-safe pattern that ensures server and client snapshots initially match.
- In `VocabCard`, `now` is captured once via `useState(() => Date.now())`, so if the same component instance can receive a different `entry` prop over its lifetime, `decorativeLevel` will be computed against a stale timestamp—if entries can change in place, consider keying the component by `entry.id` or recalculating `now` when `entry.createdAt` changes.Help me be more useful! Please click 👍 or 👎 on each comment and I'll use the feedback to improve your reviews.
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: da7cdf4835
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
Comment on lines
+23
to
+25
| const [selected, setSelected] = useState<string | null>(() => | ||
| getSelectedVoiceName() | ||
| ); |
There was a problem hiding this comment.
Initializing selected from getSelectedVoiceName() in useState breaks persisted voice selection on SSR/hydrated loads: this initializer runs during server render (where window is unavailable, so it returns null), and hydration reuses that state rather than re-reading localStorage on mount. Since this commit also removed the mount-time setSelected(getSelectedVoiceName()), users who refresh or land directly on the page lose their saved voice until they manually reselect one.
Useful? React with 👍 / 👎.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
清掉 master 已有的 5 处 react-hooks lint 错误，让
npm run lint重回 0 errors（仅剩 .venv vendor 的 1 条 warning，与项目源码无关）。Changes
components/vocab-card.tsx—Date.now()由 render 体移入useState(() => Date.now())在 mount 时锁定，render 保持 purecomponents/voice-picker.tsx—setSelected(getSelectedVoiceName())从 useEffect 移入 useState initializer，避免 cascading rendercomponents/youtube-player.tsx—reloadPlayerRef.current = reloadPlayer包进 useEffect，render 不再写 refcomponents/theme-toggle.tsx—useState/useEffect的 mounted 模式改用useSyncExternalStore，hydration-safe 且避免 setState in effectlib/azure-tts.ts— 删除未使用的catch (err)binding，改为 optional catch bindingKey Design Decisions
How to Review
components/vocab-card.tsx:51-56—decorativeLevel计算结果不变，仅Date.now()改为 mount 时一次性 capture（"创建至今天数 / 3" 装饰等级，无需 render 时实时更新）components/voice-picker.tsx:22-28—getSelectedVoiceName内部已有typeof window === "undefined"守卫，useState initializer 直接调用安全components/youtube-player.tsx:104-107— useEffect 不带依赖数组（每次 render 后跑），与原"render 时同步赋值"行为等价components/theme-toggle.tsx:7-22—subscribe/getClientSnapshot/getServerSnapshot三个函数提到模块作用域，避免每次 render 重建（useSyncExternalStore 用引用相等判断）Testing
npm run lint通过：源码 0 errors（剩唯一 warning 来自.venv/yt-dlp vendor 文件）npm run build通过：17 个静态页全部生成，路由清单完整Reproduce Locally
Summary by Sourcery
Resolve existing React hooks lint violations and align components with recommended React patterns.
Bug Fixes:
Enhancements: