perf: perf the control logic of Tab #6220
Conversation
* 每个标签页Tab使用唯一的key来控制关闭打开等逻辑 * 统一函数获取tab的key * 通过3种方式设置tab key：1、使用router query参数pageKey 2、使用路由meta参数fullPathKey设置使用fullPath或path作为key * 单个路由可以打开多个标签页 * 如果设置fullPathKey为false，则query变更不会打开新的标签（这很实用）
ming4762
requested review from
anncwb,
vince292007,
mynetfan,
jinmao88 and
a team
as code owners
|
WalkthroughThis update standardizes tab identification and management by introducing a key-based system for tabs, replacing previous path-based logic. It adds a Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant Router
participant TabbarStore
participant TabsUI
User->>Router: Navigates to a route (with or without pageKey)
Router->>TabbarStore: Calls addTab(route)
TabbarStore->>TabbarStore: getTabKey(route) // Uses pageKey, fullPath, or path
TabbarStore->>TabbarStore: Assigns key to tab
TabbarStore-->>TabsUI: Provides tabs with key property
TabsUI->>TabsUI: Renders tabs using tab.key for identification
User->>TabsUI: Interacts with tabs (activate, close, etc.)
TabsUI->>TabbarStore: Performs tab operations using tab.key
Poem
Note ⚡️ AI Code Reviews for VS Code, Cursor, WindsurfCodeRabbit now has a plugin for VS Code, Cursor and Windsurf. This brings AI code reviews directly in the code editor. Each commit is reviewed immediately, finding bugs before the PR is raised. Seamless context handoff to your AI code agent ensures that you can easily incorporate review feedback. Note ⚡️ Faster reviews with cachingCodeRabbit now supports caching for code and dependencies, helping speed up reviews. This means quicker feedback, reduced wait times, and a smoother review experience overall. Cached data is encrypted and stored securely. This feature will be automatically enabled for all accounts on May 16th. To opt out, configure ✨ Finishing Touches
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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Nitpick comments (4)
packages/stores/src/modules/tabbar.ts (1)
630-641: Guard against missingkeywhen comparing tabs
getTabKeyFromTabsilently falls back togetTabKey(tab)whentab.keyis falsy.
If a caller passes{ key: '' }, the empty string is accepted and will never equal any legitimate tab key, breaking equality checks.-function getTabKeyFromTab(tab: TabDefinition): string { - return tab.key ?? getTabKey(tab); +function getTabKeyFromTab(tab: TabDefinition): string { + return tab.key && tab.key.length > 0 ? tab.key : getTabKey(tab); }A tiny guard prevents subtle logic errors while preserving the optimisation of using the cached key when present.
packages/stores/src/modules/tabbar.test.ts (3)
8-13: Misleading test suite nameThe suite is called
'useAccessStore', yet the subject under test isuseTabbarStore.
This causes confusion when scanning test reports.-describe('useAccessStore', () => { +describe('useTabbarStore', () => {
145-170: Update test description to reflect key-based logicThe case still reads “closes multiple tabs by paths” but now targets
_bulkCloseByKeys.
Aligning the description avoids drift between intent and implementation.-it('closes multiple tabs by paths', async () => { +it('closes multiple tabs by keys', async () => {
20-32: Redundant manualkeyin first test
addTabwill auto-assign a key when none is provided, so explicitly settingkey: '/home'in the fixture is unnecessary and couples the test to implementation details.While harmless, removing it keeps the test lean:
- meta: {}, - key: '/home', + meta: {},
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (10)
docs/src/guide/essentials/route.md(3 hunks)packages/@core/base/typings/src/tabs.ts(1 hunks)packages/@core/base/typings/src/vue-router.d.ts(1 hunks)packages/@core/ui-kit/tabs-ui/src/components/tabs-chrome/tabs.vue(1 hunks)packages/@core/ui-kit/tabs-ui/src/components/tabs/tabs.vue(1 hunks)packages/effects/layouts/src/basic/content/content.vue(3 hunks)packages/effects/layouts/src/basic/tabbar/tabbar.vue(1 hunks)packages/effects/layouts/src/basic/tabbar/use-tabbar.ts(4 hunks)packages/stores/src/modules/tabbar.test.ts(8 hunks)packages/stores/src/modules/tabbar.ts(14 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/effects/layouts/src/basic/tabbar/use-tabbar.ts (1)
packages/stores/src/modules/tabbar.ts (1)
getTabKey(652-652)
packages/stores/src/modules/tabbar.test.ts (1)
packages/stores/src/modules/tabbar.ts (1)
useTabbarStore(55-553)
packages/stores/src/modules/tabbar.ts (1)
packages/@core/base/typings/src/tabs.ts (1)
TabDefinition(3-8)
⏰ Context from checks skipped due to timeout of 90000ms (7)
- GitHub Check: Check (windows-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
🔇 Additional comments (15)
packages/@core/base/typings/src/vue-router.d.ts (1)
46-49: Well-structured addition of the fullPathKey propertyThe addition of the
fullPathKeyboolean property to theRouteMetainterface is well-documented with clear comments. This property enables controlling whether a route's full path should be used as the key for tab identification, which is essential for the new key-based tab management system.packages/effects/layouts/src/basic/tabbar/tabbar.vue (1)
33-33: Updated to use key-based tab retrieval methodThe change from path-based to key-based tab retrieval (
getTabByKeyinstead of what was likelygetTabByPath) aligns with the PR's objective of using unique keys for tab operations. This makes tab management more consistent and flexible.packages/@core/ui-kit/tabs-ui/src/components/tabs/tabs.vue (2)
50-51: Properly destructures the key property from tab objectThe updated code now correctly destructures the
keyproperty along with other properties from the tab object, supporting the new key-based tab identification system.
57-57: Correctly uses the tab key for identificationThe tab's
keyproperty is now directly assigned to the returned object instead of deriving it from paths, ensuring consistent tab identification throughout the application.packages/@core/base/typings/src/tabs.ts (1)
3-8: Clear definition of the TabDefinition interface with key propertyThe change from a type alias to an interface that extends
RouteLocationNormalizedis appropriate. The addition of thekeyproperty is well-documented and provides the foundation for the key-based tab management system described in the PR objectives.This change enables:
- Unique identification of tabs using keys instead of paths
- More flexible tab management with multiple tabs for a single route
- Consistent tab operations (opening/closing) based on keys
packages/@core/ui-kit/tabs-ui/src/components/tabs-chrome/tabs.vue (1)
43-43: Updated tab property extraction to support key-based identificationThe code now explicitly destructures and uses the
keyproperty from each tab, which aligns with the PR objective of using unique keys for tab management.Also applies to: 50-50
packages/effects/layouts/src/basic/content/content.vue (2)
12-12: Added import forgetTabKeyfunctionThe import now includes the
getTabKeyfunction from the tabbar store, which will be used for consistent tab key generation.
118-118: Standardized component keys usinggetTabKeyfunctionThe component keys are now consistently derived using the
getTabKey(route)function instead of directly usingroute.fullPath. This ensures that the component caching in<KeepAlive>works correctly with the new tab key system.Also applies to: 124-124, 137-137, 143-143
packages/effects/layouts/src/basic/tabbar/use-tabbar.ts (4)
25-25: Added import forgetTabKeyfunctionThe import statement now includes the
getTabKeyfunction from the tabbar store, supporting the unified approach to tab key management.
47-52: Updated tab key computation to usegetTabKeyThe computed property
currentActivenow correctly uses thegetTabKey(route)function to determine the current active tab's key.
79-81: Enhanced tab click handler to support key-based navigationThe
handleClickfunction now retrieves the tab object by key first, then navigates to the appropriate path. This is crucial for the key-based tab system to work correctly, especially when a single route can open multiple tabs.
107-107: Updated route watcher to trackfullPathinstead ofpathThe watcher now tracks
route.fullPathwhich provides more accurate tab management, especially when dealing with query parameters.docs/src/guide/essentials/route.md (3)
343-345: Added documentation for the newfullPathKeypropertyThe
RouteMetainterface documentation now includes thefullPathKeyproperty, which controls whether the route's full path is used as the tab key.
509-515: Added detailed documentation for thefullPathKeypropertyThis section provides clear documentation for the
fullPathKeyproperty, explaining its type, default value, and purpose. This information is essential for developers to understand how tab keys are generated and controlled.
617-644: Added comprehensive documentation for Tab and Route ControlThis new section excellently documents the three ways to set tab keys (query parameter
pageKey, fullPath, or path) and their priority order. It includes a practical example of using thepageKeyquery parameter to create unique tabs, which directly addresses the PR objective of allowing a single route to open multiple tabs.
* 解决AI提示的警告
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/stores/src/modules/tabbar.ts (1)
320-329: Consider consolidating tab update logicBoth
pinTabandunpinTabhave repetitive code where they:
- Find a tab by key
- Check if it exists
- Update its properties
- Splice it back into the array
This pattern appears in multiple places. Consider creating a helper function to reduce duplication.
+ /** + * Helper to update a tab in place + * @param tab The tab to update + * @param updates Function that applies updates to the tab + * @returns Whether the tab was found and updated + */ + function updateTab(tab: TabDefinition, updates: (oldTab: TabDefinition) => void): boolean { + const index = this.tabs.findIndex((item) => equalTab(item, tab)); + if (index === -1) { + return false; + } + const oldTab = this.tabs[index]; + updates(tab); + tab.meta.title = oldTab?.meta?.title as string; + this.tabs.splice(index, 1, tab); + return true; + }Also applies to: 463-472
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
Cache: Disabled due to data retention organization setting
Knowledge Base: Disabled due to data retention organization setting
📒 Files selected for processing (1)
packages/stores/src/modules/tabbar.ts(13 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/stores/src/modules/tabbar.ts (1)
packages/@core/base/typings/src/tabs.ts (1)
TabDefinition(3-8)
⏰ Context from checks skipped due to timeout of 90000ms (8)
- GitHub Check: Check (windows-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: Check (ubuntu-latest)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
🔇 Additional comments (9)
packages/stores/src/modules/tabbar.ts (9)
60-66: Good fix for the async functionYou've properly added
awaitbeforeupdateCacheTabs(), resolving the issue where the async function wasn't awaiting the promise. This prevents potential race conditions.
109-113: Key assignment handled properlyGood implementation of key assignment for tabs. The function now ensures that every tab has a key property by falling back to
getTabKeywhen needed.
165-169: Fixed the stale reference issueYou've correctly addressed the previous issue where
addTabwas returning a stale clone. Now the method properly returns the actual tab instance that's stored in the tabs array.
206-224: Consistent key-based approach for closeOtherTabsThe logic for closing other tabs has been properly rewritten to use keys instead of paths, making it consistent with the overall approach.
607-629: Robust key extraction implementationThe
getTabKeyfunction addresses the earlier issues by:
- Handling array-type query parameters
- Using a fallback path when fullPath is missing
- Safely decoding URIs with proper error handling
This is a well-implemented fix for the potential issues raised in previous reviews.
636-638: Clean abstraction of key extractionThis helper function provides a good abstraction for getting a key from a tab, with proper fallback to
getTabKeyif no explicit key exists.
645-647: Simple and effective tab comparisonThe
equalTabfunction provides a clean way to compare tabs by their keys, which is more robust than comparing by paths or other properties.
654-654: Ensure consistent key generationThe
routeToTabfunction now properly assigns a key to the returned tab object, ensuring consistency with the new key-based approach.
658-658: Good export of the utility functionExporting
getTabKeymakes this utility available to other components that need to generate consistent keys, supporting the new key-based tab management system.
Summary by CodeRabbit
New Features
fullPathKeyoption in route meta to control tab key behavior.Documentation
Refactor
Tests