feat: 实施性能优化、UX主题、高危操作二次确认及DX规范#54
Conversation
✅ Deploy Preview for pushy ready!
To edit notification comments on pull requests, go to your Netlify project configuration. |
📝 WalkthroughWalkthroughChangesThe PR adds production bundle analysis and explicit chunk splitting, introduces lazy chart components with loading and error fallbacks, asynchronously loads the JSON editor, and replaces app deletion confirmation with a reusable text-matching danger modal. Performance loading changes
Destructive action confirmation
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant MetricsPage
participant AsyncLine
participant Suspense
participant ChartsModule
MetricsPage->>AsyncLine: Render chart props
AsyncLine->>Suspense: Render lazy chart
Suspense-->>MetricsPage: Show ChartSkeleton
Suspense->>ChartsModule: Dynamically import Line
ChartsModule-->>AsyncLine: Provide chart module
AsyncLine-->>MetricsPage: Render chart
Possibly related PRs
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 Checkov (3.3.8)package.jsonTraceback (most recent call last): 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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 6
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@rsbuild.config.ts`:
- Around line 21-30: Update the performance.chunkSplit configuration in
rsbuild.config.ts to remove the forceSplitting entries for charts, jsoneditor,
and wasm so lazy-route dependencies are not injected into the initial HTML. If
vendor grouping is still required, recreate it through splitChunks instead while
preserving lazy loading.
In `@src/components/lazy-chart.tsx`:
- Around line 6-20: Update the LazyArea, LazyLine, LazyPie, and LazyDualAxes
loaders so their lazy wrappers and import promises are recreated when
SectionErrorBoundary invokes onReset, rather than remaining module-level
singletons. Wire the reset callback through the lazy-chart component and
preserve the existing chart component mappings while ensuring “重试组件” mounts a
newly created loader.
In `@src/pages/manage/components/json-editor.tsx`:
- Around line 53-64: Update the loading spinner container in the JSON editor
wrapper to use absolute positioning with inset 0, plus an appropriate background
and z-index, so it overlays and centers over the editor without consuming layout
space. Keep the wrapper’s existing relative positioning and loading behavior
unchanged.
- Around line 18-28: Update the editor initialization around createJSONEditor to
store the latest props in a ref and read that ref when the dynamic import
resolves, rather than using the effect’s captured props. Ensure initialization
uses current content, callbacks, and options so prop changes occurring before
refEditor.current exists are not missed.
- Around line 31-34: Update the vanilla-jsoneditor import failure handler in the
JSON editor component to set a user-facing error state when loading fails, while
preserving the existing loading-state cleanup. Render that error state in the
editor area with an actionable retry path, ensuring failures are visible to
flows using JsonEditor from version-table.tsx and deps-table.tsx.
In `@src/pages/manage/components/setting-modal.tsx`:
- Around line 17-21: Update the deletion confirmation flow around
confirmTargetText to use only a non-empty appName, removing the appKey and appId
fallbacks. Ensure deletion remains disabled or blocked when appName is unset or
cleared, and preserve the existing name-matching confirmation behavior when it
is available.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4588789f-6c65-4957-8ee8-ab596801218c
📒 Files selected for processing (11)
package.jsonrsbuild.config.tssrc/components/dangerous-confirm-modal.tsxsrc/components/lazy-chart.tsxsrc/components/section-error-boundary.tsxsrc/components/skeletons.tsxsrc/pages/admin-metrics.tsxsrc/pages/manage/components/json-editor.tsxsrc/pages/manage/components/setting-modal.tsxsrc/pages/realtime-metrics.tsxsrc/pages/version-health.tsx
| performance: { | ||
| chunkSplit: { | ||
| strategy: 'split-by-experience', | ||
| forceSplitting: { | ||
| charts: /node_modules[\\/]@ant-design[\\/]charts/, | ||
| jsoneditor: /node_modules[\\/]vanilla-jsoneditor/, | ||
| wasm: /node_modules[\\/]hash-wasm/, | ||
| }, | ||
| }, | ||
| }, |
There was a problem hiding this comment.
🚀 Performance & Scalability | 🟠 Major | 🏗️ Heavy lift
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== files =="
git ls-files | rg '(^|/)(rsbuild\.config\.ts|package\.json|pnpm-lock\.yaml|yarn\.lock|package-lock\.json|README\.md)$'
echo
echo "== rsbuild.config.ts =="
if git ls-files --error-unmatch rsbuild.config.ts >/dev/null 2>&1; then
cat -n rsbuild.config.ts | sed -n '1,120p'
fi
echo
echo "== package.json (if present) =="
if git ls-files --error-unmatch package.json >/dev/null 2>&1; then
cat -n package.json | sed -n '1,220p'
fi
echo
echo "== search forceSplitting/splitChunks =="
rg -n "forceSplitting|splitChunks|chunkSplit|performance:" -S . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**'Repository: reactnativecn/pushy-admin
Length of output: 3784
🌐 Web query:
Rsbuild forceSplitting initial HTML script resources dynamic import chunkSplit documentation
💡 Result:
In modern Rsbuild (version 2.0 and later), the previous performance.chunkSplit configuration has been deprecated in favor of a new, top-level splitChunks option [1]. This change aligns Rsbuild's configuration more closely with Rspack's optimization.splitChunks [1][2]. Key points regarding your query: 1. Deprecation of performance.chunkSplit: If you are using a recent version of Rsbuild, you should use the splitChunks configuration [1]. While backward compatibility for performance.chunkSplit exists, it is recommended to migrate to the new format [1]. 2. Force Splitting vs. Cache Groups: The legacy performance.chunkSplit.forceSplitting allowed you to easily target specific npm packages to be split into their own chunks [3][4][5]. These chunks were automatically injected into the initial HTML file as script tags [3][5]. In the new splitChunks configuration, you achieve similar control by defining custom cacheGroups [6][2]. You can define a cache group that matches specific modules (e.g., using a regex for node_modules) and give it a specific name [7][8]. Unlike the legacy forceSplitting, you have more granular control over how these chunks are handled [2]. 3. Dynamic Import: Dynamic imports (import) are handled automatically by Rsbuild and Rspack [7][8]. When Rsbuild encounters a dynamic import, it automatically splits the referenced module into a separate async chunk, which is loaded on demand at runtime rather than being included in the initial HTML script resources [7][8]. 4. Documentation: For current usage, refer to the official Rsbuild documentation under the splitChunks configuration page [2]. This documentation explains how to use presets (default, per-package, single-vendor) and how to write custom Rspack-compatible splitChunks rules [2]. If you specifically require the behavior where a chunk is forced into the initial HTML script list, ensure your Rspack configuration (via splitChunks) does not exclude these chunks from the initial bundle (the default behavior for many splitChunks configurations is to handle this automatically if they are part of the initial dependency graph) [2].
Citations:
- 1: feat: introduce new splitChunks config web-infra-dev/rsbuild#7073
- 2: https://rsbuild.rs/config/split-chunks
- 3: https://v1.rsbuild.rs/config/performance/chunk-split
- 4: https://v1.rsbuild.rs/config/performance/chunk-split.md
- 5: https://v1.rsbuild.rs/guide/optimization/code-splitting.md
- 6: https://rsbuild.rs/config/split-chunks.md
- 7: https://rsbuild.rs/guide/optimization/code-splitting
- 8: https://github.com/web-infra-dev/rsbuild/blob/main/website/docs/en/guide/optimization/code-splitting.mdx
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== imports/usages =="
rg -n "`@ant-design/charts`|vanilla-jsoneditor|hash-wasm|import\\(" src . --glob '!**/node_modules/**' --glob '!**/dist/**' --glob '!**/build/**'
echo
echo "== source tree =="
git ls-files src | sed -n '1,200p'Repository: reactnativecn/pushy-admin
Length of output: 16940
rsbuild.config.ts:21-30 — Avoid force-splitting lazy-route dependencies into the initial HTML.
forceSplitting injects these chunks as initial script tags, so charts, the JSON editor, and hash-wasm are fetched on first load even though they live behind lazy routes/components. Remove these entries, or migrate the grouping to splitChunks if you still need vendor cache-group control.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@rsbuild.config.ts` around lines 21 - 30, Update the performance.chunkSplit
configuration in rsbuild.config.ts to remove the forceSplitting entries for
charts, jsoneditor, and wasm so lazy-route dependencies are not injected into
the initial HTML. If vendor grouping is still required, recreate it through
splitChunks instead while preserving lazy loading.
| const LazyArea = lazy(() => | ||
| import('@ant-design/charts').then((module) => ({ default: module.Area })), | ||
| ); | ||
|
|
||
| const LazyLine = lazy(() => | ||
| import('@ant-design/charts').then((module) => ({ default: module.Line })), | ||
| ); | ||
|
|
||
| const LazyPie = lazy(() => | ||
| import('@ant-design/charts').then((module) => ({ default: module.Pie })), | ||
| ); | ||
|
|
||
| const LazyDualAxes = lazy(() => | ||
| import('@ant-design/charts').then((module) => ({ default: module.DualAxes })), | ||
| ); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== files ==\n'
git ls-files 'src/components/lazy-chart.tsx' 'src/components/*error*' 'src/**/SectionErrorBoundary*' 'src/**/*ErrorBoundary*' | sed 's#^`#-` #'
printf '\n== outline lazy-chart ==\n'
ast-grep outline src/components/lazy-chart.tsx --view expanded || true
printf '\n== search reset / retry wiring ==\n'
rg -n "SectionErrorBoundary|onReset|retry|lazy\\(" src/components -S || trueRepository: reactnativecn/pushy-admin
Length of output: 1957
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== lazy-chart.tsx ==\n'
cat -n src/components/lazy-chart.tsx | sed -n '1,220p'
printf '\n== error boundary files ==\n'
for f in $(git ls-files 'src/**/*ErrorBoundary*' 'src/**/*error*boundary*' 'src/components/*error*'); do
echo "--- $f"
wc -l "$f"
done | sed -n '1,200p'Repository: reactnativecn/pushy-admin
Length of output: 2756
🏁 Script executed:
#!/bin/bash
set -euo pipefail
printf '\n== section-error-boundary.tsx ==\n'
cat -n src/components/section-error-boundary.tsx | sed -n '1,220p'
printf '\n== error-boundary.tsx ==\n'
cat -n src/components/error-boundary.tsx | sed -n '1,220p'Repository: reactnativecn/pushy-admin
Length of output: 4881
🏁 Script executed:
#!/bin/bash
set -euo pipefail
sed -n '1,220p' src/components/section-error-boundary.tsx | cat -nRepository: reactnativecn/pushy-admin
Length of output: 2453
Recreate the chart lazy loaders on retry. LazyArea/LazyLine/LazyPie/LazyDualAxes are module-level lazy() singletons, so a rejected import promise stays cached. SectionErrorBoundary already exposes onReset, but src/components/lazy-chart.tsx never uses it, so “重试组件” just remounts the same failed lazy component. Pass a reset callback that rebuilds the lazy wrapper/load promise.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/components/lazy-chart.tsx` around lines 6 - 20, Update the LazyArea,
LazyLine, LazyPie, and LazyDualAxes loaders so their lazy wrappers and import
promises are recreated when SectionErrorBoundary invokes onReset, rather than
remaining module-level singletons. Wire the reset callback through the
lazy-chart component and preserve the existing chart component mappings while
ensuring “重试组件” mounts a newly created loader.
| import('vanilla-jsoneditor') | ||
| .then(({ createJSONEditor, Mode }) => { | ||
| if (destroyed || !refContainer.current) return; | ||
|
|
||
| refEditor.current = createJSONEditor({ | ||
| target: refContainer.current, | ||
| props: { | ||
| mode: Mode.text, | ||
| ...props, | ||
| }, | ||
| }); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
# Map the file and inspect the relevant ranges
ast-grep outline src/pages/manage/components/json-editor.tsx --view expanded || true
wc -l src/pages/manage/components/json-editor.tsx
cat -n src/pages/manage/components/json-editor.tsx | sed -n '1,220p'
# Find related usage of the component and prop update handling
rg -n "json-editor|createJSONEditor|refEditor|destroyed|onChange|content" src/pages/manage/components -SRepository: reactnativecn/pushy-admin
Length of output: 6245
Use the latest props when creating the editor. The dynamic import can resolve after a prop change; in that case the editor is initialized with stale content/callbacks/options, and the update effect has already missed the change while refEditor.current was still empty. Keep the latest props in a ref and read from it during initialization.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/manage/components/json-editor.tsx` around lines 18 - 28, Update the
editor initialization around createJSONEditor to store the latest props in a ref
and read that ref when the dynamic import resolves, rather than using the
effect’s captured props. Ensure initialization uses current content, callbacks,
and options so prop changes occurring before refEditor.current exists are not
missed.
| .catch((err) => { | ||
| console.error('Failed to load vanilla-jsoneditor:', err); | ||
| setLoading(false); | ||
| }); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== json-editor.tsx =="
nl -ba src/pages/manage/components/json-editor.tsx | sed -n '1,220p'
echo
echo "== version-table.tsx relevant usage =="
nl -ba src/pages/manage/components/version-table.tsx | sed -n '320,390p'
echo
echo "== search for JsonEditor usages and loading/error patterns =="
rg -n "json-editor|JsonEditor|setLoading\\(|loading.*error|catch\\(" src/pages/manage/components src/pages/manage -g '*.tsx' -g '*.ts'Repository: reactnativecn/pushy-admin
Length of output: 225
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== json-editor.tsx =="
sed -n '1,220p' src/pages/manage/components/json-editor.tsx | cat -n
echo
echo "== version-table.tsx relevant usage =="
sed -n '320,390p' src/pages/manage/components/version-table.tsx | cat -n
echo
echo "== search for JsonEditor usages and loading/error patterns =="
rg -n "json-editor|JsonEditor|setLoading\\(|loading.*error|catch\\(" src/pages/manage/components src/pages/manage -g '*.tsx' -g '*.ts'Repository: reactnativecn/pushy-admin
Length of output: 6116
🏁 Script executed:
#!/bin/bash
set -euo pipefail
echo "== deps-table.tsx relevant usage =="
sed -n '120,210p' src/pages/manage/components/deps-table.tsx | cat -n
echo
echo "== all JsonEditor import/call sites =="
rg -n "from './json-editor'|from \"./json-editor\"|<JsonEditor|lazy\\(\\(\\) => import\\('./json-editor'\\)" src/pages/manage/components -g '*.tsx' -g '*.ts'
echo
echo "== any error boundary / load-fallback patterns nearby =="
rg -n "ErrorBoundary|fallback=|Spin tip=|Failed to load vanilla-jsoneditor|vanilla-jsoneditor" src/pages/manage/components -g '*.tsx' -g '*.ts'Repository: reactnativecn/pushy-admin
Length of output: 4773
Show an error state when vanilla-jsoneditor fails to load.
Right now a chunk/import failure just logs to the console and clears the spinner, leaving the editor area empty with no retry or user-facing message. This blocks the JSON editing flow in both version-table.tsx and deps-table.tsx.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/manage/components/json-editor.tsx` around lines 31 - 34, Update the
vanilla-jsoneditor import failure handler in the JSON editor component to set a
user-facing error state when loading fails, while preserving the existing
loading-state cleanup. Render that error state in the editor area with an
actionable retry path, ensuring failures are visible to flows using JsonEditor
from version-table.tsx and deps-table.tsx.
| <div className={className} style={{ position: 'relative', minHeight: 120 }}> | ||
| {loading && ( | ||
| <div | ||
| style={{ | ||
| display: 'flex', | ||
| justifyContent: 'center', | ||
| alignItems: 'center', | ||
| padding: 24, | ||
| }} | ||
| > | ||
| <Spin tip="加载编辑器..." /> | ||
| </div> |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Make the loading indicator an actual overlay.
The spinner container is not positioned absolutely, so it occupies normal layout space above the editor instead of covering the wrapper. Add position: absolute; inset: 0; (and a suitable background/z-index) to prevent layout shift and center the loading state over the editor area.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/manage/components/json-editor.tsx` around lines 53 - 64, Update the
loading spinner container in the JSON editor wrapper to use absolute positioning
with inset 0, plus an appropriate background and z-index, so it overlays and
centers over the editor without consuming layout space. Keep the wrapper’s
existing relative positioning and loading behavior unchanged.
| const appName = Form.useWatch('name') as string; | ||
| const ignoreBuildTime = Form.useWatch('ignoreBuildTime') as string; | ||
| const [showConfirmModal, setShowConfirmModal] = useState(false); | ||
|
|
||
| const confirmTargetText = appName || appKey || String(appId); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Require the application name; do not fall back to key or ID.
If name is unset or cleared, this changes the deletion guard to appKey or appId, so the app can be deleted without typing its name. Use a non-empty application name as the sole target and disable/block deletion until it is available.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In `@src/pages/manage/components/setting-modal.tsx` around lines 17 - 21, Update
the deletion confirmation flow around confirmTargetText to use only a non-empty
appName, removing the appKey and appId fallbacks. Ensure deletion remains
disabled or blocked when appName is unset or cleared, and preserve the existing
name-matching confirmation behavior when it is available.
1 participant
变更内容摘要
1. 性能与首屏加载（二）
rsbuild.config.ts中performance.chunkSplit，隔离@ant-design/charts、vanilla-jsoneditor与hash-wasmChunk。src/components/lazy-chart.tsx动态懒加载组件。admin-metrics.tsx、realtime-metrics.tsx、version-health.tsx中的同步图表引用为AsyncLine。json-editor.tsx改为异步按需加载vanilla-jsoneditor。2. 用户体验与交互设计（五）
src/components/section-error-boundary.tsx局部 Error Boundary 降级组件。src/components/skeletons.tsx规范骨架屏（TableSkeleton/ChartSkeleton/CardSkeleton）。3. 高危操作二次确认 Guard（六）
src/components/dangerous-confirm-modal.tsx高危毁灭性操作二次确认 Guard。setting-modal.tsx应用删除时接入高危二次确认弹窗，必须精准匹配应用名称方可点击删除。4. 工程化与开发者体验（八）
package.json中增加build:analyze产物体积分析指令。bun run ci/bun run lint:fix确保类型安全与 Biome 规范通过。Need help on this PR? Tag
/codesmithwith what you need. Autofix is disabled.Summary by CodeRabbit