Skip to content
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

docs: add route config desc & update en docs #4857

Merged
merged 1 commit into from
Nov 11, 2024

Conversation

ArthurDarkstone
Copy link
Contributor

@ArthurDarkstone ArthurDarkstone commented Nov 10, 2024

Description

Type of change

Please delete options that are not relevant.

  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update
  • Please, don't make changes to pnpm-lock.yaml unless you introduce a new test example.

Checklist

ℹ️ Check all checkboxes - this will indicate that you have done everything in accordance with the rules in CONTRIBUTING.

  • If you introduce new functionality, document it. You can run documentation with pnpm run docs:dev command.
  • Run the tests with pnpm test.
  • Changes in changelog are generated from PR name. Please, make sure that it explains your changes in an understandable manner. Please, prefix changeset messages with feat:, fix:, perf:, docs:, or chore:.
  • My code follows the style guidelines of this project
  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes
  • Any dependent changes have been merged and published in downstream modules

Summary by CodeRabbit

  • New Features

    • Enhanced routing documentation for better clarity on route categories and access requirements.
    • Introduction of a new section on "Core Routes" in the routing guide.
  • Documentation

    • Updated and reorganized routing documentation to improve usability and understanding of routing concepts.
    • Added detailed descriptions and examples for core, static, and dynamic routes.
  • Chores

    • Improved comments in routing configuration files to clarify the purpose of various route categories.

Copy link

changeset-bot bot commented Nov 10, 2024

⚠️ No Changeset found

Latest commit: c77dc27

Merging this PR will not cause a version bump for any packages. If these changes should not result in a new version, you're good to go. If these changes should result in a version bump, you need to add a changeset.

This PR includes no changesets

When changesets are added to this PR, you'll see the packages that this PR includes changesets for and the associated semver types

Click here to learn what changesets are, and how to add one.

Click here if you're a maintainer who wants to add a changeset to this PR

Copy link
Contributor

coderabbitai bot commented Nov 10, 2024

Walkthrough

The pull request introduces several modifications to the routing configurations across multiple index.ts files and documentation. Key changes include the initialization of staticRoutes, externalRoutes, and accessRoutes as empty arrays, and updates to comments clarifying the purpose of various route categories. Documentation has been enhanced to provide clearer guidance on routing types, including core routes, and their permission requirements. Overall, the changes focus on improving the clarity and structure of routing logic and documentation without altering the underlying functionality.

Changes

File Path Change Summary
apps/web-antd/src/router/routes/index.ts Updated comments for route categories, initialized staticRoutes, externalRoutes, and accessRoutes as empty arrays.
apps/web-ele/src/router/routes/index.ts Updated comments for route categories, initialized staticRoutes, externalRoutes, and accessRoutes as empty arrays.
apps/web-naive/src/router/routes/index.ts Updated comments for route categories, removed old comments, initialized staticRoutes, externalRoutes, and accessRoutes as empty arrays.
docs/src/en/guide/essentials/route.md Changed title, expanded sections on route types, clarified descriptions, and standardized section headers.
docs/src/guide/essentials/route.md Added new sections for core routes and routing types, improved clarity on permission control, and updated route configuration details.
playground/src/router/routes/index.ts Updated comment for routes, initialized staticRoutes and externalRoutes as empty arrays, clarified accessRoutes.

Possibly related PRs

Suggested labels

documentation

Suggested reviewers

  • vince292007
  • anncwb

Poem

🐇 In the land of routes, so bright and clear,
Comments updated, no need to fear.
Static and external, all in a row,
With access routes ready, let clarity flow!
Hop along, dear coder, to the paths anew,
With permissions in check, let’s code and pursue! 🌟


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?

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@ArthurDarkstone
Copy link
Contributor Author

maybe help with #4854

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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

🧹 Outside diff range and nitpick comments (15)
apps/web-antd/src/router/routes/index.ts (2)

Line range hint 8-9: Translate Chinese comment to English for consistency.

Consider translating the comment "有需要可以自行打开注释,并创建文件夹" to English to maintain documentation consistency.

-// 有需要可以自行打开注释,并创建文件夹
+// You can uncomment and create folders if needed

24-25: Improve comment formatting for better readability.

The multi-line comment could be formatted more clearly.

-/** 路由列表,由基本路由、外部路由和404兜底路由组成
- *  无需走权限验证(会一直显示在菜单中) */
+/**
+ * Route list composed of core routes, external routes, and 404 fallback route.
+ * No permission validation required (always displayed in the menu).
+ */
apps/web-ele/src/router/routes/index.ts (1)

Line range hint 8-8: Consider translating the Chinese comment for consistency.

For better maintainability and consistency with English documentation, consider translating the comment "有需要可以自行打开注释,并创建文件夹" to English.

-// 有需要可以自行打开注释,并创建文件夹
+// Uncomment and create folders if needed
apps/web-naive/src/router/routes/index.ts (2)

24-25: Improve comment readability and structure.

The comment's meaning is correct but could be more readable. Consider restructuring it as follows:

-/** 路由列表,由基本路由、外部路由和404兜底路由组成
- *  无需走权限验证(会一直显示在菜单中) */
+/** 路由列表:包含基本路由、外部路由和404兜底路由。
+ * 特点:
+ * - 无需权限验证
+ * - 始终显示在菜单中
+ */

Line range hint 1-37: Well-structured route organization.

The route management approach demonstrates good architectural decisions:

  • Clear separation of concerns between different route types
  • Modular organization using dynamic imports
  • Proper handling of fallback routes
  • Clear permission boundaries between verified and unverified routes

Consider documenting these routing patterns in the project's architecture documentation to establish them as team conventions.

playground/src/router/routes/index.ts (2)

24-25: Enhance security implications in route documentation.

While the comment clarifies that these routes bypass permission validation, it would be beneficial to explicitly document:

  1. Security considerations for routes without permission checks
  2. Guidelines for determining which routes should skip validation

Consider updating the comment to:

-/** 路由列表,由基本路由、外部路由和404兜底路由组成
- *  无需走权限验证(会一直显示在菜单中) */
+/** 路由列表,由基本路由、外部路由和404兜底路由组成
+ *  无需走权限验证(会一直显示在菜单中)
+ *  注意: 仅将必要的公共路由添加到此列表中,因为这些路由将绕过权限检查 */

35-37: Consider enhancing type safety for exported routes.

The documentation clearly explains the purpose of accessRoutes. However, consider improving type safety by:

  1. Creating a custom type for routes that require permission validation
  2. Adding runtime validation for route configurations

Example implementation:

/** Type for routes that require permission validation */
type AccessRoute = RouteRecordRaw & {
  meta: {
    requiresAuth: true;
    permissions?: string[];
  };
};

/** 有权限校验的路由列表,包含动态路由和静态路由 */
const accessRoutes: AccessRoute[] = [...dynamicRoutes, ...staticRoutes];
docs/src/guide/essentials/route.md (2)

15-23: Well-structured core routes documentation with good architectural guidance!

The documentation effectively explains core routes and their configuration. The tip about keeping business routes separate from core routes is particularly valuable for maintaining a clean architecture.

This separation of concerns between core routes and business routes helps maintain a cleaner codebase and makes the application more maintainable and scalable.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~17-~17: 数词与名词之间一般应存在量词,可能缺少量词。
Context: ...s)。 ### 核心路由 核心路由是框架内置的路由,包含了根路由、登录路由、404路由等,核心路由的配置在应用下 src/router/routes/core ...

(wa5)


27-33: Documentation is clear, consider adding a configuration example.

The explanation of static routes and permission control is accurate and helpful. While the content is good, consider adding a simple configuration example to illustrate the authority field usage in the meta attribute.

Consider adding a brief example like:

{
  path: '/admin',
  meta: {
    authority: ['admin', 'super-user'],  // Example permission control
  }
}
docs/src/en/guide/essentials/route.md (6)

9-9: Enhance the machine translation notice.

The current notice is vague. Consider specifying which parts are machine-translated and whether they have been reviewed for accuracy.

-This page is translated by machine translation and may not be very accurate.
+Parts of this documentation have been machine translated. While we strive for accuracy, some sections may require further review. Please report any unclear or incorrect translations.

17-19: Improve the structure of route types explanation.

The explanation of route types could be more structured for better readability.

-Routes are divided into core routes, static routes, and dynamic routes. Core routes are built-in routes of the framework, including root routes, login routes, 404 routes, etc.; static routes are routes that are determined when the project starts; dynamic routes are generally generated dynamically based on the user's permissions after the user logs in.
+Routes are divided into three types:
+1. Core Routes: Built-in framework routes (root routes, login routes, 404 routes, etc.)
+2. Static Routes: Routes determined at project startup
+3. Dynamic Routes: Routes generated based on user permissions after login

Line range hint 64-107: Enhance code examples with explanatory comments.

The example would be more educational with inline comments explaining key configurations.

 const routes: RouteRecordRaw[] = [
   {
     component: BasicLayout,
     meta: {
+      // Display a dot badge with destructive (red) color
       badgeType: 'dot',
       badgeVariants: 'destructive',
+      // Use custom logo as menu icon
       icon: VBEN_LOGO_URL,
+      // Control menu item order (higher numbers appear later)
       order: 9999,
+      // Translated menu title
       title: $t('page.vben.title'),
     },
     name: 'VbenProject',
     path: '/vben-admin',
+    // Default child route when accessing /vben-admin
     redirect: '/vben-admin/about',
     children: [
       // ... rest of the code

291-291: Enhance verification section with debugging tips.

The verification section could be more helpful with common troubleshooting steps.

At this point, the page has been added. Visit `http://localhost:5555/home/index` to see the corresponding page.

+If the page doesn't appear as expected:
+1. Check the browser console for any route-related errors
+2. Verify that the route is registered correctly in the Vue Router debug panel
+3. Ensure all required meta properties are properly configured
+4. Check that the component is being loaded (look for network requests)

586-591: Enhance query property documentation with examples.

The query property documentation would be more helpful with practical examples.

### query

- Type: `Recordable`
- Default: `{}`

Used to configure the menu parameters of the page, which will be passed to the page in the menu.

+Example usage:
+```ts
+{
+  meta: {
+    query: {
+      tab: 'overview',
+      filter: 'active'
+    }
+  }
+}
+```
+
+This will append `?tab=overview&filter=active` to the route URL when accessed from the menu.

Line range hint 595-607: Expand the Route Refresh section with use cases and considerations.

The refresh functionality documentation could be more comprehensive.

The route refresh method is as follows:

+Common use cases for route refresh:
+- Reloading data after external changes
+- Resetting component state
+- Forcing re-evaluation of route guards
+
+Note: Refreshing will re-render the entire route component tree and may clear component state.
+
```vue
<script setup lang="ts">
import { useRefresh } from '@vben/hooks';

const { refresh } = useRefresh();

// Refresh the current route
refresh();
</script>
🧰 Tools
🪛 LanguageTool

[style] ~449-~449: You have already used ‘Generally’ in other nearby sentences. Consider replacing or removing it to add variety to your writing.
Context: ...e, which will be displayed in the menu. Generally used with an icon library, if it is an ...

(REP_SENT_START_ADVERBLY)

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

📥 Commits

Reviewing files that changed from the base of the PR and between 94efcec and c77dc27.

📒 Files selected for processing (6)
  • apps/web-antd/src/router/routes/index.ts (2 hunks)
  • apps/web-ele/src/router/routes/index.ts (2 hunks)
  • apps/web-naive/src/router/routes/index.ts (2 hunks)
  • docs/src/en/guide/essentials/route.md (11 hunks)
  • docs/src/guide/essentials/route.md (1 hunks)
  • playground/src/router/routes/index.ts (2 hunks)
🧰 Additional context used
🪛 LanguageTool
docs/src/en/guide/essentials/route.md

[style] ~449-~449: You have already used ‘Generally’ in other nearby sentences. Consider replacing or removing it to add variety to your writing.
Context: ...e, which will be displayed in the menu. Generally used with an icon library, if it is an ...

(REP_SENT_START_ADVERBLY)

docs/src/guide/essentials/route.md

[uncategorized] ~11-~11: 数词与名词之间一般应存在量词,可能缺少量词。
Context: ...核心路由、静态路由和动态路由,核心路由是框架内置的路由,包含了根路由、登录路由、404路由等;静态路由是在项目启动时就已经确定的路由;动态路由一般是在用户登录后,根据用...

(wa5)


[uncategorized] ~17-~17: 数词与名词之间一般应存在量词,可能缺少量词。
Context: ...s)。 ### 核心路由 核心路由是框架内置的路由,包含了根路由、登录路由、404路由等,核心路由的配置在应用下 src/router/routes/core ...

(wa5)

🔇 Additional comments (7)
apps/web-antd/src/router/routes/index.ts (1)

35-37: LGTM! Clear documentation and correct implementation.

The comment clearly explains the purpose of accessRoutes, and the implementation correctly combines dynamic and static routes for permission validation.

apps/web-ele/src/router/routes/index.ts (3)

24-25: LGTM! Clear documentation of route composition and permissions.

The comment effectively explains that these routes don't require permission verification and will always be displayed in the menu.


35-37: LGTM! Well-structured route exports with clear documentation.

The combination of dynamic and static routes into accessRoutes is logical, and the comment clearly explains its purpose. The exports are well-organized and properly documented.


Line range hint 8-13: Verify the intentional removal of dynamic route imports.

The initialization of staticRoutes and externalRoutes has been changed from dynamic imports to empty arrays. While the commented code suggests this is optional ("有需要可以自行打开注释"), this change could affect the application's routing behavior.

Let's check if this pattern is consistent across other router configurations:

Also applies to: 22-23

✅ Verification successful

Empty route arrays and commented imports are consistent across all web apps

The search results confirm this is an intentional pattern consistently implemented across all three web applications (web-ele, web-naive, and web-antd). Each app:

  • Initializes staticRoutes and externalRoutes as empty arrays
  • Contains the same commented-out dynamic imports with identical comments
  • Maintains the same structure for route configuration

This consistency across the codebase validates that the changes are intentional and part of a standardized approach, not a potential issue.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for similar route initialization patterns in other router files
# Expected: Similar empty array initialization or commented imports

# Search for route initialization patterns
rg -A 5 "const (static|external)Routes.*=.*\[\]" "apps/"

# Search for commented route file imports
rg -A 5 "const.*RouteFiles.*=.*import\.meta\.glob.*eager.*" "apps/"

Length of output: 3397

apps/web-naive/src/router/routes/index.ts (1)

35-37: Verify the intention of empty staticRoutes array.

The code shows commented imports for static routes but initializes staticRoutes as an empty array. Please confirm if this is intentional.

Let's check if there are any static route files in the project:

✅ Verification successful

Based on the search results, I can now generate the final response since I have sufficient context to verify the intention of the empty staticRoutes array.

Empty staticRoutes array is intentional - part of the framework's design

The empty staticRoutes array is intentional as evidenced by:

  • All web applications (web-naive, web-antd, web-ele) consistently use this pattern
  • Documentation (docs/src/en/guide/essentials/route.md) explains that static routes are optional and meant for routes determined at project start
  • The commented import staticRouteFiles is left as a template for developers to uncomment when they need to add static routes
🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check for the existence of static route files
# Expected: No static route files if the empty array is intentional

# Look for any static route files
fd -t f . -p "*/router/routes/static/**/*.ts"

# Check for any TODOs or comments about static routes
rg -i "static.*route" -g "!*.{lock,json}" -A 3

Length of output: 9261

playground/src/router/routes/index.ts (1)

22-23: Verify the intentional initialization of empty route arrays.

The staticRoutes and externalRoutes are initialized as empty arrays, while the previous implementation using mergeRouteModules is commented out. This could potentially impact the routing functionality if these routes are expected to be populated.

✅ Verification successful

The search results reveal that this is a consistent pattern across multiple applications in the codebase (web-naive, web-ele, web-antd, and playground). All of them have identical changes where:

  1. The route loading code is commented out
  2. Empty arrays are used instead
  3. These empty arrays are still used in route composition

Let me verify if there are any dynamic routes being loaded to understand if this is part of a broader simplification:


Empty route arrays are intentional - dynamic routes are the primary source

The verification reveals this is an intentional architectural decision across all applications (playground, web-naive, web-ele, web-antd):

  • Dynamic routes are actively loaded using mergeRouteModules(dynamicRouteFiles)
  • Static and external routes are consistently initialized as empty arrays
  • Route files exist under modules/ directories (dashboard, demos, vben) which are loaded as dynamic routes

The empty arrays for staticRoutes and externalRoutes represent a deliberate simplification of the routing system to use dynamic routes as the primary source, while maintaining the flexibility to add static/external routes in the future.

🏁 Scripts executed

The following scripts were executed for the analysis:

Script:

#!/bin/bash
# Description: Check if there are any static or external route files that should be loaded
# Expected: No matching files if empty arrays are intentional

echo "Checking for static route files..."
fd -e ts . "playground/src/router/static"

echo "Checking for external route files..."
fd -e ts . "playground/src/router/external"

# Check if these routes are used elsewhere
echo "Checking route usage..."
rg "staticRoutes|externalRoutes" --type ts

Length of output: 2865


Script:

#!/bin/bash
# Check for dynamic route loading implementation
echo "Checking dynamic routes implementation..."
rg "dynamicRoutes.*=" -A 5 --type ts

# Check if there are any route files in the codebase
echo "Checking for route files..."
fd -e ts -e vue . | rg -i "route"

Length of output: 4440

docs/src/guide/essentials/route.md (1)

11-14: Documentation structure and content look good!

The explanation of route types and permission control is clear and comprehensive. The distinction between core, static, and dynamic routes is well-defined, and the permission control mechanism is accurately described.

🧰 Tools
🪛 LanguageTool

[uncategorized] ~11-~11: 数词与名词之间一般应存在量词,可能缺少量词。
Context: ...核心路由、静态路由和动态路由,核心路由是框架内置的路由,包含了根路由、登录路由、404路由等;静态路由是在项目启动时就已经确定的路由;动态路由一般是在用户登录后,根据用...

(wa5)

@vince292007 vince292007 merged commit ea8af98 into vbenjs:main Nov 11, 2024
14 checks passed
@vince292007 vince292007 added the documentation Improvements or additions to documentation label Nov 11, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants