-
Notifications
You must be signed in to change notification settings - Fork 11
feat(): support FN in createRoot with scope fragment #4781
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在运行时引入“隔离根”机制:为 RuntimeContext 增加可选的 isolatedRoot:symbol,并贯穿注册与解析流程。非 page 渲染创建并注册隔离函数,FN 全局在存在 isolatedRoot 时从隔离注册表读取。更新 StoryboardFunctionRegistry 工厂以接受并传递该参数,并补充相关单测与 API 声明。 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks (2 passed, 1 warning)❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. ✨ 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. Comment |
c6e9d17 to
b5fb749
Compare
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## v3 #4781 +/- ##
=======================================
Coverage 95.22% 95.22%
=======================================
Files 209 210 +1
Lines 9166 9175 +9
Branches 1764 1768 +4
=======================================
+ 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-isolated-functions
|
| Run status |
|
| Run duration | 00m 23s |
| 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.
Pull Request Overview
This PR implements support for isolated function scopes in fragment-scoped rendering contexts, allowing FN calls to be isolated per render root to prevent interference between different rendering instances.
- Adds isolated function registry for non-page root contexts
- Integrates isolated root support throughout the evaluation pipeline
- Implements automatic cleanup of isolated state on unmount
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| packages/runtime/src/internal/interfaces.ts | Adds isolatedRoot symbol to RuntimeContext |
| packages/runtime/src/internal/compute/getGeneralGlobals.ts | Updates FN global to use isolated functions when available |
| packages/runtime/src/internal/compute/evaluate.ts | Passes isolatedRoot to general globals |
| packages/runtime/src/internal/compute/IsolatedFunctions.ts | New module implementing isolated function registry |
| packages/runtime/src/createRoot.ts | Integrates isolated root creation and function registration |
| packages/runtime/src/createRoot.spec.ts | Adds test for functions in scope fragment |
| packages/runtime/src/StoryboardFunctionRegistry.ts | Updates factory to support isolatedRoot parameter |
| etc/runtime.api.md | Updates API documentation to reflect new isolatedRoot parameter |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
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
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/StoryboardFunctionRegistry.ts (1)
149-163: 修复 FN 分支:当 isolatedRoot 存在但注册表未命中时回退到全局 FN,并补充测试
- 验证:GeneralGlobalsOptions 已声明 isolatedRoot?: symbol(packages/runtime/src/internal/compute/getGeneralGlobals.ts)。
- 问题:getGeneralGlobals 中 "FN" 分支当前为
isolatedRoot ? isolatedFunctionRegistry.get(isolatedRoot) : storyboardFunctions—— 若 isolatedRoot 存在但 registry.get 返回 undefined,会导致 FN 返回 undefined 且不会回退到 storyboardFunctions(packages/runtime/src/internal/compute/getGeneralGlobals.ts)。- 覆盖缺失:未发现针对 registerIsolatedFunctions / 未注册回退 的单元测试(createRoot.spec.ts 有 FN 使用案例,但未覆盖“隔离覆盖全局”和“未注册回退”两条路径)。
建议:
- 在 packages/runtime/src/internal/compute/getGeneralGlobals.ts 将 "FN" 分支改为带回退的形式:
isolatedRoot ? (isolatedFunctionRegistry.get(isolatedRoot) ?? storyboardFunctions) : storyboardFunctions。- 补充单元测试:一)注册隔离函数时 FN 使用隔离实现;二)isolatedRoot 存在但未注册时 FN 回退到全局 storyboardFunctions(可在 createRoot.spec.ts 或新增 getGeneralGlobals.spec.ts 实现)。
🧹 Nitpick comments (13)
etc/runtime.api.md (2)
461-463: 新增 RuntimeContext.isolatedRoot?: symbol 定义正确,但建议补充文档注释与稳定性声明建议在源码处为该字段添加 TSDoc,描述“仅在 fragment/非 page 根下使用,用于隔离 FN 作用域;page 下应为 undefined”。便于后续维护者理解语义。
629-629: API Extractor 警告:FunctionCoverageSettings 未从入口导出建议在入口 index.ts/d.ts 导出该类型,避免消费者无法引用并清理 AE 警告。
packages/runtime/src/internal/interfaces.ts (1)
33-34: 为 RuntimeContext 新增 isolatedRoot?: symbol,建议补充 TSDoc类型选择 symbol 合理。建议增加简短注释以阐明仅 fragment 使用、page 为空。
appendI18nNamespace?: string; - isolatedRoot?: symbol; + /** 标识当前渲染根的隔离作用域:fragment 下为唯一 symbol;page 场景为 undefined */ + isolatedRoot?: symbol;packages/runtime/src/createRoot.spec.ts (1)
322-351: 新增“functions in scope fragment”单测覆盖核心路径,LGTM;建议补一条多根隔离性回归当前用例验证 fragment 下 FN 注入与调用。建议再补一个“两个 fragment 根互不干扰”的用例,确保函数不会串根泄漏。
可参考:
test("functions isolation across two fragment roots", async () => { const r1 = unstable_createRoot(document.createElement("div")); const r2 = unstable_createRoot(document.createElement("div")); await r1.render([{ brick: "p", properties: { textContent: "<% FN.a() %>" } }], { functions: [{ name: "a", source: "function a(){return 'A'}" }], }); await r2.render([{ brick: "p", properties: { textContent: "<% FN.b() %>" } }], { functions: [{ name: "b", source: "function b(){return 'B'}" }], }); expect(r1Container.innerHTML).toBe("<p>A</p>"); expect(r2Container.innerHTML).toBe("<p>B</p>"); r1.unmount(); r2.unmount(); });packages/runtime/src/internal/compute/getGeneralGlobals.ts (2)
10-10: 为 GeneralGlobals 引入 isolatedRoot 选项并导入注册表:方向正确,建议细化类型
- 引入 isolatedRoot 与注册表符合隔离设计。
- GeneralGlobalsOptions.storyboardFunctions 建议从 unknown 收紧为具体的 ReadonlyStoryboardFunctions 类型,减少调用端断言。
-export interface GeneralGlobalsOptions { +export interface GeneralGlobalsOptions { collectCoverage?: unknown; widgetId?: string; widgetVersion?: string; isolatedRoot?: symbol; app?: PartialMicroApp; appendI18nNamespace?: string; - storyboardFunctions?: unknown; + storyboardFunctions?: Readonly<Record<string, Function>>; isStoryboardFunction?: boolean; }Also applies to: 12-21, 41-53
58-61: FN 解析优先使用 isolatedRoot:建议增加安全兜底,避免未注册时抛 ReferenceError在极端情况下(例如 registerIsolatedFunctions 未写入或写入前的早期求值),
isolatedFunctionRegistry.get(isolatedRoot)可能为 undefined,从而导致 FN 缺失。建议兜底为空对象(或受控的只读空表),提升健壮性。- case "FN": - return isolatedRoot - ? isolatedFunctionRegistry.get(isolatedRoot) - : storyboardFunctions; + case "FN": + return isolatedRoot + ? isolatedFunctionRegistry.get(isolatedRoot) ?? Object.freeze({}) + : storyboardFunctions;请同时确认 registerIsolatedFunctions 在传入空数组时,确实会为该 isolatedRoot 写入一个空只读对象,而不是跳过写入。
packages/runtime/src/createRoot.ts (3)
105-106: 每个非 page 根使用唯一 Symbol 作为隔离标识:实现简洁,但建议加注释说明生命周期建议注明:该 symbol 在 root 生命周期内保持不变,用于跨多次 render 共享同一隔离命名空间。
- const isolatedRoot = scope === "page" ? undefined : Symbol("IsolatedRoot"); + // 每个 fragment 根唯一的隔离标识;在该根的整个生命周期内保持不变 + const isolatedRoot = scope === "page" ? undefined : Symbol("IsolatedRoot");
199-201: fragment 下注册隔离函数:确认覆盖“重复 render 时更新/清空”的语义目前对 undefined 使用空数组兜底是对的。请确认 registerIsolatedFunctions 是“替换”而非“增量合并”,以避免旧函数遗留。
可在 registerIsolatedFunctions 内显式清空旧映射后再写入,或在此处 render 前先
isolatedFunctionRegistry.set(isolatedRoot!, Object.freeze({}))再注册。
273-276: unmount 时清理注册表,避免泄漏:LGTM;建议补充防御日志(可选)可在开发模式下 console.debug 一条,用于排查未清理导致的潜在冲突。
packages/runtime/src/StoryboardFunctionRegistry.ts (1)
78-88: 为 isolatedRoot 参数补充简短 JSDoc 并标注可见性(@internal)建议在工厂函数参数处对 isolatedRoot 的用途做一句话说明(用于在 getGeneralGlobals 中解析隔离作用域的 FN),并标注 @internal 以避免被误用为公共 API。
packages/runtime/src/internal/compute/IsolatedFunctions.ts (3)
1-1: 将类型导入标记为 type-only 以避免多余运行时依赖仅在类型位置使用的导入应使用
import type,减少编译产物中的无用引入。-import { StoryboardFunction } from "@next-core/types"; +import type { StoryboardFunction } from "@next-core/types";
12-20: 注册顺序建议:先注册后暴露,消除潜在读到“半初始化”视图的窗口虽然当前调用序在同一 tick 内几乎没有风险,但更稳妥的顺序是先完成 register,再写入 registry,可避免外部在极端时序下读取到尚未注册完的 Proxy。
const { storyboardFunctions, registerStoryboardFunctions } = StoryboardFunctionRegistryFactory({ isolatedRoot }); - isolatedFunctionRegistry.set(isolatedRoot, storyboardFunctions); - registerStoryboardFunctions(functions); + registerStoryboardFunctions(functions); + isolatedFunctionRegistry.set(isolatedRoot, storyboardFunctions);
12-20: 提供对称的注销 API,集中清理职责createRoot 层据称会在卸载时删除映射,但在此模块提供一个轻量的
unregisterIsolatedFunctions能让职责更集中、调用更直观。export function registerIsolatedFunctions( isolatedRoot: symbol, functions: StoryboardFunction[] ): void { const { storyboardFunctions, registerStoryboardFunctions } = StoryboardFunctionRegistryFactory({ isolatedRoot }); isolatedFunctionRegistry.set(isolatedRoot, storyboardFunctions); registerStoryboardFunctions(functions); } + +export function unregisterIsolatedFunctions(isolatedRoot: symbol): void { + isolatedFunctionRegistry.delete(isolatedRoot); +}
📜 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(3 hunks)packages/runtime/src/StoryboardFunctionRegistry.ts(2 hunks)packages/runtime/src/createRoot.spec.ts(1 hunks)packages/runtime/src/createRoot.ts(5 hunks)packages/runtime/src/internal/compute/IsolatedFunctions.ts(1 hunks)packages/runtime/src/internal/compute/evaluate.ts(1 hunks)packages/runtime/src/internal/compute/getGeneralGlobals.ts(3 hunks)packages/runtime/src/internal/interfaces.ts(1 hunks)
🔇 Additional comments (5)
etc/runtime.api.md (1)
575-579: 确认:isolatedRoot 已正确透传且调用方兼容验证结果:StoryboardFunctionRegistryFactory 在对 getGeneralGlobals 的调用中传入了 isolatedRoot(packages/runtime/src/StoryboardFunctionRegistry.ts),主要调用点已兼容 — IsolatedFunctions 显式传入 isolatedRoot、WidgetFunctions 以具名对象传入 widgetId/widgetVersion、StoryboardFunctions 使用默认调用,测试文件也已适应。
packages/runtime/src/internal/compute/evaluate.ts (1)
518-526: 在 getGeneralGlobals 传入 isolatedRoot:实现对 fragment 根的 FN 隔离支持,LGTM改动小而关键,能在不影响 page 场景下启用隔离根的 FN 解析。
packages/runtime/src/createRoot.ts (2)
31-34: 引入 IsolatedFunctions 注册与注册表:LGTM
143-151: 将 isolatedRoot 写入 runtimeContext:LGTM这使 evaluate/getGeneralGlobals 能感知隔离根。
packages/runtime/src/internal/compute/IsolatedFunctions.ts (1)
12-20: 已验证:卸载时会清理 isolatedFunctionRegistry 映射createRoot.ts 在卸载路径调用了 isolatedFunctionRegistry.delete(isolatedRoot)(packages/runtime/src/createRoot.ts:273-274);registerIsolatedFunctions 在创建时注册映射(packages/runtime/src/createRoot.ts:200),isolatedRoot 在创建时生成(packages/runtime/src/createRoot.ts:105)。
依赖检查
组件之间的依赖声明,是微服务组件架构下的重要信息,请确保其正确性。
请勾选以下两组选项其中之一:
或者:
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG,请检查工作内容与提交信息是否相符,并在以下每组选项中都依次确认。
破坏性变更:
feat作为提交类型。BREAKING CHANGE: 你的变更说明。新特性:
feat作为提交类型。问题修复:
fix作为提交类型。杂项工作:
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动,例如修改注释、测试用例、开发文档等:
chore,docs,test等作为提交类型。Summary by CodeRabbit