-
Notifications
You must be signed in to change notification settings - Fork 8.4k
feat: auto set component name for keep-alive #5690
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
|
WalkthroughThe changes enhance the Changes
Sequence Diagram(s)sequenceDiagram
participant GR as generateRoutes
participant UT as Utility Functions
participant AW as Async Wrapper
participant OC as Original Component
GR->>GR: Process each route
GR->>UT: Validate (isFunction(component) & isString(name))
UT-->>GR: Return validation results
alt Route has `meta.keepAlive` and valid properties
GR->>AW: Wrap component with async wrapper (defineComponent)
AW->>OC: Asynchronously import original component
AW-->>GR: Return wrapped component with caching support
else
GR-->>GR: Retain original component without wrapping
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
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. 🔧 ESLint
Scope: all 5 workspace projects 📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 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 (6)
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/effects/access/src/accessible.ts (1)
93-111:❓ Verification inconclusive
Watch for naming collisions and consider caching.
Naming collisions: When wrapping the component under
route.name, if multiple routes share the same name, you might encounter collisions in<keep-alive>usage. Ensure route names are unique if keep-alive caching is expected to differentiate components.Optional caching: For performance gains, consider storing the resolved component to avoid subsequent re-fetching on repeated navigations. For example:
route.component = async () => { + if (originalComponent._cached) { + return originalComponent._cached; + } const { default: component } = await originalComponent(); const wrapped = defineComponent({ name: route.name as string, setup(props, { attrs, slots }) { return () => h(component, { ...props, ...attrs }, slots); }, }); + originalComponent._cached = wrapped; return wrapped; };Below is a script to scan for duplicate route names across the codebase, helping ensure keep-alive logic behaves as intended:
🏁 Script executed:
#!/bin/bash # Description: Find potential duplicate route names in .ts or .js files. fd -e ts -e js --exec rg -A 2 "name:\s*['\"]" {}Length of output: 39393
Action Required: Verify Route Name Uniqueness and Consider Caching Optimization
Please ensure that the keep-alive logic works as intended:
Naming Collisions:
The wrapping logic usesroute.namewhen defining the component. Our scan indicates that many route definitions (e.g., names like "Home", "About", "Affix") appear multiple times in the codebase. Duplicate names may cause unwanted collisions in<keep-alive>. Please manually verify that routes intended for caching have unique names.Optional Caching:
To avoid repeated fetching and improve performance, consider caching the wrapped component after its first resolution. For example:route.component = async () => { + if (originalComponent._cached) { + return originalComponent._cached; + } const { default: component } = await originalComponent(); const wrapped = defineComponent({ name: route.name as string, setup(props, { attrs, slots }) { return () => h(component, { ...props, ...attrs }, slots); }, }); + originalComponent._cached = wrapped; return wrapped; };Please review the potential duplicate route names (especially in areas related to keep-alive usage) and confirm that they are either intentional or need to be adjusted.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/effects/access/src/accessible.ts(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Test (windows-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Check (ubuntu-latest)
🔇 Additional comments (4)
packages/effects/access/src/accessible.ts (4)
1-1: Imports look correct.
No issues identified with importingRouteComponentfromvue-router.
9-9: Correct usage ofdefineComponentandhfrom Vue.
These imports are relevant for dynamically wrapping components in the subsequent code.
16-17: Utility imports are appropriate.
The usage ofisFunctionandisStringto safeguard route validation logic is a good practice.
90-90: Doc comment alignment looks fine.
No issues found in the updated commentary for clarifying the behavior of lazy-loaded components.
* fix: auto set component name for keep-alive * fix: type define
Description
在开启keep-alive的情况下:
自动调整路由的component名称与路由同步,解决不同路由使用同一个组件作为页面内容时的缓存问题;
合并此PR之后,无需因为keep-alive的原因为页面组件显式设置name
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