fix: 修复路由跳转的页面出现空白 #6727
fix: 修复路由跳转的页面出现空白 #6727
Conversation
Lynnettee-web
requested review from
anncwb,
vince292007,
mynetfan,
jinmao88 and
a team
as code owners
|
WalkthroughAdds a dynamic key binding Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor User
participant Router as Vue Router
participant App as App.vue
participant RV as RouterView (key=$route.fullPath)
participant Page as Routed Component
User->>Router: navigate to /path?query=...
Router-->>App: update $route (fullPath)
App->>RV: render with new :key
note over RV: key changed → triggers remount
RV--x Page: unmount previous instance
RV->>Page: mount new instance
Page-->>User: fresh lifecycle & state
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Suggested reviewers
Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Poem
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
✨ Finishing Touches🧪 Generate unit tests
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: 0
🧹 Nitpick comments (4)
apps/web-antd/src/app.vue (1)
36-36: Consider KeepAlive cache growth with fullPath keysKeying the root RouterView by
$route.fullPath(apps/web-antd/src/app.vue:36) forces remount on any path/query/hash change, but since this RouterView is wrapped in<KeepAlive>in packages/effects/layouts/src/basic/content/content.vue (lines 109–112, 128–131), each unique fullPath yields a separate cached instance, risking unbounded memory growth if queries vary. If acceptable, merge as is; otherwise, use a stable computed key (e.g. based onroute.pathorroute.name) viauseRoute()+computedto limit remounts.apps/web-naive/src/app.vue (1)
52-52: Validate remount behavior and optionally introduce computed viewKey
No<keep-alive>wrappers exist in theappsdirectory, so this change will always remount on query/hash changes—confirm this is acceptable for your routing flows. Optionally refactor to:<script lang="ts" setup> import { computed } from 'vue'; +import { useRoute } from 'vue-router'; const route = useRoute(); const viewKey = computed(() => route.fullPath); </script> -<RouterView :key="$route.fullPath" /> +<RouterView :key="viewKey" />apps/web-ele/src/app.vue (2)
15-16: Remove dead commented RouterViewThe commented adds noise. Please delete it.
- <!-- <RouterView /> --> <RouterView :key="$route.fullPath" />
16-16: Confirm fullPath keying won’t inflate KeepAlive cache or hurt perfSame considerations as other apps: guaranteed remount vs. potential cache growth and perf cost on query/hash changes. If needed, switch to a centralized computed key to tune behavior later:
<script lang="ts" setup> +import { computed } from 'vue'; +import { useRoute } from 'vue-router'; @@ +const route = useRoute(); +const viewKey = computed(() => route.fullPath); </script> @@ - <RouterView :key="$route.fullPath" /> + <RouterView :key="viewKey" />Regression checklist to run locally:
- Navigate between same-path different-query routes repeatedly; watch memory and component instance counts in Vue devtools.
- Toggle hash within a route; ensure remount is intentional.
- Test tab caching, tab close/restore, and “Open in new window” (learned to prefer fullPath for navigation).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
apps/web-antd/src/app.vue(1 hunks)apps/web-ele/src/app.vue(1 hunks)apps/web-naive/src/app.vue(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📓 Common learnings
Learnt from: dingdayu
PR: vbenjs/vue-vben-admin#5500
File: packages/stores/src/modules/tabbar.ts:557-559
Timestamp: 2025-02-08T07:05:28.825Z
Learning: In Vue Vben Admin, while tab identification uses `path` to group tabs with same path but different queries, router navigation operations (router.push) and new window operations should use `fullPath` to preserve the complete URL including query parameters. This applies specifically to:
1. Router navigation in tab maximize/restore feature
2. Opening tabs in new window via openTabInNewWindow
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: post-update (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
Description
Type of change
Please delete options that are not relevant.
pnpm-lock.yamlunless you introduce a new test example.Checklist
pnpm run docs:devcommand.pnpm test.feat:,fix:,perf:,docs:, orchore:.Summary by CodeRabbit