-
Notifications
You must be signed in to change notification settings - Fork 11
feat(): createRoot supports useChildren #4735
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
本次变更为渲染系统引入了 `supportsUseChildren` 选项,涉及接口、函数签名和相关测试的调整。核心逻辑在于根据该选项决定是否对 bricks 结构进行 `replaceUseChildren` 预处理,并相应调整了工具函数与测试用例的输入输出格式。
## Changes
| 文件/路径分组 | 变更摘要 |
|-----------------------------------------------------------------------|------------------------------------------------------------------|
| etc/runtime.api.md<br>packages/runtime/src/createRoot.ts | `CreateRootOptions` 接口新增可选属性 `supportsUseChildren?: boolean`,`unstable_createRoot` 函数签名支持该选项,渲染时根据该选项调用 `replaceUseChildren`。 |
| packages/preview/src/bootstrap.ts | 移除了 `replaceUseChildren` 的导入及相关调用,创建 root 时传入 `supportsUseChildren: true`。 |
| packages/runtime/src/createRoot.spec.ts | 新增自定义元素 `MyBrick` 及两组测试,验证 `supportsUseChildren` 开关下的渲染行为差异。 |
| packages/utils/src/storyboard/index.ts | 新增 `export * from "./replaceUseChildren.js"`,对外暴露该工具方法。 |
| packages/utils/src/storyboard/replaceUseChildren.ts | `replaceUseChildren` 支持单个或数组输入,函数签名调整并添加注释说明。 |
| packages/utils/src/storyboard/replaceUseChildren.spec.ts | 测试用例重构为单个 `BrickConf` 对象输入输出,结构更清晰,验证转换逻辑。 |
| packages/preview/package.json | 移除 `test` 和 `test:ci` 脚本,删除 `@next-core/test-next` 和 `@next-core/utils` 开发依赖。 |Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
packages/preview/src/bootstrap.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@next-core/eslint-config-next' imported from /eslint.config.mjs packages/runtime/src/createRoot.spec.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@next-core/eslint-config-next' imported from /eslint.config.mjs packages/runtime/src/createRoot.tsOops! Something went wrong! :( ESLint: 9.29.0 Error [ERR_MODULE_NOT_FOUND]: Cannot find package '@next-core/eslint-config-next' imported from /eslint.config.mjs
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (8)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (7)
⏰ Context from checks skipped due to timeout of 90000ms (2)
✨ Finishing Touches
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. 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 (
|
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 introduces support for a new flag, supportsUseChildren, which enables conditional mutation of brick configurations via the replaceUseChildren utility. Key changes include updating the replaceUseChildren function to accept a single brick or an array of bricks, propagating the supportsUseChildren flag through createRoot and its tests, and updating the API docs accordingly.
Reviewed Changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/utils/src/storyboard/replaceUseChildren.ts | Updated to support single BrickConf or an array and improved comment. |
| packages/utils/src/storyboard/replaceUseChildren.spec.ts | Modified tests to reflect the union-type input for replaceUseChildren. |
| packages/utils/src/storyboard/index.ts | Re-exported the new replaceUseChildren utility. |
| packages/runtime/src/createRoot.ts | Introduced and propagated the supportsUseChildren option in createRoot. |
| packages/runtime/src/createRoot.spec.ts | Added tests to validate behavior when supportsUseChildren is enabled. |
| packages/preview/src/bootstrap.ts | Adjusted createRoot call to pass supportsUseChildren and removed manual calls. |
| etc/runtime.api.md | Updated API documentation to reflect the new supportsUseChildren flag. |
Comments suppressed due to low confidence (2)
packages/utils/src/storyboard/replaceUseChildren.ts:12
- [nitpick] Consider updating the function documentation and/or renaming the parameter 'input' to a more descriptive name such as 'brickOrBricks' to clearly reflect that the parameter accepts both a single BrickConf and an array.
export function replaceUseChildren(input: BrickConf | BrickConf[]) {
etc/runtime.api.md:590
- [nitpick] Ensure that the API documentation comments reflect the semantics of the new supportsUseChildren flag, detailing its effect on rendering behavior for consumers.
export function unstable_createRoot(container: HTMLElement | DocumentFragment, { portal: _portal, scope, unknownBricks, supportsUseChildren, unsafe_penetrate, }?: CreateRootOptions): {
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: 2
🧹 Nitpick comments (6)
packages/utils/src/storyboard/replaceUseChildren.ts (1)
12-19: 输入类型扩展良好,但可考虑空值保护函数现在支持单个
BrickConf,实现简洁。若调用方意外传入undefined/null,当前实现会抛异常;可在入口处添加早返回以提升健壮性:+ if (!input) return;packages/runtime/src/createRoot.ts (1)
48-52: 选项文档 OK,建议注明默认值建议在注释中明确指出缺省情况下为
false,方便使用者快速了解行为差异。packages/runtime/src/createRoot.spec.ts (2)
259-289: 断言过于脆弱,可考虑放宽匹配
expect(container.innerHTML).toBe("<my-brick></my-brick>");
JSDOM 对自定义元素的序列化实现没有稳定保证(未来版本可能包含空格或自闭合形式)。- 对
useBrick的深度相等断言,如果后续在replaceUseChildren中添加其他字段(如slot、iid),测试将因无关字段失败。建议:
- 对 HTML 使用
toContain("<my-brick")或firstElementChild?.tagName/nodeName断言。- 对
useBrick使用toMatchObject而非toEqual,只关心关键字段。- expect(container.innerHTML).toBe("<my-brick></my-brick>"); + expect(container.firstElementChild?.nodeName.toLowerCase()).toBe("my-brick"); - expect((container.firstElementChild as MyBrick).useBrick).toMatchObject({ + expect((container.firstElementChild as MyBrick).useBrick).toMatchObject({ brick: "div", properties: { textContent: "Hello Use Children" }, });
293-320: 对比用例与上一用例逻辑重复,可参数化“支持”与“不支持”两种情况仅在
supportsUseChildren选项和值断言上存在差异;可以通过it.each/test.each参数化,减少重复代码并提高可读性。示例(伪码):
test.each([ [true, "<my-brick></my-brick>", expect.any(Object)], [false, '<my-brick><div slot="[items]">Hello Use Children</div></my-brick>', undefined], ])("use children (supports=%s)", async (flag, expectedHtml, expectedUseBrick) => { const root = unstable_createRoot(container, flag ? { supportsUseChildren: true } : undefined); await root.render(…); expect(container.innerHTML).toBe(expectedHtml); expect((container.firstElementChild as MyBrick).useBrick).toEqual(expectedUseBrick); root.unmount(); });packages/utils/src/storyboard/replaceUseChildren.spec.ts (2)
124-186: 测试输入结构过于嵌套,可提取辅助函数当前输入对象层级深且代码行数多,阅读困难。可以通过构造器/工厂函数或
buildBrick()辅助方法简化测试数据,使重点放在useChildren→useBrick的转换逻辑,而非冗长的 JSON。示例:
function brick(name: string, extra: Partial<BrickConf> = {}): BrickConf { return { brick: name, ...extra }; } // then const brick = brick("brick-a", { properties: { useChildren: "[cell]" }, children: [ brick("brick-b", { slot: "[cell]", children: [ … ] }), ], });
188-246: 严格深度相等断言可能导致后续演进受阻与 runtime 测试类似,这里使用
toEqual对完整对象做深比较。若replaceUseChildren未来保留部分原字段(如slot)或新增元数据,测试会不必要失败。可改用toMatchObject并仅断言关键差异(useChildren被替换为useBrick,未知 slot 保留 / 被删除等)。- expect(brick).toEqual({ + expect(brick).toMatchObject({ properties: { useBrick: expect.any(Object), missed: { useChildren: "[missed]" }, }, });
📜 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 (7)
etc/runtime.api.md(2 hunks)packages/preview/src/bootstrap.ts(1 hunks)packages/runtime/src/createRoot.spec.ts(2 hunks)packages/runtime/src/createRoot.ts(4 hunks)packages/utils/src/storyboard/index.ts(1 hunks)packages/utils/src/storyboard/replaceUseChildren.spec.ts(1 hunks)packages/utils/src/storyboard/replaceUseChildren.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (3)
packages/utils/src/storyboard/replaceUseChildren.spec.ts (2)
packages/types/src/manifest.ts (1)
BrickConf(512-570)packages/utils/src/storyboard/replaceUseChildren.ts (1)
replaceUseChildren(12-20)
packages/runtime/src/createRoot.ts (2)
packages/utils/src/storyboard/replaceUseChildren.ts (1)
replaceUseChildren(12-20)packages/runtime/src/internal/RendererContext.ts (2)
bricks(327-373)bricks(375-389)
packages/runtime/src/createRoot.spec.ts (1)
packages/runtime/src/createRoot.ts (1)
unstable_createRoot(74-275)
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (3)
packages/utils/src/storyboard/index.ts (1)
4-4: 已成功将 replaceUseChildren 暴露为公共 API统一暴露的位置与其它工具函数保持一致,没有发现问题。
packages/preview/src/bootstrap.ts (1)
90-95: 预览环境强制启用 supportsUseChildren预览场景下始终开启
supportsUseChildren是合理的,不过如果后续引入不兼容旧数据的 YAML,可能导致线上与预览渲染行为不一致,请确认其它运行环境也采用相同默认值。etc/runtime.api.md (1)
117-124: API 报告文件由工具自动生成,无需手动评审Also applies to: 590-593
cdf49d7 to
4120bff
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/createRoot.spec.ts (1)
10-14: 避免自定义元素重复注册此处直接
customElements.define("my-brick", MyBrick);,多次执行测试(如jest --watch或不同文件重复 import)时,若同名元素已存在会抛出DOMException。建议在定义前先判断:-if (!customElements.get("my-brick")) { - customElements.define("my-brick", MyBrick); -} +if (!customElements.get("my-brick")) { + customElements.define("my-brick", MyBrick); +}
🧹 Nitpick comments (2)
packages/runtime/src/createRoot.spec.ts (2)
259-291: 缺少卸载后的断言,建议补充清理验证测试最后仅调用
root.unmount(),未像文件中其它用例一样断言container/portal已被清空,减少了对内存泄漏或残留 DOM 的保护。可追加与其他测试保持一致的校验:root.unmount(); + expect(container.innerHTML).toBe(""); + expect(portal.innerHTML).toBe("");
293-320: 未验证卸载后的 DOM 状态同上,用例结束后应确认
unmount成功清理 DOM,避免潜在残留:root.unmount(); + expect(container.innerHTML).toBe(""); + expect(portal.innerHTML).toBe("");
📜 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 (8)
etc/runtime.api.md(2 hunks)packages/preview/package.json(1 hunks)packages/preview/src/bootstrap.ts(1 hunks)packages/runtime/src/createRoot.spec.ts(2 hunks)packages/runtime/src/createRoot.ts(4 hunks)packages/utils/src/storyboard/index.ts(1 hunks)packages/utils/src/storyboard/replaceUseChildren.spec.ts(1 hunks)packages/utils/src/storyboard/replaceUseChildren.ts(1 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/preview/src/bootstrap.ts
🚧 Files skipped from review as they are similar to previous changes (5)
- packages/utils/src/storyboard/index.ts
- packages/utils/src/storyboard/replaceUseChildren.ts
- etc/runtime.api.md
- packages/runtime/src/createRoot.ts
- packages/utils/src/storyboard/replaceUseChildren.spec.ts
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: build (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (1)
packages/preview/package.json (1)
23-26: 脚本命令调整:移除 test/test:ci 并保留 build
已删除test与test:ci脚本,确保预览包的测试逻辑已迁移到runtime与utils包中,并且 CI 配置已指向新的测试位置。
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3 #4735 +/- ##
==========================================
- Coverage 95.22% 95.21% -0.02%
==========================================
Files 209 209
Lines 9075 9082 +7
Branches 1739 1743 +4
==========================================
+ Hits 8642 8647 +5
- Misses 321 323 +2
Partials 112 112
🚀 New features to boost your workflow:
|
next-core
|
||||||||||||||||||||||||||||
| Project |
next-core
|
| Branch Review |
steve/v3-use-children
|
| 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 ↗︎ | |
4120bff to
8313d69
Compare
8313d69 to
5ceb8ff
Compare
依赖检查
组件之间的依赖声明,是微服务组件架构下的重要信息,请确保其正确性。
请勾选以下两组选项其中之一:
或者:
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG,请检查工作内容与提交信息是否相符,并在以下每组选项中都依次确认。
破坏性变更:
feat作为提交类型。BREAKING CHANGE: 你的变更说明。新特性:
feat作为提交类型。问题修复:
fix作为提交类型。杂项工作:
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动,例如修改注释、测试用例、开发文档等:
chore,docs,test等作为提交类型。Summary by CodeRabbit
新功能
测试
文档
重构
维护