-
Notifications
You must be signed in to change notification settings - Fork 10
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
fix(): allow transform in useBrick + slots #4555
Conversation
Walkthrough本次更改主要涉及多个文件的修改,重点是 Changes
Warning There were issues while running the following tools: 🔧 eslintpackages/runtime/src/internal/Renderer.tsOops! Something went wrong! :(
ESLint: 8.57.1
ESLint couldn't find the config "@next-core/eslint-config-next" to extend from. Please check that the name of the config is correct.
The config "@next-core/eslint-config-next" was referenced from the config file in "/.eslintrc".
If you still have problems, please stop by https://eslint.org/chat/help to chat with the team.
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 (
|
1f613a3
to
360d95c
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v3 #4555 +/- ##
=======================================
Coverage 95.23% 95.23%
=======================================
Files 206 206
Lines 8912 8917 +5
Branches 1698 1700 +2
=======================================
+ Hits 8487 8492 +5
Misses 319 319
Partials 106 106
|
next-core Run #10761
Run Properties:
|
Project |
next-core
|
Branch Review |
steve/v3-use-brick-slots-transform
|
Run status |
Passed #10761
|
Run duration | 00m 28s |
Commit |
cf1f49c67d ℹ️: Merge 360d95cc487a11489e139e4ff04f6d38374199d7 into 6359f0cac1ff84b226f437c11e9e...
|
Committer | Shenwei Wang |
View all properties for this run ↗︎ |
Test results | |
---|---|
Failures |
0
|
Flaky |
0
|
Pending |
0
|
Skipped |
0
|
Passing |
16
|
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: 0
🧹 Outside diff range and nitpick comments (4)
packages/runtime/src/internal/secret_internals.ts (2)
Line range hint
93-104
: 建议为严格模式处理添加注释说明代码逻辑本身没有问题,但建议添加注释说明严格模式的行为和影响,以帮助其他开发者理解这个重要的变更。
const strict = isStrictMode(); + // 在严格模式下,除非明确指定 errorBoundary,否则直接使用原始配置 const output = await renderBrick( renderRoot, strict && !errorBoundary ? useBrick : { errorBoundary, ...useBrick, }, scopedRuntimeContext, rendererContext, [], {} );
Line range hint
79-124
: 架构建议:考虑添加迁移指南这些变更引入了重要的新特性(严格模式和 useBrick 中的转换支持),建议:
- 在文档中添加迁移指南,说明如何从旧版本升级
- 考虑添加运行时警告,帮助开发者识别潜在的兼容性问题
- 提供配置选项允许渐进式采用新特性
这将帮助下游用户平滑过渡到新版本。
packages/runtime/src/internal/Renderer.ts (2)
Line range hint
394-413
: 严格模式下的上下文定义处理逻辑已完善代码增加了对构件上下文定义的严格模式校验,这有助于提高代码质量和可维护性。在非严格模式下保持了向后兼容性,同时通过警告提示开发者注意潜在问题。
建议考虑在后续版本中将严格模式设为默认选项,以帮助用户更早发现潜在问题。
723-743
: useBrick 中的 transform 属性处理已优化代码新增了对 useBrick 中 transform 属性的特殊处理,保持了与 v2 版本的行为兼容性。在严格模式下会发出警告,建议使用 properties 替代 transform。
建议添加以下改进:
if (runtimeContext.inUseBrick) { // Keep v2 behavior for `useBrick`: treat `transform` as `properties`. const transform = (brickConf as { transform?: Record<string, unknown> }) .transform; if (transform) { + // 添加 deprecation 警告 + console.warn( + 'Warning: "transform" in useBrick is deprecated and will be removed in v4. ' + + 'Please use "properties" instead.' + ); warnAboutStrictMode( strict, "`useBrick.transform`", 'please use "properties" instead, check your useBrick:', brickConf ); if (!strict) { confProps = { ...brickConf.properties, ...transform, }; } } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
etc/runtime.api.md
(1 hunks)packages/runtime/src/internal/Renderer.ts
(2 hunks)packages/runtime/src/internal/interfaces.ts
(1 hunks)packages/runtime/src/internal/secret_internals.spec.ts
(3 hunks)packages/runtime/src/internal/secret_internals.ts
(2 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/runtime/src/internal/interfaces.ts
- packages/runtime/src/internal/secret_internals.spec.ts
🔇 Additional comments (4)
packages/runtime/src/internal/secret_internals.ts (3)
41-41
: 导入变更看起来没有问题!
新增的 isStrictMode
导入与严格模式的集成保持一致。
Line range hint 106-124
: 错误处理逻辑完善且合理
保留了对 placeholder 和 portal 节点的错误检查,同时集成了严格模式的处理,这种做法很好地平衡了向后兼容性和新功能的添加。
79-81
: 请验证运行时上下文的变更影响
这些变更改进了上下文管理:
- 添加了
inUseBrick
标志 - 使用空值合并运算符初始化状态存储映射
建议验证这些变更对现有代码的影响。
✅ Verification successful
Let me analyze the results:
inUseBrick
usage:
- Properly defined in interfaces.ts
- Used in setupRootRuntimeContext.ts for brick setup
- Used in Renderer.ts for conditional logic
- The new assignment in secret_internals.ts aligns with existing usage
- State store maps:
- Both maps are consistently initialized as new Maps across the codebase
- Used extensively in tests and core functionality
- The nullish coalescing operator ensures safe initialization
- No incompatible usage patterns found
I can now generate the final response:
运行时上下文的变更是安全的
这些变更与现有代码完全兼容:
inUseBrick
标志的使用符合接口定义和现有逻辑- 状态存储映射的初始化方式保持一致,空值合并运算符确保了安全性
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证 inUseBrick 标志的使用情况
# 预期:找到所有依赖此标志的代码位置
echo "检查 inUseBrick 的使用..."
rg "inUseBrick" -t ts
echo "检查可能受影响的状态存储映射使用..."
rg "tplStateStoreMap|formStateStoreMap" -t ts
Length of output: 10835
etc/runtime.api.md (1)
419-420
: 需要为新属性添加文档注释
inUseBrick
属性缺少文档注释来说明其用途和使用场景。建议添加注释以帮助开发者理解该属性的作用。
让我们验证该属性在代码库中的使用情况:
✅ Verification successful
Based on the search results, I can now generate the final response since I have enough context about how inUseBrick
is used in the codebase. The property is used in the rendering logic to handle transform properties differently when a brick is being used through useBrick
.
需要为 inUseBrick
属性添加文档注释
建议为 inUseBrick
属性添加以下文档说明:
- 该属性用于标识当前上下文是否在
useBrick
中执行 - 它影响渲染器对
transform
属性的处理方式,在useBrick
中保持 v2 的行为模式
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 描述:验证 inUseBrick 属性的使用情况
# 预期:找到该属性在渲染逻辑中的使用
# 测试:搜索属性的使用
rg "inUseBrick" -A 5
Length of output: 2892
依赖检查
组件之间的依赖声明,是微服务组件架构下的重要信息,请确保其正确性。
请勾选以下两组选项其中之一:
或者:
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG,请检查工作内容与提交信息是否相符,并在以下每组选项中都依次确认。
破坏性变更:
feat
作为提交类型。BREAKING CHANGE: 你的变更说明
。新特性:
feat
作为提交类型。问题修复:
fix
作为提交类型。杂项工作:
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动,例如修改注释、测试用例、开发文档等:
chore
,docs
,test
等作为提交类型。Summary by CodeRabbit
新功能
RuntimeContext
接口中添加了可选属性inUseBrick
,用于指示当前是否在使用某个砖块。错误修复
renderBrick
函数中的错误处理逻辑,确保在砖块配置中存在errorBoundary
时能正确记录错误并返回ErrorNode
。transform
属性的处理,以保持与版本 2 的向后兼容性。测试
useBrick
功能的测试,确保在严格模式和非严格模式下的属性和转换正确渲染。