feat: no BasicLayout component needed in menu data
#5386
Conversation
所有菜单数据无需配置component为BasicLayout，它们将会默认使用基础布局，也可以通过meta.noBasicLayout来阻止这一行为
|
|
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
apps/web-antd/src/router/routes/core.tsOops! Something went wrong! :( ESLint: 9.17.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find module '/node_modules/@vben/eslint-config/dist/index.mjs' imported from /eslint.config.mjs WalkthroughThe pull request introduces significant changes to the routing configuration across multiple applications and modules. The primary modifications involve removing the Changes
Sequence DiagramsequenceDiagram
participant Router
participant Route
participant Layout
Router->>Route: Check route configuration
alt noBasicLayout is false or undefined
Route->>Layout: Apply BasicLayout
else noBasicLayout is true
Route->>Route: Render without BasicLayout
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
Finishing Touches
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (4)
playground/src/router/routes/modules/dashboard.ts (1)
Line range hint
15-36: Review child route paths for potential updates.The children routes use absolute paths ('/analytics', '/workspace'). Consider updating them to be relative to the new parent path:
{ name: 'Analytics', - path: '/analytics', + path: 'analytics', component: () => import('#/views/dashboard/analytics/index.vue'), meta: { affixTab: true, icon: 'lucide:area-chart', title: $t('page.dashboard.analytics'), }, }, { name: 'Workspace', - path: '/workspace', + path: 'workspace', component: () => import('#/views/dashboard/workspace/index.vue'), meta: { icon: 'carbon:workspace', title: $t('page.dashboard.workspace'), }, },This would make the routes more maintainable and ensure they're properly nested under the new '/dashboard' parent path.
packages/effects/access/src/accessible.ts (2)
33-33: Avoid using thedeleteoperator for better performance.Using the
deleteoperator can negatively impact performance in JavaScript. Consider setting thecomponentproperty toundefinedinstead.Apply this diff to refactor the code:
- delete route.component; + route.component = undefined;
30-34: Translate code comments to English for wider accessibility.The comments within this block are in Chinese. Translating them to English can help international collaborators understand the code more effectively.
apps/web-naive/src/router/routes/core.ts (1)
24-40: Well-structured implementation of the root layout.The root route implementation effectively centralizes the BasicLayout component, eliminating the need to specify it in child routes. The documentation clearly explains the purpose and importance of this route.
However, consider adding a comment about the
meta.noBasicLayoutoption mentioned in the PR objectives for cases where the basic layout should be bypassed./** * 根路由 * 使用基础布局，作为所有页面的父级容器，子级就不必配置BasicLayout。 * 此路由必须存在，且不应修改 + * 注意：如果需要绕过基础布局，请在路由的meta中设置noBasicLayout: true */
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (22)
apps/backend-mock/utils/mock-data.ts(0 hunks)apps/web-antd/src/router/routes/core.ts(2 hunks)apps/web-antd/src/router/routes/modules/dashboard.ts(0 hunks)apps/web-antd/src/router/routes/modules/demos.ts(0 hunks)apps/web-antd/src/router/routes/modules/vben.ts(2 hunks)apps/web-ele/src/router/routes/core.ts(2 hunks)apps/web-ele/src/router/routes/modules/dashboard.ts(0 hunks)apps/web-ele/src/router/routes/modules/demos.ts(0 hunks)apps/web-ele/src/router/routes/modules/vben.ts(2 hunks)apps/web-naive/src/router/routes/core.ts(2 hunks)apps/web-naive/src/router/routes/modules/dashboard.ts(0 hunks)apps/web-naive/src/router/routes/modules/demos.ts(0 hunks)apps/web-naive/src/router/routes/modules/vben.ts(2 hunks)docs/src/en/guide/essentials/route.md(0 hunks)docs/src/guide/essentials/route.md(2 hunks)packages/@core/base/typings/src/vue-router.d.ts(1 hunks)packages/effects/access/src/accessible.ts(1 hunks)playground/src/router/routes/core.ts(2 hunks)playground/src/router/routes/modules/dashboard.ts(1 hunks)playground/src/router/routes/modules/demos.ts(1 hunks)playground/src/router/routes/modules/examples.ts(0 hunks)playground/src/router/routes/modules/vben.ts(2 hunks)
💤 Files with no reviewable changes (9)
- apps/web-antd/src/router/routes/modules/dashboard.ts
- apps/backend-mock/utils/mock-data.ts
- apps/web-antd/src/router/routes/modules/demos.ts
- apps/web-naive/src/router/routes/modules/demos.ts
- apps/web-naive/src/router/routes/modules/dashboard.ts
- playground/src/router/routes/modules/examples.ts
- apps/web-ele/src/router/routes/modules/dashboard.ts
- docs/src/en/guide/essentials/route.md
- apps/web-ele/src/router/routes/modules/demos.ts
🧰 Additional context used
🪛 Biome (1.9.4)
packages/effects/access/src/accessible.ts
[error] 43-43: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
⏰ Context from checks skipped due to timeout of 90000ms (9)
- GitHub Check: Lint (windows-latest)
- GitHub Check: Test (windows-latest)
- GitHub Check: Lint (ubuntu-latest)
- GitHub Check: post-update (windows-latest)
- GitHub Check: Check (windows-latest)
- GitHub Check: post-update (ubuntu-latest)
- GitHub Check: Test (ubuntu-latest)
- GitHub Check: Analyze (javascript-typescript)
- GitHub Check: Check (ubuntu-latest)
🔇 Additional comments (18)
playground/src/router/routes/modules/dashboard.ts (2)
5-12: LGTM! Changes align with PR objectives.The removal of the explicit BasicLayout component from the route configuration aligns with the PR objectives to make it the default behavior. The meta configuration is correctly maintained without needing the noBasicLayout property since we want the default layout behavior here.
13-13: Verify path change impact and update children routes.The path change from '/' to '/dashboard' could affect existing bookmarks and deep links. Additionally, the children routes' paths should be reviewed for consistency with the new parent path.
Let's verify the path changes and their impact:
✅ Verification successful
Path change from '/' to '/dashboard' appears safe
The codebase analysis shows no critical dependencies on the root path that would be affected by this change. Router guards and navigation logic handle paths dynamically, and existing redirects are feature-specific.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for potential path conflicts and verify route consistency # Check for any hardcoded references to the old root path rg -l "path: '/'" --type ts # Check for any navigation or route guards that might depend on the old path rg "router\.(push|replace)\s*\(\s*['\"]/" --type ts rg "beforeEach|beforeResolve|afterEach" --type ts # Look for potential redirect configurations that need updating rg "redirect.*?['\"]/" --type tsLength of output: 5413
playground/src/router/routes/modules/vben.ts (3)
13-14: LGTM! Clean import statements.The new imports are necessary and properly used throughout the file.
21-21: LGTM! Logical order adjustment.The order change from 9999 to 9998 makes sense as it ensures VbenProject appears before the newly separated VbenAbout route in the menu structure.
82-91: Verify the moved route's accessibility.The VbenAbout route has been moved from VbenProject's children to the root level while maintaining the same path prefix. While this aligns with the PR's objective, we should verify that:
- The route is still accessible
- The menu item appears in the correct location
- The breadcrumb navigation works as expected
apps/web-antd/src/router/routes/modules/vben.ts (3)
11-11: ImportIFrameViewcomponent correctly.The import statement for
IFrameViewis accurate and necessary for the updated route components.
19-19: Update theorderproperty to adjust menu positioning.Changing the
orderfrom9999to9998alters the menu item's position. Ensure this reflects the intended navigation order.Please confirm that the menu items display in the desired sequence after this change.
69-78: AddVbenAboutas a top-level route for improved accessibility.Elevating
VbenAboutto a top-level route makes it more accessible and may enhance user navigation.apps/web-ele/src/router/routes/modules/vben.ts (3)
12-12: ImportIFrameViewcomponent appropriately.The import statement for
IFrameViewis correct and aligns with the changes made to the route configurations.
20-20: Modify theorderproperty to adjust the route's position.Updating the
orderfrom9999to9998changes the route's placement in the navigation hierarchy.Please verify that the updated order achieves the intended menu arrangement.
70-79: IntroduceVbenAboutas a separate top-level route.Adding
VbenAboutas a top-level route enhances its prominence within the application.apps/web-naive/src/router/routes/modules/vben.ts (2)
20-20: Verify menu ordering impact.The order value has been decreased from 9999 to 9998, which will affect the positioning of this menu item. Please ensure this change in ordering aligns with the intended menu structure.
70-79: Consider keeping route hierarchy consistent.The VbenAbout route has been moved from being a child of VbenProject to a top-level route. While this works with the new BasicLayout structure, consider if this change in hierarchy affects the user's navigation experience and menu organization.
Run this script to verify the impact:
apps/web-ele/src/router/routes/core.ts (1)
24-40: Implementation matches web-naive version.The root route implementation is consistent with the web-naive version, which is the correct approach for maintaining consistency across applications.
packages/@core/base/typings/src/vue-router.d.ts (1)
100-103: LGTM! Well-documented interface addition.The
noBasicLayoutproperty is properly typed and documented, clearly indicating its top-level-only effect.apps/web-antd/src/router/routes/core.ts (1)
24-40: LGTM! Well-structured root route implementation.The root route implementation:
- Centralizes the
BasicLayoutusage- Has clear documentation and warning
- Properly configured meta to hide from UI elements
playground/src/router/routes/modules/demos.ts (1)
3-3: LGTM! Simplified import statement.The removal of
BasicLayoutaligns with the new centralized layout approach while maintaining the route structure.docs/src/guide/essentials/route.md (1)
584-590: LGTM! Clear and comprehensive documentation.The
noBasicLayoutproperty is well-documented with:
- Clear explanation of purpose and effect
- Proper type information and default value
- Important note about top-level limitation
Description
所有菜单数据无需配置component为BasicLayout，它们将会默认使用基础布局，也可以通过meta.noBasicLayout来阻止这一行为。
close #5378
close #5329
close #5325
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
Release Notes
Routing Changes
BasicLayoutacross multiple applicationsBasicLayoutfrom various route definitionsnoBasicLayoutconfiguration for more flexible routingDocumentation Updates
Technical Improvements