Conversation
- LoginForm/LoginFormPage: 为 logo 添加 属性 - ProLayout: 背景图 添加 ,JSDoc 示例补充 - ProLayout: 将 useEffect 中的派生状态改为渲染时推导 menuLoading - LightFilter: 使用 key 重置 moreValues,移除 useEffect 派生状态 - SearchSelect: 使用 useControlledState 替代 useState+useEffect 派生 - EditableTable: 移除未使用的 _useEditableDataSource 及相关死代码 - FormItem/Group: 为隐藏区域 div 添加 key - demos/tests: 为 img 添加 alt,为 SearchInput 添加 key react-doctor 评分: 79 → 96 Co-authored-by: 陈帅 <[email protected]>
|
Cursor Agent can help with this pull request. Just |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthrough本次变更主要改进可访问性与键盘交互(为图片添加 Changes
Sequence Diagram(s)sequenceDiagram
participant ProLayout as ProLayout
participant MenuRequest as Menu.request
participant SWR as SWR
ProLayout->>ProLayout: menuOnLoadingChange(rgba(0,128,0,0.5)) true
ProLayout->>MenuRequest: 发起 menu.request()
MenuRequest->>SWR: 触发 isValidating
MenuRequest-->>ProLayout: 返回数据 / finally
ProLayout->>ProLayout: menuOnLoadingChange(rgba(255,0,0,0.5)) false
Note right of SWR: menuLoading = menu?.loading ?? (menu?.request ? isValidating : menuLoadingState)
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 1 | ❌ 2❌ Failed checks (1 warning, 1 inconclusive)
✅ Passed checks (1 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
Summary of ChangesHello @chenshuai2144, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request significantly enhances the project's health and stability by resolving 16 Highlights
🧠 New Feature in Public Preview: You can now enable Memory to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request improves the project's health by addressing several issues reported by react-doctor. The changes include adding missing alt attributes to images and key attributes to list elements, which are great for accessibility and React best practices.
The refactoring to remove derived state from useEffect and useMemo in LightFilter and ProLayout is a significant improvement for maintainability and correctness.
However, I've found a couple of instances where useControlledState from @rc-component/util seems to be used incorrectly. The arguments appear to be swapped, which could lead to unexpected behavior in controlled components. Please see my detailed comments on these.
A11y 修复: - 将可点击的 <a> 改为 <button>(demos/tests) - FormItem/Group: 为可折叠标题添加 role、tabIndex、onKeyDown - 测试: 为 menuItemRender 的 div 添加键盘事件支持 正确性修复: - ProLayout: bgLayoutImgList 使用 item.src 作为 key 替代 index 架构优化: - LightFilter: 将 renderCollapseLabelRender 提取为 collapseLabelNode useMemo react-doctor: 96 → 99,警告 33 → 3(剩余为组件拆分建议) Co-authored-by: 陈帅 <[email protected]>
There was a problem hiding this comment.
Pull request overview
This PR improves the React project health by fixing 16 react-doctor errors, increasing the health score from 79 to 96. The changes focus on accessibility improvements, proper React Hook usage, and eliminating anti-patterns.
Changes:
- Added missing
altattributes to all<img>tags throughout the codebase for accessibility compliance - Enhanced keyboard accessibility by adding
role="button",tabIndex={0}, andonKeyDownhandlers to clickable div elements - Replaced anchor tags with proper button elements where appropriate, using unstyled button styling
- Fixed React Hook issues by migrating from
useState+useEffectpattern touseControlledStatehook - Eliminated derived state anti-patterns in
useEffecthooks - Added missing
keyprops to list items with meaningful identifiers - Removed unused code (
_useEditableDataSourcefunction and related helpers) from EditableTable component
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| tests/layout/index.test.tsx | Added accessibility attributes and replaced anchor tags with buttons in test fixtures |
| tests/layout/snapshots/index.test.tsx.snap | Updated snapshots to reflect CSS class changes and structural modifications |
| src/table/components/EditableTable/index.tsx | Removed unused _useEditableDataSource function and related helper functions |
| src/layout/ProLayout.tsx | Fixed derived state issue with menu loading, added alt attributes, improved key props |
| src/form/layouts/LoginFormPage/index.tsx | Added alt attribute to logo image |
| src/form/layouts/LoginForm/index.tsx | Added alt attribute to logo image |
| src/form/layouts/LightFilter/index.tsx | Eliminated derived state by using component key-based remounting, converted function to memoized value |
| src/form/components/FormItem/Group/index.tsx | Added keyboard accessibility handlers and key prop to hidden div |
| src/field/components/Select/SearchSelect/index.tsx | Migrated from useState+useEffect to useControlledState hook |
| demos/layout/topMode.tsx | Added alt attribute, keyboard accessibility, and replaced anchor with button |
| demos/layout/base.tsx | Added alt attribute, keyboard accessibility, key prop, and replaced anchor with button |
| demos/layout/appList-group-simple.tsx | Added alt attribute to image |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 3
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
demos/layout/base.tsx (1)
1-508:⚠️ Potential issue | 🟡 Minor需要更新快照
Pipeline 失败显示快照不匹配,这是由于 DOM 结构变更(如添加
role、tabIndex属性和替换元素类型)导致的。请运行以下命令更新快照:pnpm run test -u🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@demos/layout/base.tsx` around lines 1 - 508, Tests are failing because the DOM changed (e.g., added role/tabIndex and element replacements in menuItemRender/headerTitleRender/MenuCard), so update the Jest snapshots by running the test update command; run pnpm run test -u from the repo root, review the updated snapshot files to ensure the new attributes (menuItemRender's role/tabIndex/keyboard handling and headerTitleRender/MenuCard markup) are intended, then commit the updated snapshot changes.
🧹 Nitpick comments (2)
src/form/components/FormItem/Group/index.tsx (1)
168-181: 建议添加aria-label和aria-expanded以完善无障碍支持。根据编码规范,具有
role="button"的元素应包含aria-label属性。对于可折叠的组件,aria-expanded也能帮助屏幕阅读器用户理解当前状态。此外,按照规范,事件处理函数应使用 "handle" 前缀命名(如
handleTitleClick、handleTitleKeyDown)。♻️ 建议的改进
+ const handleTitleClick = useCallback(() => { + setCollapsed(!collapsed); + }, [collapsed, setCollapsed]); + + const handleTitleKeyDown = useCallback( + (e: React.KeyboardEvent) => { + if (e.key === 'Enter' || e.key === ' ') { + e.preventDefault(); + setCollapsed(!collapsed); + } + }, + [collapsed, setCollapsed], + );然后在 JSX 中使用:
<div role="button" tabIndex={0} + aria-label={typeof title === 'string' ? title : undefined} + aria-expanded={collapsible ? !collapsed : undefined} className={clsx(`${className}-title`, hashId)} style={titleStyle} - onClick={() => { - setCollapsed(!collapsed); - }} - onKeyDown={(e) => { - if (e.key === 'Enter' || e.key === ' ') { - e.preventDefault(); - setCollapsed(!collapsed); - } - }} + onClick={handleTitleClick} + onKeyDown={handleTitleKeyDown} >Based on learnings: "Implement accessibility features on elements. For example, a tag should have a tabindex="0", aria-label, on:click, and on:keydown, and similar attributes."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/components/FormItem/Group/index.tsx` around lines 168 - 181, The clickable title div using role="button" should include accessibility attributes and clearer handler names: add aria-label (e.g., `${className}-collapse-toggle`) and aria-expanded={String(!collapsed)} to the div, and extract the inline handlers into named functions handleTitleClick (calls setCollapsed(!collapsed)) and handleTitleKeyDown (checks e.key === 'Enter' || e.key === ' ' to preventDefault and toggle via setCollapsed). Update the JSX for the element that uses className, hashId, titleStyle and setCollapsed/collapsed to use handleTitleClick and handleTitleKeyDown and ensure the aria attributes are present.src/form/layouts/LightFilter/index.tsx (1)
312-312: 使用 JSON.stringify 作为 key 可能存在性能问题使用
JSON.stringify(values || {})作为 key 会导致:
- 每次 values 变化时,LightFilterContainer 会完全卸载并重新挂载
- JSON.stringify 对于复杂对象的序列化成本较高
- 可能导致子组件的内部状态丢失
如果目的是在 values 变化时重置 moreValues,建议考虑以下替代方案:
♻️ 建议使用更轻量的 key 策略
可以考虑使用 values 的长度或特定字段的组合作为 key,而不是完整的 JSON 序列化:
- key={JSON.stringify(values || {})} + key={Object.keys(values || {}).join(',')}或者,如果确实需要在 values 变化时同步 moreValues,可以在 LightFilterContainer 中使用 useEffect:
useEffect(() => { setMoreValues({ ...values }); }, [values]);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/form/layouts/LightFilter/index.tsx` at line 312, The current key={JSON.stringify(values || {})} on LightFilterContainer causes full unmount/remount on any values change and is expensive; replace this strategy by using a lighter, stable key (e.g., a composite of specific identifying fields or values length) or remove the key entirely and instead synchronize moreValues inside LightFilterContainer by watching values and calling setMoreValues({ ...values }) in a useEffect; locate the key usage and the LightFilterContainer component and apply one of these fixes to avoid costly JSON.stringify and preserve child state.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@demos/layout/topMode.tsx`:
- Line 358: Fix the typo in the onClick handler string: replace the malformed
log text "titheaderTitle clicked" with the correct "headerTitle clicked" inside
the onClick callback (the inline arrow function used for the headerTitle click
handler) in topMode.tsx so the console message correctly reads "headerTitle
clicked".
In `@src/layout/ProLayout.tsx`:
- Line 762: The bgLayoutImgList rendering uses item.src as the preferred key
(see key={item.src ?? `bg-layout-${index}`}) which can produce duplicate keys
when multiple items share the same src; update the key to guarantee uniqueness
by combining item.src with a stable unique identifier such as the index or an
item id (e.g., use `${item.src ?? 'bg-layout'}-${index}` or item.id when
available) in the component rendering bgLayoutImgList so each element has a
deterministic, unique key; modify the key expression where key={item.src ??
`bg-layout-${index}`} is used (in the ProLayout component) to the new combined
unique-key format.
- Around line 452-455: The internal menu loading setter (_setMenuLoadingInner)
is never called, so menuLoadingState doesn't update on subsequent requests;
update the code paths that notify loading (the menuOnLoadingChange handler and
any effect watching isLoading used around isLoading and
actionRef.current.reload) to call _setMenuLoadingInner(true/false) when loading
starts/ends and then call menu?.onLoadingChange?.(value). Specifically, inside
the menuOnLoadingChange function call _setMenuLoadingInner(loading) before/after
invoking menu?.onLoadingChange?.(loading), and also ensure the effect or logic
that reacts to isLoading (the code near where isLoading is used around lines
~518) sets _setMenuLoadingInner(isLoading) so SWR revalidation and
actionRef.current.reload scenarios reflect the true loading state.
---
Outside diff comments:
In `@demos/layout/base.tsx`:
- Around line 1-508: Tests are failing because the DOM changed (e.g., added
role/tabIndex and element replacements in
menuItemRender/headerTitleRender/MenuCard), so update the Jest snapshots by
running the test update command; run pnpm run test -u from the repo root, review
the updated snapshot files to ensure the new attributes (menuItemRender's
role/tabIndex/keyboard handling and headerTitleRender/MenuCard markup) are
intended, then commit the updated snapshot changes.
---
Nitpick comments:
In `@src/form/components/FormItem/Group/index.tsx`:
- Around line 168-181: The clickable title div using role="button" should
include accessibility attributes and clearer handler names: add aria-label
(e.g., `${className}-collapse-toggle`) and aria-expanded={String(!collapsed)} to
the div, and extract the inline handlers into named functions handleTitleClick
(calls setCollapsed(!collapsed)) and handleTitleKeyDown (checks e.key ===
'Enter' || e.key === ' ' to preventDefault and toggle via setCollapsed). Update
the JSX for the element that uses className, hashId, titleStyle and
setCollapsed/collapsed to use handleTitleClick and handleTitleKeyDown and ensure
the aria attributes are present.
In `@src/form/layouts/LightFilter/index.tsx`:
- Line 312: The current key={JSON.stringify(values || {})} on
LightFilterContainer causes full unmount/remount on any values change and is
expensive; replace this strategy by using a lighter, stable key (e.g., a
composite of specific identifying fields or values length) or remove the key
entirely and instead synchronize moreValues inside LightFilterContainer by
watching values and calling setMoreValues({ ...values }) in a useEffect; locate
the key usage and the LightFilterContainer component and apply one of these
fixes to avoid costly JSON.stringify and preserve child state.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
⛔ Files ignored due to path filters (1)
tests/layout/__snapshots__/index.test.tsx.snapis excluded by!**/*.snap
📒 Files selected for processing (11)
demos/layout/appList-group-simple.tsxdemos/layout/base.tsxdemos/layout/topMode.tsxsrc/field/components/Select/SearchSelect/index.tsxsrc/form/components/FormItem/Group/index.tsxsrc/form/layouts/LightFilter/index.tsxsrc/form/layouts/LoginForm/index.tsxsrc/form/layouts/LoginFormPage/index.tsxsrc/layout/ProLayout.tsxsrc/table/components/EditableTable/index.tsxtests/layout/index.test.tsx
- topMode: 修正 console.log 拼写 titheaderTitle -> headerTitle - ProLayout: 使用 isValidating 替代 isLoading,修复 actionRef.reload() 重刷时 菜单 loading 不更新问题(isLoading 仅在初次请求为 true,revalidate 时需 isValidating) - tests: menuItemRender 使用 vi.fn() 替代 void 表达式,提供可测试的 click 行为 useControlledState: @rc-component/util 签名为 (defaultValue, controlledValue), 当前用法正确,无需调整 Co-authored-by: 陈帅 <[email protected]>
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@tests/layout/index.test.tsx`:
- Around line 69-76: The onKeyDown handler for the element with role="button"
(where handleMenuItemClick(item.path) is called) does not call
e.preventDefault(), so pressing Space can trigger page scrolling; update the
onKeyDown callback to call e.preventDefault() when the activating keys are
detected (Space and Enter) before invoking handleMenuItemClick. Locate the
onKeyDown attached to the role="button" and add e.preventDefault() for the
branch that handles e.key === 'Enter' or e.key === ' ' (and optionally
'Spacebar'/'Space' variants) so keyboard activation matches mouse click
behavior.
ℹ️ Review info
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
demos/layout/topMode.tsxsrc/layout/ProLayout.tsxtests/layout/index.test.tsx
🚧 Files skipped from review as they are similar to previous changes (1)
- src/layout/ProLayout.tsx
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
Co-authored-by: gemini-code-assist[bot] <176961590+gemini-code-assist[bot]@users.noreply.github.com>
|
@chenshuai2144 I've opened a new pull request, #9509, to work on those changes. Once the pull request is ready, I'll request review from you. |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
src/layout/ProLayout.tsx (1)
762-762:⚠️ Potential issue | 🟡 Minor
key仍可能冲突(src重复时不唯一)。当前优先使用
item.src作为 key,多个相同src时会出现重复 key,影响 React diff 稳定性。💡 建议修改
- key={item.src ?? `bg-layout-${index}`} + key={`${item.src ?? 'bg-layout'}-${index}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/layout/ProLayout.tsx` at line 762, The current key expression (key={item.src ?? `bg-layout-${index}`}) can produce duplicates when multiple items share the same src; change the key to guarantee uniqueness by incorporating the loop index or a unique identifier (e.g., use `${item.src ?? 'bg-layout'}-${index}` or prefer item.id if available) so the key is always distinct—update the JSX where key is set for the mapped items (the key prop that currently references item.src and `bg-layout-${index}`) to a deterministic unique string.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/layout/ProLayout.tsx`:
- Around line 497-509: The fetcher passed to useSWR is calling
menuOnLoadingChange(true/false) even when no menu.request is provided, causing
spurious loading notifications; update the async fetcher used in the useSWR call
to first check if menu?.request exists and immediately return (or resolve to
undefined) without calling menuOnLoadingChange when it's absent, and only invoke
menuOnLoadingChange(true) / menuOnLoadingChange(false) around the actual
menu.request invocation (e.g., reference the existing useSWR fetcher closure,
menu?.request, menuOnLoadingChange, and route?.children/route?.routes to locate
and change the logic).
---
Duplicate comments:
In `@src/layout/ProLayout.tsx`:
- Line 762: The current key expression (key={item.src ?? `bg-layout-${index}`})
can produce duplicates when multiple items share the same src; change the key to
guarantee uniqueness by incorporating the loop index or a unique identifier
(e.g., use `${item.src ?? 'bg-layout'}-${index}` or prefer item.id if available)
so the key is always distinct—update the JSX where key is set for the mapped
items (the key prop that currently references item.src and `bg-layout-${index}`)
to a deterministic unique string.
| const { data, mutate, isValidating } = useSWR( | ||
| [defaultId, menu?.params], | ||
| async ([, params]) => { | ||
| setMenuLoading(true); | ||
| const menuDataItems = await menu?.request?.( | ||
| params || {}, | ||
| route?.children || route?.routes || [], | ||
| ); | ||
| setMenuLoading(false); | ||
| return menuDataItems; | ||
| menuOnLoadingChange(true); | ||
| try { | ||
| const menuDataItems = await menu?.request?.( | ||
| params || {}, | ||
| route?.children || route?.routes || [], | ||
| ); | ||
| return menuDataItems; | ||
| } finally { | ||
| menuOnLoadingChange(false); | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
cat -n src/layout/ProLayout.tsx | sed -n '490,520p'Repository: ant-design/pro-components
Length of output: 1045
🏁 Script executed:
# First, let's find the type definition for menu parameter
head -100 src/layout/ProLayout.tsx | cat -n
# Then search for menuOnLoadingChange and menu?.request usage
rg "menu\?.request|menuOnLoadingChange" src/layout/ProLayout.tsx -A 2 -B 2Repository: ant-design/pro-components
Length of output: 4702
🏁 Script executed:
# Search for the menu type definition in ProLayoutProps
rg "menu\s*\?*:" src/layout/ProLayout.tsx -A 5 | head -50
# Also check typing.ts for menu-related types
fd typing.ts src/layout/ -x cat -n {}Repository: ant-design/pro-components
Length of output: 2403
🏁 Script executed:
# Search for menu type definition in ProLayout.tsx or nearby files
rg "menu.*\?.*:\s*" src/layout/ProLayout.tsx -B 2 -A 3 | head -80
# Also search in the same directory for type definitions
fd -e ts -e tsx src/layout/ | xargs rg "interface.*Menu|type.*Menu" | grep -i "request\|onLoading"Repository: ant-design/pro-components
Length of output: 1004
🏁 Script executed:
# Search for menu type definition in ProLayout.tsx
rg "menu\s*\?" src/layout/ProLayout.tsx -B 2 -A 2 | head -100
# Search for the interface/type that defines menu with request property
rg "request\s*\?.*:|onLoadingChange\s*\?.*:" src/layout/ -r . -A 2 -B 2Repository: ant-design/pro-components
Length of output: 1765
🏁 Script executed:
# Get the menu type definition from defaultSettings.ts
cat -n src/layout/defaultSettings.ts | grep -A 30 "onLoadingChange"Repository: ant-design/pro-components
Length of output: 902
当未提供 menu.request 时,应避免触发 loading 回调。
SWR 会在 menu?.params 变化时重新运行,即使 menu?.request 为空也会调用 menuOnLoadingChange(true/false),导致外部监听方收到不必要的"伪 loading"通知。建议在函数开始处进行判空,防止无效的 loading 状态变更。
建议修改
const { data, mutate, isValidating } = useSWR(
[defaultId, menu?.params],
async ([, params]) => {
+ if (!menu?.request) {
+ return undefined;
+ }
menuOnLoadingChange(true);
try {
- const menuDataItems = await menu?.request?.(
+ const menuDataItems = await menu.request(
params || {},
route?.children || route?.routes || [],
);
return menuDataItems;
} finally {
menuOnLoadingChange(false);
}
},🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/layout/ProLayout.tsx` around lines 497 - 509, The fetcher passed to
useSWR is calling menuOnLoadingChange(true/false) even when no menu.request is
provided, causing spurious loading notifications; update the async fetcher used
in the useSWR call to first check if menu?.request exists and immediately return
(or resolve to undefined) without calling menuOnLoadingChange when it's absent,
and only invoke menuOnLoadingChange(true) / menuOnLoadingChange(false) around
the actual menu.request invocation (e.g., reference the existing useSWR fetcher
closure, menu?.request, menuOnLoadingChange, and route?.children/route?.routes
to locate and change the logic).
Co-authored-by: 陈帅 <[email protected]>
@rc-component/util 签名为 (defaultStateValue, value),非 options 对象形式, 修正 SearchSelect 和 ProLayout 的调用以避免 TS2345 声明生成失败 Co-authored-by: 陈帅 <[email protected]>
There was a problem hiding this comment.
♻️ Duplicate comments (2)
src/layout/ProLayout.tsx (2)
497-509:⚠️ Potential issue | 🟡 Minor当
menu?.request未提供时,应避免触发 loading 回调。即使
menu?.request为undefined,SWR fetcher 仍会执行并调用menuOnLoadingChange(true/false),导致不必要的 loading 状态变更。建议在调用前判空。建议修改
const { data, mutate, isValidating } = useSWR( [defaultId, menu?.params], async ([, params]) => { + if (!menu?.request) { + return undefined; + } menuOnLoadingChange(true); try { - const menuDataItems = await menu?.request?.( + const menuDataItems = await menu.request( params || {}, route?.children || route?.routes || [], ); return menuDataItems; } finally { menuOnLoadingChange(false); } },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/layout/ProLayout.tsx` around lines 497 - 509, The SWR fetcher currently calls menuOnLoadingChange(true/false) even when menu?.request is undefined; update the fetcher passed to useSWR (the async ([, params]) => { ... } function) to first check if menu?.request exists and if not return early (no menuOnLoadingChange calls), and only call menuOnLoadingChange and await menu.request(params, route?.children || route?.routes || []) when menu?.request is present; reference the fetcher function, menu?.request, and menuOnLoadingChange to locate and fix the logic.
762-762:⚠️ Potential issue | 🟡 Minor
bgLayoutImgList的 key 在多个src相同时会产生冲突。当前
key={item.src ?? \bg-layout-${index}`}仅在src为空时使用index,多项拥有相同src` 时会产生重复的 key。建议修改
- key={item.src ?? `bg-layout-${index}`} + key={`${item.src ?? 'bg-layout'}-${index}`}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/layout/ProLayout.tsx` at line 762, The key for items in bgLayoutImgList (currently rendered with key={item.src ?? `bg-layout-${index}`}) can collide when multiple items share the same src; update the key generation in the ProLayout.tsx render to ensure uniqueness by incorporating the index (or another unique identifier) together with the src (e.g., use a template like `${item.src ?? 'bg-layout'}-${index}`) so each element has a stable, unique key even when src values repeat.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Duplicate comments:
In `@src/layout/ProLayout.tsx`:
- Around line 497-509: The SWR fetcher currently calls
menuOnLoadingChange(true/false) even when menu?.request is undefined; update the
fetcher passed to useSWR (the async ([, params]) => { ... } function) to first
check if menu?.request exists and if not return early (no menuOnLoadingChange
calls), and only call menuOnLoadingChange and await menu.request(params,
route?.children || route?.routes || []) when menu?.request is present; reference
the fetcher function, menu?.request, and menuOnLoadingChange to locate and fix
the logic.
- Line 762: The key for items in bgLayoutImgList (currently rendered with
key={item.src ?? `bg-layout-${index}`}) can collide when multiple items share
the same src; update the key generation in the ProLayout.tsx render to ensure
uniqueness by incorporating the index (or another unique identifier) together
with the src (e.g., use a template like `${item.src ?? 'bg-layout'}-${index}`)
so each element has a stable, unique key even when src values repeat.
Co-authored-by: 陈帅 <[email protected]>
Fixes all 16
react-doctorerrors to improve project health score from 79 to 96.This PR addresses missing
altattributes, incorrect Hook usage in_useEditableDataSource, derived state inuseEffect, and missingkeyattributes in lists.Summary by CodeRabbit
发布说明
错误修复
新功能
改进