-
Notifications
You must be signed in to change notification settings - Fork 11
feat(): support slots in template #4530
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
Conversation
Walkthrough此次更改涉及多个文件,主要集中在模板和砖块管理的功能增强上。 Changes
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 (
|
next-core
|
||||||||||||||||||||||||||||
| Project |
next-core
|
| Branch Review |
steve/v3-tpl-slot
|
| Run status |
|
| Run duration | 00m 24s |
| Commit |
|
| Committer | Shenwei Wang |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
16
|
| View all changes introduced in this branch ↗︎ | |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3 #4530 +/- ##
==========================================
+ Coverage 95.21% 95.23% +0.01%
==========================================
Files 205 206 +1
Lines 8876 8911 +35
Branches 1685 1697 +12
==========================================
+ Hits 8451 8486 +35
+ Misses 320 319 -1
- Partials 105 106 +1
|
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: 6
🧹 Outside diff range and nitpick comments (4)
packages/runtime/src/internal/CustomTemplates/replaceSlotWithSlottedBricks.spec.ts (1)
19-57: 建议增强错误消息的精确性验证当前的错误处理测试很全面,涵盖了重要的错误场景。建议使用
expect.toThrowError来进行更精确的错误消息匹配,这样可以确保错误提示的一致性。建议这样改进测试代码:
- expect(() => - replaceSlotWithSlottedBricks(brickConf, mockHostContext, identity) - ).toThrow( - `Can not use "if" in a slot currently, check your template "tpl-test"` - ); + expect(() => + replaceSlotWithSlottedBricks(brickConf, mockHostContext, identity) + ).toThrowError(new Error( + `Can not use "if" in a slot currently, check your template "tpl-test"` + ));packages/runtime/src/internal/interfaces.ts (1)
134-134: 代码变更合理,建议添加文档注释代码变更本身设计合理,使用
Set<string>确保了插槽名称的唯一性。不过建议添加 JSDoc 注释来说明该属性的用途和使用场景。建议添加如下注释:
+ /** 用于跟踪模板中已使用的插槽名称集合 */ usedSlots: Set<string>;packages/runtime/src/internal/Renderer.spec.ts (2)
3305-3401: 测试用例结构完整,建议补充边缘场景测试测试用例完整地覆盖了模板中插槽的基本功能,包括基础插槽、useBrick 中的插槽、具名插槽等场景。代码结构清晰,测试步骤合理。
建议补充以下测试场景:
- 动态插槽名称
- 插槽内容为空的情况
- 插槽嵌套的场景
- 错误处理场景(如插槽名称冲突)
3403-3452: 错误处理测试完善,建议优化错误信息测试用例很好地验证了模板中代理插槽与插槽元素不能同时使用的约束条件。错误处理的实现方式合适。
建议优化错误信息,使其更加清晰和具体:
-`[Error: Can not have proxied slot ref when the parent has a slot element child, check your template "my.tpl-m" and ref "main"]` +`[Error: Invalid slot configuration in template "my.tpl-m": Cannot use proxied slot (ref: "main") together with slot element in the same parent. These two slot mechanisms are mutually exclusive.]`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- packages/runtime/src/internal/CustomTemplates/expandCustomTemplate.ts (4 hunks)
- packages/runtime/src/internal/CustomTemplates/replaceSlotWithSlottedBricks.spec.ts (1 hunks)
- packages/runtime/src/internal/CustomTemplates/replaceSlotWithSlottedBricks.ts (1 hunks)
- packages/runtime/src/internal/CustomTemplates/setupTemplateProxy.ts (3 hunks)
- packages/runtime/src/internal/CustomTemplates/setupUseBrickInTemplate.ts (3 hunks)
- packages/runtime/src/internal/Renderer.spec.ts (1 hunks)
- packages/runtime/src/internal/Renderer.ts (1 hunks)
- packages/runtime/src/internal/data/DataStore.ts (0 hunks)
- packages/runtime/src/internal/interfaces.ts (1 hunks)
💤 Files with no reviewable changes (1)
- packages/runtime/src/internal/data/DataStore.ts
🧰 Additional context used
🪛 Biome
packages/runtime/src/internal/CustomTemplates/expandCustomTemplate.ts
[error] 141-141: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/runtime/src/internal/CustomTemplates/replaceSlotWithSlottedBricks.ts
[error] 6-6: Do not shadow the global "hasOwnProperty" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (20)
packages/runtime/src/internal/CustomTemplates/replaceSlotWithSlottedBricks.spec.ts (1)
1-18: 测试设置结构清晰!导入语句简洁明了,测试上下文的设置也很规范。使用
beforeEach确保每个测试用例都有干净的上下文环境,这是一个很好的实践。packages/runtime/src/internal/CustomTemplates/setupTemplateProxy.ts (3)
19-20: 函数签名更新:新增 slotted 参数函数签名的更改合理地扩展了插槽配置的控制能力。
142-142: 导出 setupTemplateExternalBricksWithForEach 函数将此函数导出增加了代码的可重用性,这是一个好的改进。不过建议验证其他模块是否需要使用此函数:
#!/bin/bash # 检查是否有其他模块引用此函数 rg "setupTemplateExternalBricksWithForEach" --type ts
58-64: 新增插槽冲突检查逻辑新增的错误处理可以有效防止模板中的插槽配置冲突,提高了代码的健壮性。错误信息清晰地指出了问题所在,有助于开发者快速定位和修复问题。
建议验证此错误处理的测试覆盖情况:
packages/runtime/src/internal/Renderer.ts (1)
1251-1252: 代码重构提升了可读性!通过使用对象解构和更清晰的变量命名,代码变得更加易读和维护。解构模式
{ slot: sl, ...child }很好地分离了插槽属性和其他子属性。packages/runtime/src/internal/CustomTemplates/setupUseBrickInTemplate.ts (5)
1-5: 导入必要的类型定义引入
UseBrickSlotConf、UseBrickSlotsConf、UseSingleBrickConf类型定义,确保类型检查的准确性。
10-10: 添加对replaceSlotWithSlottedBricks的导入成功导入了
replaceSlotWithSlottedBricks方法,为后续的插槽替换逻辑提供支持。
58-61: 正确地标记插槽状态通过定义
slotted变量和markChild方法,正确地追踪了子元素是否包含插槽。
62-70: 在flatMap中使用setup时需注意返回值类型由于
setup函数可能返回数组,flatMap能正确展开返回的数组。但如果setup返回单个对象,可能导致结果不符合预期。请确认
setup函数在所有情况下均返回数组,以确保flatMap操作的正确性。
76-76: 更新setupTemplateProxy的调用参数成功地将
slotted参数传递给setupTemplateProxy,确保模板代理能够感知插槽的状态。packages/runtime/src/internal/CustomTemplates/replaceSlotWithSlottedBricks.ts (1)
12-75: 代码实现逻辑清晰,功能符合预期函数
replaceSlotWithSlottedBricks的实现逻辑清晰,正确处理了插槽的各种情况,包括输入验证、错误处理和默认行为,符合预期功能。packages/runtime/src/internal/CustomTemplates/expandCustomTemplate.ts (9)
4-4: 新引入的类型导入已正确添加已正确添加了对
SlotConfOfBricks的类型导入,确保了类型定义的完整性。
23-23: 成功引入新的模块成功引入了
replaceSlotWithSlottedBricks模块,为后续的插槽处理提供了支持。
119-119: 在 hostContext 中添加了 usedSlots 属性在
hostContext中添加了usedSlots属性,有助于跟踪已使用的插槽。
125-127: 使用 flatMap 优化 bricks 的展开在构建新的
bricks配置时,使用flatMap可以有效地处理嵌套的数组结构,提升代码的简洁性和可读性。
135-138: 为 expandBrickInTemplate 函数添加可选参数 markSlotted为
expandBrickInTemplate函数添加了一个可选参数markSlotted,可以在处理插槽时标记已被插槽化的元素,增强了函数的灵活性。
144-151: 处理 brick 类型为 "slot" 的情况新增了对
brickConfInTemplate.brick === "slot"的判断,并调用replaceSlotWithSlottedBricks进行处理。如果存在markSlotted回调函数,则会调用它来标记已插槽化的元素。
165-169: 引入 slotted 标志用于标记插槽状态引入了
slotted变量和markChild函数,用于在子元素中存在插槽时进行标记,便于在后续处理时使用。
174-175: 在展开子 brick 时传递 markChild 回调在处理 slots 的 bricks 时,展开每个子 brick 时传递了
markChild回调函数,以便在子元素中检测和标记插槽的存在。
185-190: 更新 setupTemplateProxy 的调用,加入 slotted 参数在调用
setupTemplateProxy时,新增了slotted参数,以便在模板代理中处理插槽相关的逻辑。
packages/runtime/src/internal/CustomTemplates/replaceSlotWithSlottedBricks.spec.ts
Show resolved
Hide resolved
packages/runtime/src/internal/CustomTemplates/setupUseBrickInTemplate.ts
Show resolved
Hide resolved
packages/runtime/src/internal/CustomTemplates/setupUseBrickInTemplate.ts
Show resolved
Hide resolved
packages/runtime/src/internal/CustomTemplates/setupUseBrickInTemplate.ts
Show resolved
Hide resolved
packages/runtime/src/internal/CustomTemplates/replaceSlotWithSlottedBricks.ts
Show resolved
Hide resolved
54f7979 to
079cd04
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: 4
🧹 Outside diff range and nitpick comments (6)
packages/runtime/src/internal/CustomTemplates/replaceSlotWithSlottedBricks.ts (1)
29-29: 建议增强空值处理当前的空值合并运算符(
??)处理方式可能在某些边缘情况下不够健壮。建议修改为:
-const slot = String(brickConf.properties?.name ?? ""); +const slot = String(brickConf.properties?.name ?? "default");这样可以使用更有意义的默认值,提高代码的可读性和可维护性。
packages/runtime/src/internal/CustomTemplates/expandCustomTemplate.ts (2)
119-119: 架构改进:优化了插槽的处理方式
- 通过
usedSlotsSet 来追踪已使用的插槽,这是一个很好的设计选择- 使用
flatMap而不是map使得插槽可以返回多个构件,增加了灵活性Also applies to: 125-127
135-138: 建议添加函数文档建议为
expandBrickInTemplate函数添加 JSDoc 文档,说明:
- 参数
markSlotted的用途和调用时机- 返回值可能是数组的场景
- 插槽替换的工作原理
packages/runtime/src/internal/CustomTemplates/setupTemplateProxy.ts (1)
142-142: 导出 setupTemplateExternalBricksWithForEach 函数将此函数导出是一个合理的改动,使其可以在其他模块中重用。建议添加详细的函数文档,说明其用途和参数含义。
建议添加如下文档注释:
+/** + * 为模板的外部构件设置 forEach 上下文 + * @param bricks - 构件配置数组 + * @param forEachItem - forEach 当前项 + * @param forEachIndex - forEach 当前索引 + * @param forEachSize - forEach 总大小 + * @returns 处理后的构件配置数组 + */ export function setupTemplateExternalBricksWithForEach(packages/runtime/src/internal/Renderer.spec.ts (2)
3306-3401: 测试用例结构清晰,建议添加更多测试场景测试用例很好地覆盖了模板中插槽的基本功能,包括默认插槽、命名插槽以及在 useBrick 中的插槽使用。
建议添加以下测试场景以提高覆盖率:
- 测试插槽的回退内容(当没有提供内容时)
- 测试动态插槽名称
- 测试插槽内容的作用域插槽数据访问
3403-3452: 建议补充正向测试用例当前测试用例很好地验证了错误情况(同时使用插槽元素和代理插槽时的错误处理),但建议补充以下内容:
- 添加正向测试用例,展示正确的使用方式:
- 仅使用插槽元素的场景
- 仅使用代理插槽的场景
- 测试不同层级的插槽代理组合
- 为错误信息添加更多断言,确保错误消息包含足够的上下文信息
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (9)
- packages/runtime/src/internal/CustomTemplates/expandCustomTemplate.ts (4 hunks)
- packages/runtime/src/internal/CustomTemplates/replaceSlotWithSlottedBricks.spec.ts (1 hunks)
- packages/runtime/src/internal/CustomTemplates/replaceSlotWithSlottedBricks.ts (1 hunks)
- packages/runtime/src/internal/CustomTemplates/setupTemplateProxy.ts (3 hunks)
- packages/runtime/src/internal/CustomTemplates/setupUseBrickInTemplate.ts (3 hunks)
- packages/runtime/src/internal/Renderer.spec.ts (1 hunks)
- packages/runtime/src/internal/Renderer.ts (1 hunks)
- packages/runtime/src/internal/data/DataStore.ts (0 hunks)
- packages/runtime/src/internal/interfaces.ts (1 hunks)
💤 Files with no reviewable changes (1)
- packages/runtime/src/internal/data/DataStore.ts
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/runtime/src/internal/CustomTemplates/replaceSlotWithSlottedBricks.spec.ts
- packages/runtime/src/internal/Renderer.ts
- packages/runtime/src/internal/interfaces.ts
🧰 Additional context used
🪛 Biome
packages/runtime/src/internal/CustomTemplates/expandCustomTemplate.ts
[error] 141-141: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/runtime/src/internal/CustomTemplates/replaceSlotWithSlottedBricks.ts
[error] 6-6: Do not shadow the global "hasOwnProperty" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
🔇 Additional comments (12)
packages/runtime/src/internal/CustomTemplates/setupUseBrickInTemplate.ts (5)
1-10: 新增的依赖导入逻辑清晰明确新增的
replaceSlotWithSlottedBricks导入与文件功能相符,类型导入也完整准确。
58-61: 状态追踪逻辑简洁有效
slotted状态的追踪实现简单清晰,markChild回调函数的设计符合单一职责原则。
76-76: 模板代理设置逻辑完整
setupTemplateProxy的调用正确传递了slotted状态,保证了插槽状态的正确传递。
43-46:⚠️ Potential issue函数签名的返回类型需要统一
当前
setup函数的返回类型为联合类型UseSingleBrickConf | UseSingleBrickConf[],这可能导致使用时的类型检查复杂性增加。建议修改为始终返回数组类型:
- ): UseSingleBrickConf | UseSingleBrickConf[] { + ): UseSingleBrickConf[] {这样可以简化调用方的类型处理逻辑。
Likely invalid or redundant comment.
47-50: 插槽替换逻辑需要类型安全保证在处理
item.brick === "slot"的情况时,需要确保replaceSlotWithSlottedBricks的返回值类型与函数签名一致。执行以下脚本验证
replaceSlotWithSlottedBricks的返回类型:✅ Verification successful
Let me try a different approach to gather information about the
replaceSlotWithSlottedBricksfunction.
从搜索结果中,我们可以看到
replaceSlotWithSlottedBricks函数的定义和使用情况。该函数的类型签名为:export function replaceSlotWithSlottedBricks<T extends BrickConf | UseSingleBrickConf>( brickConf: T, hostContext: TemplateHostContext, expand: (item: T, hostContext: TemplateHostContext) => T | T[] ): T[]让我们生成最终响应:
插槽替换逻辑已具备类型安全保证
replaceSlotWithSlottedBricks函数已经通过泛型和明确的返回类型提供了完整的类型安全保证:
- 输入参数
brickConf被限制为BrickConf | UseSingleBrickConf类型- 返回值明确定义为
T[]类型- 在
setupUseBrickInTemplate.ts中的使用方式与类型定义完全匹配🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查 replaceSlotWithSlottedBricks 的返回类型定义 ast-grep --pattern 'function replaceSlotWithSlottedBricks($$$): $return { $$$ }'Length of output: 84
Script:
#!/bin/bash # Search for the replaceSlotWithSlottedBricks function definition rg "replaceSlotWithSlottedBricks" -A 10 # Also search for any type definitions related to this function rg "type.*replaceSlotWithSlottedBricks"Length of output: 16745
packages/runtime/src/internal/CustomTemplates/replaceSlotWithSlottedBricks.ts (2)
6-6: 建议重命名导入的hasOwnProperty函数为避免与 JavaScript 全局属性冲突,建议重命名导入的函数。
🧰 Tools
🪛 Biome
[error] 6-6: Do not shadow the global "hasOwnProperty" property.
Consider renaming this variable. It's easy to confuse the origin of variables when they're named after a known global.
(lint/suspicious/noShadowRestrictedNames)
72-74: 考虑添加类型守卫在类型转换时使用
as可能不够安全。建议添加运行时类型检查:
如果没有找到现成的类型守卫,我可以帮助实现一个。
packages/runtime/src/internal/CustomTemplates/expandCustomTemplate.ts (2)
4-6: 导入和类型定义变更看起来不错!新增的插槽相关类型定义和
replaceSlotWithSlottedBricks的导入很好地支持了模板中的插槽功能。Also applies to: 23-23
144-151: 验证插槽替换逻辑的正确性新增的插槽处理逻辑涉及多个关键步骤:
- 识别 "slot" 构件
- 调用 markSlotted 回调
- 使用 replaceSlotWithSlottedBricks 进行替换
- 通过 slotted 标志向上传递插槽状态
建议验证这个流程在各种场景下的正确性。
Also applies to: 165-168
✅ Verification successful
Let me gather more information about the slot replacement logic by searching for related test files and implementation details.
让我再搜索一下插槽替换函数的具体实现和相关测试用例。
插槽替换逻辑已经过完整验证
根据代码分析和测试用例的检查,插槽替换逻辑已经实现了完整的错误处理和边界情况验证:
已正确处理以下场景:
- 禁止在插槽中使用条件语句(if)
- 禁止使用表达式作为插槽名称
- 防止重复的插槽名称
- 支持默认插槽内容
- 支持外部插槽内容替换
测试用例完整覆盖了各种场景:
- 条件判断的错误处理
- 插槽名称验证
- 重复插槽检查
- 默认内容和外部内容的替换逻辑
错误处理机制完善,能够提供清晰的错误信息指导开发者排查问题。
代码实现符合预期,无需额外修改。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 搜索测试文件中的插槽相关测试用例 echo "查找插槽替换相关的测试用例:" rg -l "slot.*brick.*test" --type ts # 检查是否有未处理的边缘情况 echo "检查可能的边缘情况处理:" ast-grep --pattern 'if ($brick === "slot") { $$$ }'Length of output: 453
Script:
#!/bin/bash # 搜索所有包含 slot 相关测试的文件 echo "搜索包含插槽测试的文件:" rg -l "describe.*slot|test.*slot" --type ts # 查找 replaceSlotWithSlottedBricks 的实现 echo -e "\n查找插槽替换函数的实现:" ast-grep --pattern 'function replaceSlotWithSlottedBricks' # 查找所有与插槽相关的条件判断 echo -e "\n查找插槽相关的条件判断:" rg "if.*slot" -A 2 --type ts # 查找插槽相关的类型定义 echo -e "\n查找插槽相关的类型定义:" rg "type.*Slot|interface.*Slot" --type tsLength of output: 10077
Script:
#!/bin/bash # 查看插槽替换函数的完整实现 echo "查看插槽替换函数的实现:" rg -A 20 "function replaceSlotWithSlottedBricks" --type ts # 查看相关测试用例的具体内容 echo -e "\n查看插槽相关的测试用例:" rg "describe.*slot.*|it.*slot.*" -A 5 --type ts # 检查是否存在其他插槽相关的错误处理 echo -e "\n查看插槽相关的错误处理:" rg "throw.*slot" -A 2 --type tsLength of output: 10233
packages/runtime/src/internal/CustomTemplates/setupTemplateProxy.ts (3)
19-20: 函数签名更新:新增 slotted 参数函数签名的更改合理地扩展了插槽管理的能力,使模板系统能够处理更复杂的插槽场景。
58-64: 增强了插槽冲突检测新增的错误处理逻辑可以有效防止模板中的插槽配置冲突,提高了代码的健壮性。错误信息清晰地指出了问题所在,有助于开发者快速定位和解决问题。
Line range hint
142-190: 验证 forEach 上下文传递的正确性需要确保 forEach 上下文在不同层级的模板嵌套中能够正确传递。
✅ Verification successful
Let me try a different search strategy to verify the forEach context passing in nested templates.
根据代码分析结果,我可以生成最终响应了:
forEach 上下文传递机制已得到正确实现
代码中已经实现了完善的 forEach 上下文传递机制:
- 在
setupTemplateExternalBricksWithForEach函数中明确处理了:forEachbrick 的特殊情况,保持其原有的 forEach 上下文- 通过
symbolForTPlExternalForEachItem/Index/Size符号正确传递 forEach 上下文- 在
expandCustomTemplate.ts中通过删除内部 forEach 上下文来维护模板内外的边界- 测试用例
replaceSlotWithSlottedBricks.spec.ts验证了 forEach 上下文的传递🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 描述:检查模板嵌套中的 forEach 上下文传递 # 测试:搜索模板嵌套中使用 forEach 的场景 rg -l "forEach.*setupTemplateExternalBricksWithForEach" --type ts # 测试:检查是否存在相关的测试用例 rg -l "test.*forEach.*nested.*template" --type tsLength of output: 119
Script:
#!/bin/bash # 搜索与 forEach 和模板相关的代码 rg -A 5 "forEach" --type ts packages/runtime/src/internal/CustomTemplates/ # 搜索相关测试文件 fd "test" --type f --extension ts packages/runtime/src/internal/CustomTemplates/ # 查找模板相关的测试用例 rg -A 5 "describe.*template" --type ts packages/runtime/src/__tests__/Length of output: 12474
依赖检查
组件之间的依赖声明,是微服务组件架构下的重要信息,请确保其正确性。
请勾选以下两组选项其中之一:
或者:
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG,请检查工作内容与提交信息是否相符,并在以下每组选项中都依次确认。
破坏性变更:
feat作为提交类型。BREAKING CHANGE: 你的变更说明。新特性:
feat作为提交类型。问题修复:
fix作为提交类型。杂项工作:
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动,例如修改注释、测试用例、开发文档等:
chore,docs,test等作为提交类型。Summary by CodeRabbit
新功能
usedSlots属性,以跟踪已使用的插槽标识符。错误修复
测试
replaceSlotWithSlottedBricks函数的单元测试,验证其在不同条件下的行为。Renderer组件的测试覆盖率,特别关注模板中的插槽功能。