fix(): handle sibling routes matching order #4825
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在 Renderer、DataStore 和 interfaces 模块中新增并导出了 Changes
Estimated code review effort🎯 3 (中等) | ⏱️ ~20 分钟
Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
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/Renderer.ts (1)
892-952: 增量子路由的匹配逻辑更严谨，但依赖 parentRoutes 非空约束这一段增量渲染逻辑：
先通过
const { route: parentRoute, routes: pageSiblingRoutes } = parentRoutes[parentRoutes.length - 1];拿到当前页面层级的 route 与其 siblings，并把
pageSiblingRoutes传给performIncrementalRender；在回调里对每一层
parentRoutes调用matchTargetRoute(route, routes, homepage, pathname)，只在“该层中按 sibling 顺序首个命中的就是当前route且前后 URL 都满足这一点，并且最底层 parentRoute 的 params 没变”的情况下才认为可以进行增量渲染。这样可以避免 sibling 顺序变化或被其他路由“抢先匹配”时依然错误地走增量路径，逻辑上更安全，符合“按同层顺序取首个命中路由”的语义。
需要注意的一点是，这里仍然假设在进入
slotConf.type === "routes"分支时parentRoutes.length > 0（否则解构会在运行时抛错）。这个假设看起来与旧版实现一致，但如果未来有调用方直接在“非路由上下文”里调用renderBricks(..., parentRoutes = [])并使用 routes 槽位，会触发这个问题。建议在调用侧保持不变的约束，或者在这里加一层防御（例如在parentRoutes.length === 0时直接跳过增量渲染逻辑）。Also applies to: 955-987
🧹 Nitpick comments (1)
packages/runtime/src/internal/Renderer.ts (1)
1379-1399: matchTargetRoute 有效封装了兄弟路由首命中的查找逻辑
matchTargetRoute在 siblings 列表上顺序调用matchRoute，一旦命中就break，仅当“首个命中的 sibling 恰好是当前 route”时才返回对应的MatchResult。配合上层parentRoutes.every的判断，可以精确表达“上一次和这一次在每一层的首个命中的都是同一个 route”这一条件，避免遗漏 sibling 匹配顺序带来的边界情况，实现上简洁且易于复用，看起来没有问题。如果后续这段逻辑有更多使用场景，可以考虑加上显式返回类型（如MatchResult | null）和简短注释，以便类型提示更清晰。
📜 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 (3)
packages/runtime/src/internal/Renderer.ts(9 hunks)packages/runtime/src/internal/data/DataStore.ts(5 hunks)packages/runtime/src/internal/interfaces.ts(1 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). (3)
- GitHub Check: CodeQL analysis (javascript-typescript)
- GitHub Check: build (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/runtime/src/internal/interfaces.ts (1)
207-211: RouteNode 类型设计清晰，便于在渲染和数据层共享路由上下文
RouteNode同时携带当前route以及同层routes，很好地抽象出了“路由 + 兄弟路由”的上下文，后续在渲染和 DataStore 中解构使用会更直观，语义也和注释保持一致，看起来没有问题。packages/runtime/src/internal/data/DataStore.ts (1)
20-26: RouteNode 在线路由路径上的接入与原有行为保持一致这里把
routePath类型切到RouteNode[]，并在两处使用中通过for (const { route } of routePath)解构出实际的RouteConf，继续用routeMap/routeStackMap按RouteConf做 WeakMap key，整体语义与原来传RouteConf[]基本等价，同时为增量渲染那边需要的 sibling 信息留出了空间，看起来比较自然，没有发现行为上的回归风险。Also applies to: 330-361, 414-581
packages/runtime/src/internal/Renderer.ts (1)
48-56: 将 parentRoutes 升级为 RouteNode[] 的改动整体自然且兼容现有调用
parentRoutes从RouteConf[]升级为RouteNode[]，并在renderRoutes中通过const routePath = parentRoutes.concat({ route, routes });构造新的路径，从而在后续递归和 DataStore 中既能拿到“当前 route”，又能拿到“该层所有 siblings”。函数签名与内部调用（
renderRoutes/renderBricks/renderBrick/renderForEach）都已经同步更新，看起来数据流清晰、没有遗漏调用点，行为上与旧版保持一致的同时，为后面的增量匹配逻辑提供了更多上下文信息。Also applies to: 98-108, 245-255, 282-292, 319-329, 1127-1138
weareoutman
force-pushed
the
steve/v3-sibling-routes
branch
from
27fb02f to
605677f
Compare
weareoutman
force-pushed
the
steve/v3-sibling-routes
branch
from
605677f to
b64d8bd
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/runtime/src/internal/Renderer.ts (1)
893-1005: 兄弟路由匹配顺序逻辑清晰，但parentRoutes非空是假定条件，建议显式约束或保护这里通过
RouteNode增强了增量子路由的匹配判断：
const { route: parentRoute } = parentRoutes[parentRoutes.length - 1];取当前父路由；在增量回调中，
parentRoutes.every(({ route, routes }) => { ... })对父链每一层执行：let prevMatch: MatchResult | null = null; let newMatch: MatchResult | null = null; return ( (prevMatch = matchTargetRoute(route, routes, homepage, prevLocation.pathname)) && (newMatch = matchTargetRoute(route, routes, homepage, pathname)) && (route !== parentRoute || isRouteParamsEqual(prevMatch.params, newMatch.params)) );新增的
matchTargetRoute(route, routes, homepage, pathname)会在 siblings 中按顺序找到第一个匹配的路由：
- 若首匹配 === 当前
route，返回对应MatchResult；- 否则返回
null，表示“虽然有匹配，但当前 route 已经被前面的 sibling 抢占”，从而阻止增量渲染。这很好地满足了“按兄弟路由顺序处理匹配”的需求，只有在“父链上每一层的目标路由都仍然是其 siblings 中的首个匹配，且最底层父路由的参数未变”时才执行增量渲染。
需要注意的一点：
const { route: parentRoute } = parentRoutes[parentRoutes.length - 1];隐含假设parentRoutes.length > 0。从当前调用链（renderRoutes自顶向下构造routePath并传递）来看，这个条件成立，但如果未来有新的调用方（例如测试或内部工具）以空数组调用renderBrick/legacyRenderBrick并触发路由 slot 分支，这里会在运行时抛错。建议：
要么在进入路由 slot 分支前加一个早期保护，例如：
if (!parentRoutes.length) { // 理论上不应发生，可以视情况选择直接返回 lastOutput 或抛出更明确的错误 return lastOutput; }要么在 API 文档/类型注释上明确规定：只有在存在至少一层已匹配路由时才允许传入
parentRoutes（例如通过调用侧断言）。这样可以把当前实现依赖的前置条件表达得更清楚，减少后续维护时的踩坑概率。
Also applies to: 916-943, 1391-1408
🧹 Nitpick comments (1)
packages/runtime/src/internal/Renderer.ts (1)
932-943: matchTargetRoute 实现符合“首个匹配”的语义，便于后续复用
matchTargetRoute把原先散落在增量逻辑中的“在 siblings 中找第一个匹配路由”的模式抽成了独立函数：function matchTargetRoute( route: RouteConf, routes: RouteConf[], homepage: string, pathname: string ) { for (const sibling of routes) { const match = matchRoute(sibling, homepage, pathname); if (match) { return sibling === route ? match : null; } } return null; }优点：
- 语义更清晰，显式体现“首个匹配路由必须是传入的
route本身，否则认为不匹配”的策略；- 与上方
parentRoutes.every(...)的使用方式契合，能很好地表达“父链上每层的路由在新旧 URL 上都没有被其他 sibling 抢占”的约束。如果后续还有其他地方需要以 siblings 顺序判断“目标 route 是否仍是首个匹配”，建议也统一调用该 helper，避免重复实现。
Also applies to: 1391-1408
📜 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 (3)
packages/runtime/src/internal/Renderer.ts(9 hunks)packages/runtime/src/internal/data/DataStore.ts(5 hunks)packages/runtime/src/internal/interfaces.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/runtime/src/internal/interfaces.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 (2)
packages/runtime/src/internal/data/DataStore.ts (1)
20-25: RouteNode 引入与 routePath 解构用法整体合理，行为与旧实现一致这里将
routePath从RouteConf[]升级为RouteNode[]，在两处使用中都通过for (const { route } of routePath) { ... }只取
route字段来更新routeMap/routeStackMap，与原先直接遍历RouteConf[]的逻辑等价，因此：
disposeDataInRoutes(routes: RouteConf[])依旧只依赖路由对象本身，不受routes（兄弟集合）字段影响；mountAsyncData(route?: RouteConf)仍按单个路由键查询routeMap，与新结构兼容；resolve上的routePath?: RouteNode[]参数虽然目前未使用，但调用方（如Renderer.renderRoutes）已经传入了完整的 RouteNode 路径，为后续在数据解析阶段利用兄弟路由信息预留了空间。从 DataStore 视角看，这次改动没有引入行为改变，看起来可以接受。
Also applies to: 330-360, 414-420, 568-577
packages/runtime/src/internal/Renderer.ts (1)
48-56: parentRoutes 统一使用 RouteNode[] 的改动与上下文一致本文件中所有
parentRoutes参数统一改为RouteNode[]，并在renderRoutes中通过const routePath = parentRoutes.concat({ route, routes });构造每一层的
RouteNode（携带当前匹配route及其同级routes），然后一路向下传递到renderBricks/renderBrick/legacyRenderBrick/renderForEach，以及ctxStore.define。这样：
- 每一层路由的“当前匹配 + 兄弟列表”都保存在
parentRoutes链中，为后续增量渲染时的父链校验提供了足够上下文；- DataStore 侧只解构其中的
route字段，依旧以RouteConf作为 key，不影响既有数据回收与 async 挂载逻辑。整体类型与数据流更新是自洽的，看不出明显回归风险。
Also applies to: 98-108, 245-255, 282-292, 319-329, 1140-1151
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3 #4825 +/- ##
==========================================
- Coverage 95.17% 95.16% -0.01%
==========================================
Files 212 212
Lines 9348 9357 +9
Branches 1800 1802 +2
==========================================
+ Hits 8897 8905 +8
- Misses 331 332 +1
Partials 120 120
🚀 New features to boost your workflow:
|
next-core
|
||||||||||||||||||||||||||||
| Project |
next-core
|
| Branch Review |
steve/v3-sibling-routes
|
| Run status |
|
| Run duration | 00m 22s |
| Commit |
|
| Committer | Shenwei Wang |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
17
|
| View all changes introduced in this branch ↗︎ | |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated no new comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/runtime/src/internal/Renderer.ts (1)
892-953: 增量子路由匹配逻辑基本正确，但建议增强边界防御与注释说明
新增的
matchTargetRoute通过在兄弟数组中按顺序调用matchRoute，并仅在“首个匹配刚好是目标 route”时返回结果，可以修正之前忽略兄弟优先级的问题，逻辑上符合“按 storyboard 中 siblings 顺序匹配”的意图。回调中对父路由链的校验：
const { route: parentRoute } = parentRoutes[parentRoutes.length - 1]; ... parentRoutes.every(({ route, routes }) => { let prevMatch: MatchResult | null = null; let newMatch: MatchResult | null = null; return ( (prevMatch = matchTargetRoute(...prevLocation.pathname)) && (newMatch = matchTargetRoute(...pathname)) && (route !== parentRoute || isRouteParamsEqual(prevMatch.params, newMatch.params)) ); });能确保：
- 新旧 location 下每一层父路由在同一 siblings 集合中仍是“首个匹配”的路由；
- 对最外层启用
incrementalSubRoutes的那一层，还会比对 params（通过isRouteParamsEqual）以避免参数变化时错误增量。这与 PR 标题“处理兄弟路由匹配顺序”相符。
有两个小的防御性点可以考虑（其中第一个与已有评论重复）：
- 这里直接解构
parentRoutes[parentRoutes.length - 1]，假定parentRoutes至少有一个元素。从 v3 的使用约定来看，routes 型 slot 通常只出现在 route 树内部，确实应该有父路由；但为了防止未来调用方误传空数组，建议在进入这段逻辑前加一个显式判断（例如if (!parentRoutes.length) return routesOutput;或提前return），既能消除潜在运行时错误，也能让意图更清晰。- 回调体中访问
prevLocation.pathname，依赖rendererContext.performIncrementalRender始终传入非空的prevLocation。如果该约定没有在类型或实现里严格保证，可以考虑在回调入口做一次防御性校验（例如若!prevLocation则直接返回false），避免未来改动导致 NPE。另外，
matchTargetRoute本身依赖“第二个参数routes必须是完整兄弟集合且顺序与 storyboard 一致”，建议在函数注释中补一句这点约束，方便读者快速理解它与普通matchRoute的差别。整体上，这段增量渲染逻辑方向正确，建议仅补充上述边界防御和文档说明即可。
Also applies to: 975-1004, 1040-1050, 1378-1396
🧹 Nitpick comments (1)
packages/runtime/src/internal/data/DataStore.ts (1)
20-25: RouteNode 路径改造与现有 routeMap/routeStackMap 兼容良好这里将
routePath从RouteConf[]升级为RouteNode[]，但在内部仍通过解构{ route }来使用，routeMap/routeStackMap继续以RouteConf作为 WeakMap 键，disposeDataInRoutes的行为也保持不变，看起来是无损改造。可以考虑在
define/resolve的注释或类型文档中明确说明routePath的语义（例如“从根到当前路由的链，每个节点携带该层所有兄弟 routes”），方便后续维护者理解这一结构。Also applies to: 330-335, 349-357, 414-420, 568-576
📜 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 (3)
packages/runtime/src/internal/Renderer.ts(9 hunks)packages/runtime/src/internal/data/DataStore.ts(5 hunks)packages/runtime/src/internal/interfaces.ts(1 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). (3)
- GitHub Check: codecov/patch
- GitHub Check: Agent
- GitHub Check: build (20.x)
🔇 Additional comments (2)
packages/runtime/src/internal/interfaces.ts (1)
207-210: RouteNode 类型设计清晰，契合后续使用场景
route与同层兄弟路由集合routes的拆分非常直观，能很好支撑 Renderer/DataStore 中对“路由链 + 兄弟列表”的建模，目前看没有语义或类型问题。packages/runtime/src/internal/Renderer.ts (1)
48-56: parentRoutes 改为 RouteNode[] 的整体改造结构合理
- 几个渲染入口统一将
parentRoutes从RouteConf[]升级为RouteNode[]，调用链始终沿用这一新结构，类型一致性良好。- 在
renderRoutes中通过const routePath = parentRoutes.concat({ route, routes })构造“从根到当前路由”的节点链，随后传给ctxStore.define，与 DataStore 中基于RouteNode的 routePath 使用方式保持对齐。- 下游的
renderBricks/renderBrick/renderForEach均透传parentRoutes，保证在子树（尤其是增量子路由）中可以获知完整路由链和兄弟集合。整体看，这部分纯粹是结构升级，没有改变原有控制流，属于比较稳妥的重构。
Also applies to: 98-108, 245-255, 283-292, 319-329, 1126-1137
依赖检查
组件之间的依赖声明，是微服务组件架构下的重要信息，请确保其正确性。
请勾选以下两组选项其中之一：
或者：
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG，请检查工作内容与提交信息是否相符，并在以下每组选项中都依次确认。
破坏性变更：
feat作为提交类型。BREAKING CHANGE: 你的变更说明。新特性：
feat作为提交类型。问题修复：
fix作为提交类型。杂项工作：
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动，例如修改注释、测试用例、开发文档等：
chore,docs,test等作为提交类型。Summary by CodeRabbit
发行说明