[codex] refactor notification legacy structure#384
Conversation
- Remove unused props: children, icon, title - Add duration, pauseOnHover, onClick props - Simplify component structure Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Add useNoticeTimer hook for auto-close timer with pause/resume support - Add onClose callback prop for timeout-based closing - Add pauseOnHover support (default: true) to pause timer on mouse hover - Add onClick handler support - Set default duration to 4.5 seconds Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
… types Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
|
All alerts resolved. Learn more about Socket for GitHub. This PR previously contained dependency changes with security issues that have been resolved, removed, or ignored. |
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Nitpick comments (7)
src/Progress.tsx (1)
9-11: 可选:对percent进行边界保护
<progress>元素本身能容忍越界值,但建议在传入前对percent做夹断处理(0–100),或在 JSDoc 中标注取值范围,避免上层(src/Notification.tsx中的计时器)出现 NaN/Infinity 时把异常值直接透传到 DOM。当前实现对内部使用足够,这只是一个收敛建议。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Progress.tsx` around lines 9 - 11, Progress 组件直接把传入的 percent 透传到 <progress>,存在 NaN/Infinity 或越界的风险;在 React.FC<NotificationProgressProps> 的 Progress 组件内对 percent 做数值化与边界夹断(将非数值归为 0,并 clamp 到 0–100 范围)再传给 value,或在组件上添加 JSDoc 明确要求 0–100 范围;参考并关注上层的 Notification.tsx 计时器输出以确保不会把非法值透传到 DOM。src/Notifications.tsx (2)
90-92:maxCount > 0判断与前面的真值检查重复
maxCount && maxCount > 0中,JS 里maxCount为真值时已经隐含> 0(Number.NaN、-1等异常值会落到正负判断),但语义上maxCount > 0已涵盖前者。建议简化为if (maxCount > 0 && clone.length > maxCount),可读性更好,行为不变。属于可选清理。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Notifications.tsx` around lines 90 - 92, Simplify the redundant truthy check in the notification trimming logic: locate the block that uses the variables clone and maxCount (the if that currently reads `maxCount && maxCount > 0 && clone.length > maxCount`) and replace the condition with a single numeric check that maxCount is greater than zero and that clone.length exceeds it (i.e., use a condition that checks maxCount > 0 && clone.length > maxCount) so readability is improved while behavior remains unchanged.
105-120: 建议添加竞态条件的测试用例第 115-117 行保留空数组以支持
onAllNoticeRemoved的逻辑是正确的。onAllNoticeRemoved中的保护条件if (!(clone[placement] || []).length)确保只有在 placement 数组为空时才会删除该 placement。即使在NotificationList退场动画期间有新通知添加到同一 placement,新项会存在于数组中,防止 placement 被误删,所以不存在逻辑上的竞态问题。建议补充一个测试用例,覆盖"前一组动画尚未结束之前同 placement 又有新通知插入"的场景,以确保这个并发行为在未来的维护中不会产生回归。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Notifications.tsx` around lines 105 - 120, 为防止未来回归,请为并发场景补充一个测试用例:模拟在 NotificationList 退场动画尚未完成时向同一 placement 插入新通知,验证 React.useEffect 中基于 configList 构建 placements(通过 setPlacements)仍会保留该 placement 的空数组占位(即 nextPlacements[placement] = [])且 onAllNoticeRemoved 的保护逻辑(函数 onAllNoticeRemoved)不会错误删除该 placement;在测试中使用触发退场动画的移除操作然后在动画期间调用添加操作,最后断言 placements 中该 placement 仍存在且包含新通知(引用符号:React.useEffect, configList, placements, setPlacements, onAllNoticeRemoved, NotificationList)。src/index.ts (1)
4-16: 公共 API 导出变更需要在 changelog/迁移说明中提及移除
Notice导出并新增Progress/Notification及相关类型属于面向使用方的破坏性变更,任何依赖旧Notice命名的下游项目都会编译失败。建议在 README/CHANGELOG/迁移说明里补充这次重命名的说明,并在 PR 描述中标注 BREAKING CHANGE。代码本身没有问题。🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/index.ts` around lines 4 - 16, 本次变更移除了对旧导出名 Notice 并新增了 Progress/Notification 及相关类型(在 src/index.ts 中导出的 useNotification, NotificationProvider, Progress, Notification 及类型 NotificationAPI/NotificationConfig/ComponentsType/NotificationProps/NotificationProgressProps),属于破坏性 API 变更;请在项目的 README 或 CHANGELOG 中新增一条迁移说明,明确注明 “Notice -> Notification 重命名” 以及新增 Progress 的影响和示例迁移步骤,并在 PR 描述中标注 BREAKING CHANGE;可选地,为减小升级成本,在同一提交中临时恢复对旧名 Notice 的兼容导出(例如 re-export Notice 指向 Notification)并在迁移说明中注明该兼容将于下个主要版本移除。src/hooks/useListPosition/index.ts (1)
15-44:useMemo依赖整个stack对象,父组件未 memo 时会逐帧重算依赖数组中的
stack是引用比较。若调用方useListPosition(configList, stackPosition, gap)中stackPosition在每次渲染都构造新对象(例如stack === true ? defaultStack : stack),会导致每次父组件渲染都重新计算 position map。可考虑展开为基本类型依赖以稳定缓存:♻️ 提议
- }, [configList, gap, sizeMap, stack]); + }, [configList, gap, sizeMap, stack?.offset, stack?.threshold, !!stack]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/hooks/useListPosition/index.ts` around lines 15 - 44, The useMemo inside useListPosition is currently depending on the entire stack object (stack) which causes recomputation whenever the parent recreates that object; change the dependency array to depend on primitive pieces instead (e.g., stack?.threshold, stack?.offset and a boolean existence flag like Boolean(stack)) along with configList, gap and sizeMap so the memo stabilizes; update the dependency list referenced in the useMemo call inside useListPosition to [configList, gap, sizeMap, stack?.threshold, stack?.offset, Boolean(stack)] (use optional chaining) to avoid unnecessary recalculations.src/NotificationList/index.tsx (1)
261-265:if (placement)是死分支,可以直接调用第 37 行已经把
placement: Placement声明为必填(Placement是字符串字面量联合类型,运行时也总是真值字符串),因此第 262 行的if (placement)永远为真。建议去掉这个无效守卫,或者把placement改成可选并保留判断,二选一以保持类型与运行时一致。♻️ 建议
onAllRemoved={() => { - if (placement) { - onAllRemoved?.(placement); - } + onAllRemoved?.(placement); }}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/NotificationList/index.tsx` around lines 261 - 265, The guard `if (placement)` around the callback is dead code because `placement` is declared as a required Placement (string literal union) and always truthy; remove the redundant conditional in the onAllRemoved handler and call onAllRemoved?.(placement) directly (update the handler where onAllRemoved is invoked in NotificationList/index.tsx), or alternatively make the `placement` prop optional and keep the check—prefer removing the guard to keep types and runtime consistent with `placement: Placement`.src/Notification.tsx (1)
154-165:mergedOffset/mergedNotificationIndex的回退表达式可以简化
offsetRef.current在if (offset !== undefined)时就已经被同步成最新值,所以再写offset ?? offsetRef.current等价于直接读offsetRef.current。notificationIndex同理。这块代码读起来略绕,可以更直观。♻️ 建议简化
const offsetRef = React.useRef(offset); if (offset !== undefined) { offsetRef.current = offset; } const notificationIndexRef = React.useRef(notificationIndex); if (notificationIndex !== undefined) { notificationIndexRef.current = notificationIndex; } - const mergedOffset = offset ?? offsetRef.current; - const mergedNotificationIndex = notificationIndex ?? notificationIndexRef.current ?? 0; + const mergedOffset = offsetRef.current; + const mergedNotificationIndex = notificationIndexRef.current ?? 0;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/Notification.tsx` around lines 154 - 165, 当前对 mergedOffset 和 mergedNotificationIndex 的回退写法冗余:offsetRef.current 与 notificationIndexRef.current 在对应的 if 同步后已是最新值,因此把 `offset ?? offsetRef.current` 和 `notificationIndex ?? notificationIndexRef.current ?? 0` 简化为直接读取 ref 即可;在代码中修改 mergedOffset 为 `offsetRef.current`,mergedNotificationIndex 为 `notificationIndexRef.current ?? 0`,并保留现有的 refs 和同步逻辑(offsetRef, notificationIndexRef)以保证行为不变。
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@docs/examples/NotificationList.tsx`:
- Around line 33-43: 在 createFiveConfigs 中不要在 setConfigList 的 updater 内部递增
keyRef;先在函数作用域读取并推进 keyRef(例如 const startKey = keyRef.current; keyRef.current +=
5),然後在 setConfigList 的 updater 中只使用 startKey 生成新项;也就是把 keyRef.current += 5 从
updater 中移出,保留 createNotification 与 setConfigList(prev => [...prev,
...Array.from({length:5}, (_,i) => createNotification(startKey + i))]) 的生成逻辑,以和
createConfig 的“先读后写在 updater 之外”模式保持一致,确保在并发/StrictMode 下不会重复推进
keyRef(相关符号:createFiveConfigs、setConfigList、keyRef、createNotification、createConfig)。
In `@package.json`:
- Around line 83-85: You raised the peerDependency bump of "react" and
"react-dom" to ">=18.0.0" which is a breaking change; update package.json and
release strategy to reflect a major version bump: change the package version for
this release to 2.0.0 (or otherwise follow your semver scheme for a major
release), keep the peerDependencies explicitly set to ">=18.0.0" as intended,
and add explicit notes to CHANGELOG / release notes describing the React 18
requirement and the breaking API changes (e.g., the content→description rename
and removal of src/legacy) plus migration steps so downstream consumers have
clear guidance when upgrading.
In `@src/hooks/useListPosition/useSizes.ts`:
- Around line 16-46: setNodeSize (the ref callback injected via setItemRef) only
runs on mount/unmount so height isn't re-measured when a notification's content
updates, leaving stale entries in sizeMap and causing layout shifts; fix by
attaching a ResizeObserver inside setNodeSize to observe the node and update
sizeMap on resize (ensure you store/cleanup the observer when node becomes
null), or alternately call setNodeSize(key, node) explicitly whenever the
notification config for that key is replaced (e.g., from Notifications.open) so
the latest offsetWidth/offsetHeight are recorded; reference setNodeSize and
wherever setItemRef is used to locate the injection point.
In `@src/Notification.tsx`:
- Line 251: The actions container uses a raw className "actions" which breaks
the project's prefix convention; change the rendered div to use the prefixed
class `${noticePrefixCls}-actions` (i.e. replace className="actions" with
className={`${noticePrefixCls}-actions`}) and then add the corresponding key to
the NotificationClassNames and NotificationStyles enums/records and to
NotificationList's noticeSlotKeys so consumers can style/actions via the
existing classNames/styles system; update any type definitions or exports
related to NotificationClassNames/NotificationStyles and ensure NotificationList
references include the new "actions" slot key.
- Line 54: NotificationListConfig.times is being passed down but not consumed in
Notification, so changes to times don't reset the auto-close timer; update the
Notification component to destructure the times prop, pass it into
useNoticeTimer (or otherwise use it to control/reset timing), and include times
in the effect dependency array that currently depends on [durationMs, walking]
so the hook re-runs when times changes; reference the Notification component's
prop destructuring and the useNoticeTimer call to locate and fix the code.
In `@src/NotificationList/Content.tsx`:
- Around line 14-19: The code mutates prevHeightRef.current during render
(prevHeightRef, prevHeight, heightStatus, height), which is unsafe in concurrent
rendering; change to derive previous height via state and effects: introduce a
state (e.g., prevHeightState) and compute heightStatus from current height vs
that state during render, then in a useEffect update prevHeightState = height
after paint; remove the direct assignment prevHeightRef.current = height from
render and ensure any initial value handles first render correctly so
increase/decrease logic remains stable.
In `@src/NotificationList/index.tsx`:
- Around line 210-222: The effect in NotificationList that measures gap
(React.useEffect reading window.getComputedStyle on contentRef and calling
setGap) depends only on hasConfigList, so changes to placement, className,
style, theme or responsive CSS won't retrigger measurement; update the effect to
also depend on placement, className and style (or any props that affect layout)
or, better, replace the one-off useEffect with a ResizeObserver attached to
contentRef.current that re-reads rowGap/gap and calls setGap whenever the
container size/style changes, ensuring NotificationList/useListPosition
totalHeight stays correct across placement/theme/class/style changes.
---
Nitpick comments:
In `@src/hooks/useListPosition/index.ts`:
- Around line 15-44: The useMemo inside useListPosition is currently depending
on the entire stack object (stack) which causes recomputation whenever the
parent recreates that object; change the dependency array to depend on primitive
pieces instead (e.g., stack?.threshold, stack?.offset and a boolean existence
flag like Boolean(stack)) along with configList, gap and sizeMap so the memo
stabilizes; update the dependency list referenced in the useMemo call inside
useListPosition to [configList, gap, sizeMap, stack?.threshold, stack?.offset,
Boolean(stack)] (use optional chaining) to avoid unnecessary recalculations.
In `@src/index.ts`:
- Around line 4-16: 本次变更移除了对旧导出名 Notice 并新增了 Progress/Notification 及相关类型(在
src/index.ts 中导出的 useNotification, NotificationProvider, Progress, Notification
及类型
NotificationAPI/NotificationConfig/ComponentsType/NotificationProps/NotificationProgressProps),属于破坏性
API 变更;请在项目的 README 或 CHANGELOG 中新增一条迁移说明,明确注明 “Notice -> Notification 重命名” 以及新增
Progress 的影响和示例迁移步骤,并在 PR 描述中标注 BREAKING CHANGE;可选地,为减小升级成本,在同一提交中临时恢复对旧名 Notice
的兼容导出(例如 re-export Notice 指向 Notification)并在迁移说明中注明该兼容将于下个主要版本移除。
In `@src/Notification.tsx`:
- Around line 154-165: 当前对 mergedOffset 和 mergedNotificationIndex
的回退写法冗余:offsetRef.current 与 notificationIndexRef.current 在对应的 if 同步后已是最新值,因此把
`offset ?? offsetRef.current` 和 `notificationIndex ??
notificationIndexRef.current ?? 0` 简化为直接读取 ref 即可;在代码中修改 mergedOffset 为
`offsetRef.current`,mergedNotificationIndex 为 `notificationIndexRef.current ??
0`,并保留现有的 refs 和同步逻辑(offsetRef, notificationIndexRef)以保证行为不变。
In `@src/NotificationList/index.tsx`:
- Around line 261-265: The guard `if (placement)` around the callback is dead
code because `placement` is declared as a required Placement (string literal
union) and always truthy; remove the redundant conditional in the onAllRemoved
handler and call onAllRemoved?.(placement) directly (update the handler where
onAllRemoved is invoked in NotificationList/index.tsx), or alternatively make
the `placement` prop optional and keep the check—prefer removing the guard to
keep types and runtime consistent with `placement: Placement`.
In `@src/Notifications.tsx`:
- Around line 90-92: Simplify the redundant truthy check in the notification
trimming logic: locate the block that uses the variables clone and maxCount (the
if that currently reads `maxCount && maxCount > 0 && clone.length > maxCount`)
and replace the condition with a single numeric check that maxCount is greater
than zero and that clone.length exceeds it (i.e., use a condition that checks
maxCount > 0 && clone.length > maxCount) so readability is improved while
behavior remains unchanged.
- Around line 105-120: 为防止未来回归,请为并发场景补充一个测试用例:模拟在 NotificationList 退场动画尚未完成时向同一
placement 插入新通知,验证 React.useEffect 中基于 configList 构建 placements(通过
setPlacements)仍会保留该 placement 的空数组占位(即 nextPlacements[placement] = [])且
onAllNoticeRemoved 的保护逻辑(函数 onAllNoticeRemoved)不会错误删除该
placement;在测试中使用触发退场动画的移除操作然后在动画期间调用添加操作,最后断言 placements 中该 placement
仍存在且包含新通知(引用符号:React.useEffect, configList, placements, setPlacements,
onAllNoticeRemoved, NotificationList)。
In `@src/Progress.tsx`:
- Around line 9-11: Progress 组件直接把传入的 percent 透传到 <progress>,存在 NaN/Infinity
或越界的风险;在 React.FC<NotificationProgressProps> 的 Progress 组件内对 percent
做数值化与边界夹断(将非数值归为 0,并 clamp 到 0–100 范围)再传给 value,或在组件上添加 JSDoc 明确要求 0–100
范围;参考并关注上层的 Notification.tsx 计时器输出以确保不会把非法值透传到 DOM。
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 3e87dbb5-f679-4372-bd7c-e7d8b26b07d6
📒 Files selected for processing (30)
.eslintrc.js.gitignoreassets/index.lessdocs/demo/NotificationList.mddocs/examples/NotificationList.tsxdocs/examples/context.tsxdocs/examples/hooks.tsxdocs/examples/maxCount.tsxdocs/examples/showProgress.tsxdocs/examples/stack.tsxpackage.jsonsrc/Notice.tsxsrc/NoticeList.tsxsrc/Notification.tsxsrc/NotificationList/Content.tsxsrc/NotificationList/index.tsxsrc/NotificationProvider.tsxsrc/Notifications.tsxsrc/Progress.tsxsrc/hooks/useClosable.tssrc/hooks/useListPosition/index.tssrc/hooks/useListPosition/useSizes.tssrc/hooks/useNoticeTimer.tssrc/hooks/useNotification.tsxsrc/hooks/useStack.tssrc/index.tssrc/interface.tstests/hooks.test.tsxtests/index.test.tsxtests/stack.test.tsx
💤 Files with no reviewable changes (3)
- src/interface.ts
- src/NoticeList.tsx
- src/Notice.tsx
| const createFiveConfigs = React.useCallback(() => { | ||
| setConfigList((prevConfigList) => { | ||
| const startKey = keyRef.current; | ||
| keyRef.current += 5; | ||
|
|
||
| return [ | ||
| ...prevConfigList, | ||
| ...Array.from({ length: 5 }, (_, index) => createNotification(startKey + index)), | ||
| ]; | ||
| }); | ||
| }, [createNotification]); |
There was a problem hiding this comment.
keyRef 在 setState updater 内部递增不安全
setConfigList 的 updater 在 StrictMode 与并发渲染下可能被调用多次,导致 keyRef.current += 5 被执行两次,实际推进 10。虽然 key 仍然唯一,不会引发功能性 bug,但会让生成的 key 出现跳跃,与 createConfig 中"先读后写在 updater 之外"的模式不一致。建议把 keyRef 的推进移到 updater 之外(与 createConfig 保持一致)。
♻️ 建议修复
const createFiveConfigs = React.useCallback(() => {
+ const startKey = keyRef.current;
+ keyRef.current += 5;
+
setConfigList((prevConfigList) => {
- const startKey = keyRef.current;
- keyRef.current += 5;
-
return [
...prevConfigList,
...Array.from({ length: 5 }, (_, index) => createNotification(startKey + index)),
];
});
}, [createNotification]);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const createFiveConfigs = React.useCallback(() => { | |
| setConfigList((prevConfigList) => { | |
| const startKey = keyRef.current; | |
| keyRef.current += 5; | |
| return [ | |
| ...prevConfigList, | |
| ...Array.from({ length: 5 }, (_, index) => createNotification(startKey + index)), | |
| ]; | |
| }); | |
| }, [createNotification]); | |
| const createFiveConfigs = React.useCallback(() => { | |
| const startKey = keyRef.current; | |
| keyRef.current += 5; | |
| setConfigList((prevConfigList) => { | |
| return [ | |
| ...prevConfigList, | |
| ...Array.from({ length: 5 }, (_, index) => createNotification(startKey + index)), | |
| ]; | |
| }); | |
| }, [createNotification]); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@docs/examples/NotificationList.tsx` around lines 33 - 43, 在 createFiveConfigs
中不要在 setConfigList 的 updater 内部递增 keyRef;先在函数作用域读取并推进 keyRef(例如 const startKey =
keyRef.current; keyRef.current += 5),然後在 setConfigList 的 updater 中只使用 startKey
生成新项;也就是把 keyRef.current += 5 从 updater 中移出,保留 createNotification 与
setConfigList(prev => [...prev, ...Array.from({length:5}, (_,i) =>
createNotification(startKey + i))]) 的生成逻辑,以和 createConfig 的“先读后写在 updater
之外”模式保持一致,确保在并发/StrictMode 下不会重复推进
keyRef(相关符号:createFiveConfigs、setConfigList、keyRef、createNotification、createConfig)。
| "react": ">=18.0.0", | ||
| "react-dom": ">=18.0.0" | ||
| }, |
There was a problem hiding this comment.
Peer dependency 提升至 React 18 是 breaking change,建议同步发布 major 版本。
将 react / react-dom 的 peer 范围从 >=16.9.0 抬升到 >=18.0.0 会导致仍在使用 React 16/17 的下游项目在 npm install 时收到 peer 警告并失去兼容性保证。结合本 PR 中 content → description 的 API rename、src/legacy 的移除等不向后兼容的改动,建议下次发版直接升至 2.0.0(并在 CHANGELOG / release notes 中明确列出迁移要点),避免在 1.x 线打破 SemVer 约定。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@package.json` around lines 83 - 85, You raised the peerDependency bump of
"react" and "react-dom" to ">=18.0.0" which is a breaking change; update
package.json and release strategy to reflect a major version bump: change the
package version for this release to 2.0.0 (or otherwise follow your semver
scheme for a major release), keep the peerDependencies explicitly set to
">=18.0.0" as intended, and add explicit notes to CHANGELOG / release notes
describing the React 18 requirement and the breaking API changes (e.g., the
content→description rename and removal of src/legacy) plus migration steps so
downstream consumers have clear guidance when upgrading.
| const setNodeSize = React.useCallback((key: string, node: HTMLDivElement | null) => { | ||
| if (!node) { | ||
| setSizeMap((prevSizeMap) => { | ||
| if (!(key in prevSizeMap)) { | ||
| return prevSizeMap; | ||
| } | ||
|
|
||
| const nextSizeMap = { ...prevSizeMap }; | ||
| delete nextSizeMap[key]; | ||
| return nextSizeMap; | ||
| }); | ||
| return; | ||
| } | ||
|
|
||
| const nextSize = { | ||
| width: node.offsetWidth, | ||
| height: node.offsetHeight, | ||
| }; | ||
| setSizeMap((prevSizeMap) => { | ||
| const prevSize = prevSizeMap[key]; | ||
|
|
||
| if (prevSize && prevSize.width === nextSize.width && prevSize.height === nextSize.height) { | ||
| return prevSizeMap; | ||
| } | ||
|
|
||
| return { | ||
| ...prevSizeMap, | ||
| [key]: nextSize, | ||
| }; | ||
| }); | ||
| }, []); |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 检查是否有任何 ResizeObserver/getBoundingClientRect 用于通知尺寸测量
rg -nP -C2 '\b(ResizeObserver|getBoundingClientRect)\b' --type=ts --type=tsx
# 检查 setNodeSize 的所有调用点,确认是否有内容更新后再次调用的路径
rg -nP -C3 '\bsetNodeSize\s*\(' --type=ts --type=tsxRepository: react-component/notification
Length of output: 128
🏁 Script executed:
# First, check the file types available and look for ResizeObserver usage
rg -i 'ResizeObserver' . --type-list | head -20Repository: react-component/notification
Length of output: 462
🏁 Script executed:
# Search for ResizeObserver in the codebase
rg -i 'ResizeObserver' . 2>/dev/nullRepository: react-component/notification
Length of output: 54
🏁 Script executed:
# Check for getBoundingClientRect usage
rg 'getBoundingClientRect|offsetHeight|offsetWidth' . 2>/dev/nullRepository: react-component/notification
Length of output: 339
🏁 Script executed:
# Look at the useSizes.ts file more carefully and surrounding context
cd src/hooks/useListPosition && ls -la && cat -n useSizes.ts 2>/dev/null | head -100Repository: react-component/notification
Length of output: 1786
🏁 Script executed:
# Find all references to setNodeSize
rg -n 'setNodeSize' . 2>/dev/nullRepository: react-component/notification
Length of output: 964
🏁 Script executed:
# Check NotificationListItem implementation to understand content updates
find . -name '*NotificationListItem*' -o -name '*notification-list-item*' 2>/dev/nullRepository: react-component/notification
Length of output: 54
🏁 Script executed:
# Read NotificationList/index.tsx to see how setNodeSize ref callback is used
cat -n src/NotificationList/index.tsx | head -130 | tail -50Repository: react-component/notification
Length of output: 1701
🏁 Script executed:
# Read more of NotificationList/index.tsx around lines 200-210
cat -n src/NotificationList/index.tsx | sed -n '195,215p'Repository: react-component/notification
Length of output: 753
🏁 Script executed:
# Find NotificationListItem component
find . -name '*NotificationListItem*' -o -name 'NotificationListItem*' 2>/dev/nullRepository: react-component/notification
Length of output: 54
🏁 Script executed:
# Check for the NotificationListItem definition in NotificationList directory
ls -la src/NotificationList/Repository: react-component/notification
Length of output: 304
🏁 Script executed:
# Look for how content is rendered and if components remount on update
cat -n src/NotificationList/index.tsx | sed -n '280,310p'Repository: react-component/notification
Length of output: 988
🏁 Script executed:
# Check if there's an API or manager that handles notification updates
find . -name '*.ts' -o -name '*.tsx' | xargs grep -l 'instance.open\|notificationManager' 2>/dev/null | head -10Repository: react-component/notification
Length of output: 118
🏁 Script executed:
# Look for the notification config and how it's updated
rg -n 'key.*description\|open.*description' . --type ts --type tsx 2>/dev/null | head -20Repository: react-component/notification
Length of output: 54
🏁 Script executed:
# Check the Content.tsx component which likely renders the items
cat -n src/NotificationList/Content.tsxRepository: react-component/notification
Length of output: 1413
🏁 Script executed:
# Search for how CSSMotionList processes the list
rg -n 'CSSMotionList' . 2>/dev/nullRepository: react-component/notification
Length of output: 283
🏁 Script executed:
# Check if there's a notification manager that handles updates
find . -type f \( -name '*.ts' -o -name '*.tsx' \) | xargs grep -l 'useNotification\|NotificationManager' 2>/dev/nullRepository: react-component/notification
Length of output: 332
🏁 Script executed:
# Read around CSSMotionList usage in NotificationList
cat -n src/NotificationList/index.tsx | sed -n '250,297p'Repository: react-component/notification
Length of output: 2096
🏁 Script executed:
# Check useNotification hook to understand the API
cat -n src/hooks/useNotification.tsxRepository: react-component/notification
Length of output: 6414
🏁 Script executed:
# Check the test file to see if there are tests for updating notifications with same key
cat -n tests/index.test.tsx | head -150Repository: react-component/notification
Length of output: 4660
🏁 Script executed:
# Continue reading the test file to see if there's a test for updating with same key
cat -n tests/index.test.tsx | sed -n '150,250p'Repository: react-component/notification
Length of output: 3278
🏁 Script executed:
# Find and read the Notifications component
find . -name 'Notifications*' -type f 2>/dev/nullRepository: react-component/notification
Length of output: 96
🏁 Script executed:
# Check the Notifications component
cat -n src/Notifications.tsxRepository: react-component/notification
Length of output: 6863
内容更新后高度不会重新测量,可能导致布局错位
setNodeSize 作为 ref callback 通过 setItemRef 注入,React 仅在节点挂载/卸载或 ref 本身变化时调用它。测试用例验证了真实场景:同一个 key 的通知内容更新时(见 tests/index.test.tsx 第 180-240 行),Notifications.tsx 的 open() 方法会替换该 key 对应的配置对象,但 NotificationListItem 使用稳定的 key,React 不会重新挂载组件,仅更新 props。因此 ref callback 不会重新触发,sizeMap[key] 保持旧的高度值。useListPosition 依赖该 height 计算堆叠偏移,会导致可见的位置错位。
建议改用 ResizeObserver 监听节点尺寸变化,或在配置更新后显式调用 setNodeSize(key, node) 重新测量。
♻️ ResizeObserver 方案示意
- const setNodeSize = React.useCallback((key: string, node: HTMLDivElement | null) => {
- if (!node) {
- setSizeMap((prevSizeMap) => {
- if (!(key in prevSizeMap)) {
- return prevSizeMap;
- }
-
- const nextSizeMap = { ...prevSizeMap };
- delete nextSizeMap[key];
- return nextSizeMap;
- });
- return;
- }
-
- const nextSize = {
- width: node.offsetWidth,
- height: node.offsetHeight,
- };
- setSizeMap((prevSizeMap) => {
- const prevSize = prevSizeMap[key];
-
- if (prevSize && prevSize.width === nextSize.width && prevSize.height === nextSize.height) {
- return prevSizeMap;
- }
-
- return {
- ...prevSizeMap,
- [key]: nextSize,
- };
- });
- }, []);
+ const observersRef = React.useRef<Map<string, ResizeObserver>>(new Map());
+
+ const measure = React.useCallback((key: string, node: HTMLDivElement) => {
+ setSizeMap((prev) => {
+ const next = { width: node.offsetWidth, height: node.offsetHeight };
+ const cur = prev[key];
+ if (cur && cur.width === next.width && cur.height === next.height) return prev;
+ return { ...prev, [key]: next };
+ });
+ }, []);
+
+ const setNodeSize = React.useCallback((key: string, node: HTMLDivElement | null) => {
+ const prevObserver = observersRef.current.get(key);
+ if (prevObserver) {
+ prevObserver.disconnect();
+ observersRef.current.delete(key);
+ }
+ if (!node) {
+ setSizeMap((prev) => {
+ if (!(key in prev)) return prev;
+ const next = { ...prev };
+ delete next[key];
+ return next;
+ });
+ return;
+ }
+ measure(key, node);
+ const ro = new ResizeObserver(() => measure(key, node));
+ ro.observe(node);
+ observersRef.current.set(key, ro);
+ }, [measure]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/hooks/useListPosition/useSizes.ts` around lines 16 - 46, setNodeSize (the
ref callback injected via setItemRef) only runs on mount/unmount so height isn't
re-measured when a notification's content updates, leaving stale entries in
sizeMap and causing layout shifts; fix by attaching a ResizeObserver inside
setNodeSize to observe the node and update sizeMap on resize (ensure you
store/cleanup the observer when node becomes null), or alternately call
setNodeSize(key, node) explicitly whenever the notification config for that key
is replaced (e.g., from Notifications.open) so the latest
offsetWidth/offsetHeight are recorded; reference setNodeSize and wherever
setItemRef is used to locate the injection point.
| // Behavior | ||
| duration?: number | false | null; | ||
| showProgress?: boolean; | ||
| times?: number; |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# 1) 确认 times 在源码中所有引用
rg -nP --type=ts --type=tsx -C2 '\btimes\b' -g '!**/*.test.*' -g '!**/__tests__/**'
# 2) 确认 useNoticeTimer 是否接收/消费 times
rg -nP -C3 'useNoticeTimer\s*\(' -g '!**/*.test.*'
# 3) 查看 Notifications.tsx 中是否在 open() 时维护 times 计数
fd -e ts -e tsx Notifications | xargs rg -nP -C3 '\btimes\b'Repository: react-component/notification
Length of output: 1440
🏁 Script executed:
# Check NotificationProps interface and Notification component structure
cat -n src/Notification.tsx | head -120Repository: react-component/notification
Length of output: 4000
🏁 Script executed:
# Get the full useNoticeTimer hook implementation
cat -n src/hooks/useNoticeTimer.tsRepository: react-component/notification
Length of output: 3199
🏁 Script executed:
# Find and examine Notifications.tsx file
fd -name 'Notifications.tsx' -o -name 'Notification.tsx'Repository: react-component/notification
Length of output: 303
🏁 Script executed:
# Search for NotificationListConfig to understand the times property
rg -n 'NotificationListConfig' -A5 -B2Repository: react-component/notification
Length of output: 8143
🏁 Script executed:
# Check how NotificationListItemProps is used to pass config to Notification
rg -n -A10 'NotificationListItemProps' src/NotificationList/index.tsx | head -30Repository: react-component/notification
Length of output: 743
🏁 Script executed:
# Verify the full flow from NotificationListItem to Notification component
rg -n -B3 -A5 'config\.' src/NotificationList/index.tsx | grep -E '(times|Notification)' | head -20Repository: react-component/notification
Length of output: 54
🏁 Script executed:
# Get the complete NotificationListItem component to see how config is passed to Notification
sed -n '101,200p' src/NotificationList/index.tsxRepository: react-component/notification
Length of output: 2769
🏁 Script executed:
# Double-check: see what properties NotificationListConfig has by looking at its definition
rg -n -A10 'interface NotificationListConfig' src/NotificationList/index.tsxRepository: react-component/notification
Length of output: 409
🏁 Script executed:
# Verify the Omit behavior - what properties are actually included in NotificationListConfig
rg -n -B2 -A15 'export interface NotificationListConfig' src/NotificationList/index.tsxRepository: react-component/notification
Length of output: 607
times 属性被传入但未被消费,导致"同 key 通知再次打开时重置计时器"的功能丧失
NotificationListConfig.times(第 23 行)在 Notifications.tsx 中被追踪和递增(第 83-86 行),并通过 NotificationListItem 的 {...notificationConfig} 传递到 Notification 组件(第 119、131 行)。但 Notification 的属性解构(第 67-98 行)中并未提取 times,也未将其传入 useNoticeTimer。由于 useNoticeTimer 的副作用依赖仅为 [durationMs, walking](第 95 行),当 times 改变时不会重置自动关闭计时器。
若要保留"同 key 通知再次打开时重新计时"的行为,需要:
- 在
Notification的属性解构中提取times - 将
times传入useNoticeTimer或在组件中单独处理 - 将
times纳入相关副作用的依赖数组
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Notification.tsx` at line 54, NotificationListConfig.times is being
passed down but not consumed in Notification, so changes to times don't reset
the auto-close timer; update the Notification component to destructure the times
prop, pass it into useNoticeTimer (or otherwise use it to control/reset timing),
and include times in the effect dependency array that currently depends on
[durationMs, walking] so the hook re-runs when times changes; reference the
Notification component's prop destructuring and the useNoticeTimer call to
locate and fix the code.
| </button> | ||
| )} | ||
|
|
||
| {actions && <div className="actions">{actions}</div>} |
There was a problem hiding this comment.
actions 容器缺少前缀类名
第 251 行渲染 actions 时直接使用了 className="actions",与文件中其他所有元素都遵循 ${noticePrefixCls}-xxx 规则不一致。这会与外部样式产生冲突,且无法通过约定的前缀进行样式定制。
🔧 建议修复
- {actions && <div className="actions">{actions}</div>}
+ {actions && <div className={`${noticePrefixCls}-actions`}>{actions}</div>}如果 actions 也希望支持 classNames/styles 自定义,建议同时把 actions 加入 NotificationClassNames/NotificationStyles(以及 NotificationList 的 noticeSlotKeys)。
📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| {actions && <div className="actions">{actions}</div>} | |
| {actions && <div className={`${noticePrefixCls}-actions`}>{actions}</div>} |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/Notification.tsx` at line 251, The actions container uses a raw className
"actions" which breaks the project's prefix convention; change the rendered div
to use the prefixed class `${noticePrefixCls}-actions` (i.e. replace
className="actions" with className={`${noticePrefixCls}-actions`}) and then add
the corresponding key to the NotificationClassNames and NotificationStyles
enums/records and to NotificationList's noticeSlotKeys so consumers can
style/actions via the existing classNames/styles system; update any type
definitions or exports related to NotificationClassNames/NotificationStyles and
ensure NotificationList references include the new "actions" slot key.
| // ========================= Height ========================= | ||
| const prevHeightRef = React.useRef(height); | ||
| const prevHeight = prevHeightRef.current; | ||
| const heightStatus = height < prevHeight ? 'decrease' : 'increase'; | ||
|
|
||
| prevHeightRef.current = height; |
There was a problem hiding this comment.
🛠️ Refactor suggestion | 🟠 Major
🧩 Analysis chain
🌐 Web query:
React 19 best practice for tracking previous prop value during render: useRef mutation vs useState derived state
💡 Result:
In React 19, the best practice for tracking the previous value of a prop during render is to use derived state with useState, calling the setter during render under a condition, rather than mutating useRef.current (which is forbidden during render by the React Compiler). Official React docs recommend this pattern for "storing information from previous renders": function CountLabel({ count }) { const [prevCount, setPrevCount] = useState(count); const [trend, setTrend] = useState(null); if (prevCount !== count) { setPrevCount(count); setTrend(count > prevCount ? 'increasing' : 'decreasing'); } return ( <>
{count}
{trend &&The count is {trend}
} </> ); } This updates immediately on the next render without extra re-renders for children, unlike useEffect approaches. The traditional usePrevious hook using return ref.current during render triggers ESLint errors from React Compiler: "Ref values may not be accessed during render." Workarounds exist (class-based refs evaluated in render, double useState), but docs prefer derived useState as it's explicit, predictable, and avoids ref rules. Use this for rare cases where prev prop is needed in render (e.g., animations, labels). Prefer useEffect for side effects.Citations:
- 1: https://beta.reactjs.org/reference/react/useState
- 2: https://react.dev/reference/react/useState
- 3: https://beta.reactjs.org/reference/react/useRef
- 4: https://react.dev/reference/react/useRef
- 5: https://stackoverflow.com/questions/78929408/how-to-implement-useprevious-in-react-v19-without-referencing-ref-current-during
🏁 Script executed:
cat -n src/NotificationList/Content.tsx | head -40Repository: react-component/notification
Length of output: 1413
在渲染期间写入 useRef 是反模式,建议用 state 派生
第 19 行在渲染过程中直接 mutate prevHeightRef.current。在 React 19 中,React Compiler 明确禁止在渲染期间访问或修改 ref。在 React 18/19 的并发渲染或 StrictMode 下,渲染可能被丢弃或重复执行,导致 ref 中的值与预期脱节,从而把 increase/decrease 类名计算错。React 官方推荐用"在渲染中比较 state 并 setState"的方式来派生此类状态,语义更稳定。
♻️ 建议改为 state 派生
- // ========================= Height =========================
- const prevHeightRef = React.useRef(height);
- const prevHeight = prevHeightRef.current;
- const heightStatus = height < prevHeight ? 'decrease' : 'increase';
-
- prevHeightRef.current = height;
+ // ========================= Height =========================
+ const [prevHeight, setPrevHeight] = React.useState(height);
+ const [heightStatus, setHeightStatus] = React.useState<'increase' | 'decrease'>('increase');
+ if (height !== prevHeight) {
+ setHeightStatus(height < prevHeight ? 'decrease' : 'increase');
+ setPrevHeight(height);
+ }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/NotificationList/Content.tsx` around lines 14 - 19, The code mutates
prevHeightRef.current during render (prevHeightRef, prevHeight, heightStatus,
height), which is unsafe in concurrent rendering; change to derive previous
height via state and effects: introduce a state (e.g., prevHeightState) and
compute heightStatus from current height vs that state during render, then in a
useEffect update prevHeightState = height after paint; remove the direct
assignment prevHeightRef.current = height from render and ensure any initial
value handles first render correctly so increase/decrease logic remains stable.
| React.useEffect(() => { | ||
| const listNode = contentRef.current; | ||
|
|
||
| if (!listNode) { | ||
| return; | ||
| } | ||
|
|
||
| // CSS gap impacts stack offset and total list height calculation. | ||
| const { gap: cssGap, rowGap } = window.getComputedStyle(listNode); | ||
| const nextGap = parseFloat(rowGap || cssGap) || 0; | ||
|
|
||
| setGap((prevGap) => (prevGap === nextGap ? prevGap : nextGap)); | ||
| }, [hasConfigList]); |
There was a problem hiding this comment.
gap 测量副作用依赖过窄,placement 切换或样式变化不会触发重测
当前 effect 的依赖只有 hasConfigList,意味着只有列表从"空 ↔ 非空"切换时才会重新读取 getComputedStyle。如果出现以下场景,gap 不会被刷新,会导致 useListPosition 的 totalHeight 计算偏差(列表高度多算或少算一个 gap):
placement在不同位置使用不同的gap/row-gap样式;- 主题/CSS 变量切换或响应式断点让 gap 改变;
- 用户通过
style/className在运行时调整间距。
建议把 placement(以及 className、style 中可能影响 gap 的变量)纳入依赖,或者改用 ResizeObserver 直接监听容器尺寸变化:
🔧 简单加固
- React.useEffect(() => {
+ React.useEffect(() => {
const listNode = contentRef.current;
if (!listNode) {
return;
}
// CSS gap impacts stack offset and total list height calculation.
const { gap: cssGap, rowGap } = window.getComputedStyle(listNode);
const nextGap = parseFloat(rowGap || cssGap) || 0;
setGap((prevGap) => (prevGap === nextGap ? prevGap : nextGap));
- }, [hasConfigList]);
+ }, [hasConfigList, placement, className]);如需更稳健,可改用 ResizeObserver 监听 contentRef.current 的尺寸变化时同步重新读取 gap。
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/NotificationList/index.tsx` around lines 210 - 222, The effect in
NotificationList that measures gap (React.useEffect reading
window.getComputedStyle on contentRef and calling setGap) depends only on
hasConfigList, so changes to placement, className, style, theme or responsive
CSS won't retrigger measurement; update the effect to also depend on placement,
className and style (or any props that affect layout) or, better, replace the
one-off useEffect with a ResizeObserver attached to contentRef.current that
re-reads rowGap/gap and calls setGap whenever the container size/style changes,
ensuring NotificationList/useListPosition totalHeight stays correct across
placement/theme/class/style changes.
Summary
Notification/NotificationListstructuresrc/legacyimplementation after replacing the remaining context dependencydescriptionand DOM changes, plus a minimal custom progress component testValidation
npm testnpm run lintnpm run compileSummary by CodeRabbit
发布说明
新功能
文档
Chores