-
Notifications
You must be signed in to change notification settings - Fork 11
fix(): route path array #4778
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
base: v3
Are you sure you want to change the base?
fix(): route path array #4778
Conversation
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough在 runtime 内部路由匹配实现中新增并导出 Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes ✨ 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. 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. CodeRabbit Commands (Invoked using PR/Issue comments)Type Other keywords and placeholders
CodeRabbit Configuration File (
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3 #4778 +/- ##
=======================================
Coverage 95.22% 95.22%
=======================================
Files 209 209
Lines 9166 9175 +9
Branches 1764 1766 +2
=======================================
+ Hits 8728 8737 +9
Misses 324 324
Partials 114 114
🚀 New features to boost your workflow:
|
next-core
|
Project |
next-core
|
Branch Review |
steve/v3-route-path-array
|
Run status |
|
Run duration | 00m 24s |
Commit |
|
Committer | Shenwei Wang |
View all properties for this run ↗︎ |
Test results | |
---|---|
|
0
|
|
0
|
|
0
|
|
0
|
|
17
|
View all changes introduced in this branch ↗︎ |
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: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
packages/runtime/src/internal/matchRoutes.spec.ts (1)
6-28
: 修复断言用法:.rejects 需要 Promise 而非函数当前传入
() => matchRoutes(...)
会导致断言失效;应直接传入 Promise。另建议在用例结束后恢复console.error
的 mock。- test("handle path not string", async () => { - await expect(() => - matchRoutes( + test("handle path not string", async () => { + await expect( + matchRoutes( [{}] as any, { app: { homepage: "/x", }, location: { pathname: "/x/y", }, } as any - ) - ).rejects.toMatchInlineSnapshot( + ) + ).rejects.toMatchInlineSnapshot( `[Error: Invalid route with invalid type of path: undefined]` ); expect(consoleError).toHaveBeenCalledWith( "Invalid route with invalid path:", {} ); });在文件末尾或合适位置补充:
afterAll(() => consoleError.mockRestore());
🧹 Nitpick comments (3)
packages/runtime/src/internal/matchRoutes.ts (2)
7-7
: 常量定义可微调为字面量类型并考虑集中复用建议加上
as const
,并视情况将该前缀常量集中到共享常量处,避免散落各处定义。-const HOMEPAGE_PREFIX = "${APP.homepage}"; +const HOMEPAGE_PREFIX = "${APP.homepage}" as const;
61-75
: 多路径展开建议修剪空白并消除双斜杠
- 当前
split(",")
后未trim
,若配置含空格会导致匹配失败。- 直接字符串拼接可能在
homepage
末尾含/
或子路径不含前导/
时产生//
。建议:
- if ( + if ( restPath.startsWith("[") && restPath.endsWith("]") && restPath.includes(",") ) { - const paths = restPath.slice(1, -1).split(","); - return paths.map((p) => `${homepage}${p}`); + const paths = restPath.slice(1, -1).split(","); + return paths.map((p) => joinHome(homepage, p)); } - return `${homepage}${restPath}`; + return joinHome(homepage, restPath);在文件内新增一个安全拼接工具(变更范围外补充):
function joinHome(homepage: string, p: string): string { const home = homepage.endsWith("/") ? homepage.slice(0, -1) : homepage; const part = p.trim(); if (part === "") return home; return `${home}${part.startsWith("/") ? "" : "/"}${part}`; }packages/runtime/src/internal/matchRoutes.spec.ts (1)
30-58
: 补充边界测试:空白与尾斜杠、三分支以上为避免回归,建议新增:
${APP.homepage}[, /b]
(含空格)在homepage="/home"
与homepage="/home/"
的等价匹配。- 三分支示例:
${APP.homepage}[, /b, /c/d]
。示例用例片段:
test("trim spaces and handle trailing slash", () => { const route: RouteConf = { path: "${APP.homepage}[, /b]", exact: true, bricks: [] }; expect(matchRoute(route, "/home", "/home")).toBeTruthy(); expect(matchRoute(route, "/home", "/home/b")).toBeTruthy(); expect(matchRoute(route, "/home/", "/home")).toBeTruthy(); expect(matchRoute(route, "/home/", "/home/b")).toBeTruthy(); }); test("multiple branches (3+)", () => { const route: RouteConf = { path: "${APP.homepage}[, /b, /c/d]", exact: true, bricks: [] }; expect(matchRoute(route, "/home", "/home")).toBeTruthy(); expect(matchRoute(route, "/home", "/home/b")).toBeTruthy(); expect(matchRoute(route, "/home", "/home/c/d")).toBeTruthy(); });
📜 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
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/runtime/src/internal/matchRoutes.spec.ts
(2 hunks)packages/runtime/src/internal/matchRoutes.ts
(2 hunks)
⏰ 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: build (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/runtime/src/internal/matchRoutes.spec.ts (1)
1-2
: 导入项合理同时测试
matchRoutes
与matchRoute
很好地覆盖了新增导出。
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.
Pull Request Overview
This PR adds support for array-like path patterns in route configuration, allowing a single route to match multiple paths with a homepage prefix while maintaining exact matching and parameter parsing behavior.
- Introduces
getRoutePath
function to handle array-like path syntax${APP.homepage}[path1,path2]
- Updates
matchRoute
function to use the new path processing logic - Adds comprehensive test cases for the new array path functionality
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 2 comments.
File | Description |
---|---|
packages/runtime/src/internal/matchRoutes.ts | Adds array path support with new getRoutePath function and HOMEPAGE_PREFIX constant |
packages/runtime/src/internal/matchRoutes.spec.ts | Adds test cases for array path matching scenarios |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
4895487
to
52ddc91
Compare
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
♻️ Duplicate comments (1)
packages/runtime/src/internal/matchRoutes.ts (1)
49-59
: 为 matchRoute 显式标注返回类型,并将 MatchResult 以 type-only 方式导入
- 建议补上返回类型,稳定公开 API,避免隐式 any/回归。
- 同时将
MatchResult
改为import type
,避免在启用verbatimModuleSyntax
时产生无用的运行时导入。可直接在本段落内修改函数签名:
-export function matchRoute( +export function matchRoute( route: RouteConf, homepage: string, pathname: string -) { +): MatchResult | null { const path = getRoutePath(route, homepage); return matchPath(pathname, { path, exact: route.exact, }); }并在文件顶部将类型以 type-only 导入(此修改发生在变更片段外,作为补充说明):
// 顶部导入改为: import { matchPath } from "./matchPath.js"; import type { MatchResult } from "./matchPath.js";另外,
matchRoute
作为公开导出,相比matchRoutes
少了对route.path
的类型守卫。若担心外部误用,可在本函数开头补一行防御式校验(可选)。运行以下脚本快速确认 matchPath 的
path
参数是否确认为string | string[]
,以及是否已有其它数组形态用例:#!/bin/bash # 1) 查看 matchPath 的签名与 path 类型 rg -nP -C3 --type=ts '\bexport\s+(?:function|const)\s+matchPath\b' packages/runtime/src/internal # 2) 验证仓库内是否存在向 matchPath 传入数组 path 的用例 rg -nP -C2 --type=ts 'matchPath\([^,]+,\s*\{\s*path:\s*\[' packages # 3) 检查是否启用了 verbatimModuleSyntax(决定是否必须使用 import type) fd -a 'tsconfig*.json' | xargs -I{} sh -c 'echo "== {} =="; cat "{}"'
🧹 Nitpick comments (1)
packages/runtime/src/internal/matchRoutes.ts (1)
61-75
: 提升路径拼接的健壮性:去重斜杠并裁剪逗号分隔项的空白当前
${homepage}${p}
在homepage
以/
结尾或p
未以/
开头时,可能产生//
或缺少分隔符,且未裁剪分隔项空白。建议在本函数内做轻量规范化,避免匹配失败的边缘情况(例如 homepage 配置为/home/
)。-function getRoutePath(route: RouteConf, homepage: string): string | string[] { +function getRoutePath(route: RouteConf, homepage: string): string | string[] { if (route.path.startsWith(HOMEPAGE_PREFIX)) { const restPath = route.path.slice(HOMEPAGE_PREFIX.length); - if ( + if ( restPath.startsWith("[") && restPath.endsWith("]") && restPath.includes(",") ) { - const paths = restPath.slice(1, -1).split(","); - return paths.map((p) => `${homepage}${p}`); + const raw = restPath.slice(1, -1).split(","); + const base = homepage.endsWith("/") ? homepage.slice(0, -1) : homepage; + const join = (seg: string) => { + const s = seg.trim(); + const withSlash = s === "" || s.startsWith("/") ? s : `/${s}`; + return `${base}${withSlash}`; + }; + return raw.map((p) => join(p)); } - return `${homepage}${restPath}`; + const base = homepage.endsWith("/") ? homepage.slice(0, -1) : homepage; + const s = restPath.trim(); + const withSlash = s === "" || s.startsWith("/") ? s : `/${s}`; + return `${base}${withSlash}`; } return route.path; }说明:
- 保留空字符串分支(例如
"[,/b/...]"
的首项),以便产生纯 homepage 路径。- 只进行
trim
,不移除空项,避免改变语义。
📜 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
💡 Knowledge Base configuration:
- MCP integration is disabled by default for public repositories
- Jira integration is disabled by default for public repositories
- Linear integration is disabled by default for public repositories
You can enable these sources in your CodeRabbit configuration.
📒 Files selected for processing (2)
packages/runtime/src/internal/matchRoutes.spec.ts
(2 hunks)packages/runtime/src/internal/matchRoutes.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/runtime/src/internal/matchRoutes.spec.ts
⏰ 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: build (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/runtime/src/internal/matchRoutes.ts (1)
7-7
: HOMEPAGE_PREFIX 命名与用途清晰,保持内部常量即可当前用法简单直接,无需导出。继续保持即可。
依赖检查
组件之间的依赖声明,是微服务组件架构下的重要信息,请确保其正确性。
请勾选以下两组选项其中之一:
或者:
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG,请检查工作内容与提交信息是否相符,并在以下每组选项中都依次确认。
破坏性变更:
feat
作为提交类型。BREAKING CHANGE: 你的变更说明
。新特性:
feat
作为提交类型。问题修复:
fix
作为提交类型。杂项工作:
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动,例如修改注释、测试用例、开发文档等:
chore
,docs
,test
等作为提交类型。Summary by CodeRabbit