-
Notifications
You must be signed in to change notification settings - Fork 11
feat(): support native mode for storyboard functions #4571
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
Compute-intensive functions should use native mode
Walkthrough此次更改涉及多个文件的API报告更新,主要集中在 Changes
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/runtime/src/StoryboardFunctionRegistry.spec.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 (
|
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 (5)
packages/runtime/src/internal/compute/getGeneralGlobals.ts (1)
26-26: 参数类型扩展增强了函数的灵活性!将
attemptToVisitGlobals的类型从Set<string>扩展为Set<string> | string[]是一个很好的改进:
- 提高了函数的适用性,支持更多使用场景
- 避免了不必要的 Set 转换,可能带来性能提升
建议添加 JSDoc 注释说明参数类型的变化,以便其他开发者更好地理解。
+/** + * @param attemptToVisitGlobals 要访问的全局变量名称集合,支持 Set<string> 或 string[] 类型 + * @param options 全局变量选项配置 + */ export function getGeneralGlobals( attemptToVisitGlobals: Set<string> | string[], options: GeneralGlobalsOptions ): Record<string, unknown> {packages/supply/src/index.ts (1)
10-16: 建议重构以提高代码可读性和可维护性当前实现中对
undefined的特殊处理逻辑增加了代码的复杂度。建议将这个逻辑提取到一个单独的辅助函数中。建议按照以下方式重构:
const globalVariables: Record<string, unknown> = {}; + +function handleSpecialVariable(name: string): unknown | null { + if (name === "undefined") { + return undefined; + } + return null; +} + // Allow limited browser builtin values. for (const variableName of attemptToVisitGlobals) { if (!Object.prototype.hasOwnProperty.call(globalVariables, variableName)) { - if (variableName === "undefined") { - globalVariables[variableName] = undefined; + const specialValue = handleSpecialVariable(variableName); + if (specialValue !== null) { + globalVariables[variableName] = specialValue;packages/runtime/src/StoryboardFunctionRegistry.spec.ts (1)
158-158: 建议增强测试断言的覆盖范围当前的测试断言仅验证了基本功能,建议添加更多断言来确保原生模式的正确性。
建议添加如下断言:
- expect(fn.nativeMode()).toEqual(["from", "native", "mode"]); + // 测试原生模式的基本功能 + expect(fn.nativeMode()).toEqual(["from", "native", "mode"]); + + // 测试 transformed 模式下的行为 + global.document = undefined; + expect(fn.nativeMode()).toEqual(["from", "native", "mode"]); + + // 测试全局变量注入 + global.document = { title: "test" }; + expect(fn.nativeMode()).toBe(global.document);etc/runtime.api.md (1)
建议完善 RuntimeHooks 接口文档
根据代码分析,RuntimeHooks 接口确实缺乏完整的文档说明。在
etc/runtime.api.md和源代码中都只有接口定义,没有详细的使用说明。建议添加以下文档内容:
- 为每个钩子添加 JSDoc 注释,说明其功能和使用场景:
auth: 用户认证相关钩子checkPermissions: 权限检查相关钩子checkInstalledApps: 应用安装检查钩子flowApi: 流程 API 相关钩子menu: 菜单相关钩子messageDispatcher: 消息分发钩子pageView: 页面视图钩子- 补充钩子的调用时序说明
- 添加各个钩子的实现示例
🔗 Analysis chain
Line range hint
435-507: 建议完善 RuntimeHooks 接口文档RuntimeHooks 接口提供了丰富的扩展点,但缺少详细的使用说明。
建议添加以下文档内容:
- 每个钩子的具体用途和使用场景
- 钩子的调用时机和执行顺序
- 实现钩子时需要注意的事项
- 示例代码展示如何正确实现和使用这些钩子
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # 检查是否存在相关文档 fd -e md -e mdx "runtime.*hooks|hooks.*guide"Length of output: 47
Script:
#!/bin/bash # Search for any documentation about hooks in markdown files rg -l "hooks?" -g "*.md" -g "*.mdx" # Search for any documentation about runtime hooks in the codebase rg -i "runtime.*hooks?" -g "*.md" -g "*.mdx" -A 5 -B 5 # Check if there are any JSDoc comments for RuntimeHooks ast-grep --pattern 'interface RuntimeHooks { $$$ }'Length of output: 8216
packages/types/src/manifest.ts (1)
1368-1375: 建议完善 TransformedFunction 接口的文档
TransformedFunction接口的实现清晰明确,但建议补充以下文档说明:
source字段中编译后的 JS 代码的格式要求globals字段中全局对象的命名规范- 性能优化的最佳实践建议
建议添加如下文档:
/** * 编译后的函数 + * + * @remarks + * - source: 编译后的 JS 代码应当符合 ES5 标准以确保最大兼容性 + * - globals: 全局对象名称应当使用完整的命名空间路径 + * + * @example + * ```ts + * { + * source: "function(x,y){return x+y}", + * globals: ["Math", "console"] + * } + * ``` + */ export interface TransformedFunction { /** 编译后的 JS 代码 */ source: string; /** 要访问的全局对象列表 */ globals: string[]; }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (6)
etc/runtime.api.md(1 hunks)packages/runtime/src/StoryboardFunctionRegistry.spec.ts(2 hunks)packages/runtime/src/StoryboardFunctionRegistry.ts(8 hunks)packages/runtime/src/internal/compute/getGeneralGlobals.ts(1 hunks)packages/supply/src/index.ts(1 hunks)packages/types/src/manifest.ts(1 hunks)
🧰 Additional context used
🪛 Biome (1.9.4)
packages/runtime/src/StoryboardFunctionRegistry.ts
[error] 15-15: 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)
[error] 243-243: Don't use 'Function' as a type.
Prefer explicitly define the function shape. This type accepts any function-like value, which can be a common source of bugs.
(lint/complexity/noBannedTypes)
🔇 Additional comments (13)
packages/runtime/src/internal/compute/getGeneralGlobals.ts (1)
Line range hint 26-33: 验证类型变更的影响范围
需要确保所有调用此函数的地方都已适配新的参数类型。
✅ Verification successful
Let me check the actual type changes by looking at the git history.
Let me check the actual implementation and usage to verify the type compatibility.
Let me try a different approach to check the type definitions and usage.
参数类型变更已经得到正确处理
根据代码分析,getGeneralGlobals 函数的参数类型 attemptToVisitGlobals: Set<string> | string[] 在所有调用处都得到了正确的处理:
- 在
StoryboardFunctionRegistry.ts中传入globals参数 - 在
evaluate.ts中传入precooked.attemptToVisitGlobals参数 - 在测试文件
getGeneralGlobals.spec.ts中使用attemptToVisitGlobals参数
所有调用都符合函数签名要求的 Set<string> | string[] 类型。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索所有调用 getGeneralGlobals 的地方
rg "getGeneralGlobals\(" -A 2
Length of output: 2134
Script:
#!/bin/bash
# Check git history for the file
git log -p packages/runtime/src/internal/compute/getGeneralGlobals.ts | head -n 50
Length of output: 0
Script:
#!/bin/bash
# Check the implementation and usage
ast-grep --pattern 'getGeneralGlobals($attemptToVisitGlobals, $options)'
# Check the type definitions
rg "type.*GeneralGlobalsOptions" -A 3
Length of output: 114
Script:
#!/bin/bash
# Get the content of the file
cat packages/runtime/src/internal/compute/getGeneralGlobals.ts
# Check all imports of this file
rg "from.*getGeneralGlobals" -B 2
Length of output: 4511
packages/supply/src/index.ts (2)
6-6: 参数类型扩展增强了函数的灵活性!
通过支持 Set<string> 和 string[] 两种类型,提高了函数的可用性,同时保持了向后兼容性。
Line range hint 10-24: 验证变更对现有功能的影响
需要确保对全局变量处理的改动不会影响现有的功能调用。
✅ Verification successful
全局变量处理的改动已经过充分验证
根据代码分析结果,对 undefined 的处理是安全的:
supply函数被广泛使用在cook、precook和evaluate等核心功能中- 测试用例充分覆盖了全局变量的处理,包括
attemptToVisitGlobals的各种场景 - 对
undefined的处理是作为内置浏览器值的一部分,与其他全局变量处理保持一致 - 没有发现任何依赖于旧行为的代码
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 搜索代码库中对 supply 函数的调用,确保兼容性
rg -l "supply\(" | xargs rg -A 3 "supply\([^)]*\)"
# 检查是否有测试用例覆盖新的功能
rg -l "test|spec" | xargs rg "supply.*undefined"
Length of output: 2521
Script:
#!/bin/bash
# 检查 supply 函数的实现和测试用例
ast-grep --pattern 'function supply($_) {
$$$
}'
# 查找与 undefined 相关的测试用例
rg -B 2 -A 2 "undefined.*test|test.*undefined" packages/cook/src/cook.spec.ts
# 检查 attemptToVisitGlobals 的定义和使用
rg -l "attemptToVisitGlobals" | xargs rg -B 2 -A 2 "attemptToVisitGlobals"
Length of output: 27742
etc/runtime.api.md (2)
Line range hint 565-578: 检查 StoryboardFunctionRegistryFactory 的安全性
该工厂函数允许注入自定义的调试器覆盖,这可能带来潜在的安全风险。
建议:
- 添加运行时类型检查以验证注入的覆盖函数
- 考虑限制 debuggerOverrides 的使用范围
- 添加文档说明潜在的安全影响
✅ Verification successful
debuggerOverrides 的实现是安全的
根据代码分析,debuggerOverrides 的实现具有以下安全特性:
- 仅在调试模式下启用
- 作用域限定在特定函数内(precookFunction、cook、supply)
- 只允许覆盖有限的工具函数,如 Lodash
- 通过类型系统限制了可覆盖的范围
之前的安全顾虑是多余的,当前实现已经采取了足够的安全措施。
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查是否有其他地方使用了 debuggerOverrides
rg -l "debuggerOverrides"
Length of output: 151
Script:
#!/bin/bash
# 检查 StoryboardFunctionRegistry 中 debuggerOverrides 的具体实现和使用
ast-grep --pattern 'interface StoryboardFunctionRegistryFactory {
$$$
}'
# 检查测试文件中的使用场景
rg "debuggerOverrides" packages/runtime/src/StoryboardFunctionRegistry.spec.ts -A 5
# 检查实现文件中的使用场景
rg "debuggerOverrides" packages/runtime/src/StoryboardFunctionRegistry.ts -A 5
Length of output: 962
Line range hint 39-39: 关于废弃的认证相关函数
以下函数已被标记为废弃:
- authenticate()
- isLoggedIn()
- logout()
建议:
- 添加迁移指南,说明新的认证方式
- 在废弃的函数中添加警告消息,指向新的 API
- 考虑设置具体的移除时间表
Also applies to: 147-147, 156-156
✅ Verification successful
Let's gather more information about these deprecated functions to understand their implementation and any migration guidance.
Let's try a different approach to find the deprecation notices and implementation details.
认证相关函数已迁移至新包,需要更新导入路径
这些认证相关函数已经从 @next-core/runtime 迁移至 @next-core/easyops-runtime。废弃注释中已明确提供了迁移指南。
具体变更:
authenticate()、isLoggedIn()、logout()等函数需要从@next-core/easyops-runtime导入- 新包中的实现提供了完整的认证功能,包括用户信息管理、登录状态检查等
- 原包中的实现仅作为兼容层,通过 hooks 转发到新实现
建议:
- 所有使用这些函数的代码都应该更新导入路径
- 在后续版本中可以考虑完全移除旧包中的这些函数
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# 检查这些废弃函数的使用情况
rg -l "authenticate\(|isLoggedIn\(|logout\("
Length of output: 572
Script:
#!/bin/bash
# Check the implementation and deprecation notices of these functions
ast-grep --pattern 'export function authenticate' -A 10
ast-grep --pattern 'export function isLoggedIn' -A 10
ast-grep --pattern 'export function logout' -A 10
# Check for any migration guides or deprecation notices in comments
rg -A 5 "@deprecated.*(?:authenticate|isLoggedIn|logout)"
Length of output: 225
Script:
#!/bin/bash
# Search for the functions in auth.ts files with context
rg -A 10 "export (async )?function (authenticate|isLoggedIn|logout)" packages/*/src/auth.ts
# Search for deprecation comments with different patterns
rg -B 2 -A 2 "[@\*]\s*deprecated" packages/*/src/auth.ts
Length of output: 4073
packages/types/src/manifest.ts (1)
1360-1366: 代码变更符合性能优化目标
新增的 transformed 属性用于支持函数的原生模式执行,这与 PR 的性能优化目标相符。通过在构建时将函数转换为原生 JS 代码来提高执行性能是一个很好的优化方向。
packages/runtime/src/StoryboardFunctionRegistry.ts (7)
1-5: 导入 TransformedFunction 类型:改动正确
此改动正确地引入了 TransformedFunction 类型,以支持新功能。
25-25: 更新 StoryboardFunctionPatch 类型
新增了 "transformed" 属性,符合新功能需求。
58-58: 在 RuntimeStoryboardFunction 接口中添加 "transformed" 属性
新增可选属性 transformed?: TransformedFunction;,提升了对变换函数的支持。
75-75: 更新 PartialMicroApp 类型,新增 "config"
PartialMicroApp 类型中新增了 "config" 属性,扩展了微应用的配置信息支持。
139-139: 在函数注册中添加 "transformed" 属性
在 registeredFunctions 中正确地保存了 fn.transformed,确保变换后的函数信息被记录。
147-161: 新增 getGlobalVariables 函数
正确地封装了全局变量的获取逻辑,提高了代码的可读性和可维护性。
269-269: 在 updateStoryboardFunction 中添加 "transformed" 属性
在更新函数时包括了 transformed 属性,确保变换函数的信息被正确更新。
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## v3 #4571 +/- ##
=======================================
Coverage 95.22% 95.23%
=======================================
Files 206 206
Lines 8927 8935 +8
Branches 1702 1705 +3
=======================================
+ Hits 8501 8509 +8
Misses 319 319
Partials 107 107
|
next-core
|
||||||||||||||||||||||||||||
| Project |
next-core
|
| Branch Review |
steve/v3-native-function
|
| Run status |
|
| Run duration | 00m 28s |
| Commit |
|
| Committer | Shenwei Wang |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
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.
Copilot reviewed 5 out of 6 changed files in this pull request and generated no suggestions.
Files not reviewed (1)
- etc/runtime.api.md: Evaluated as low risk
Comments skipped due to low confidence (1)
packages/supply/src/index.ts:15
- The handling of
undefinedis redundant as it is already a built-in value. This line can be removed.
globalVariables[variableName] = undefined;
Compute-intensive functions should use native mode
依赖检查
组件之间的依赖声明,是微服务组件架构下的重要信息,请确保其正确性。
请勾选以下两组选项其中之一:
或者:
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG,请检查工作内容与提交信息是否相符,并在以下每组选项中都依次确认。
破坏性变更:
feat作为提交类型。BREAKING CHANGE: 你的变更说明。新特性:
feat作为提交类型。问题修复:
fix作为提交类型。杂项工作:
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动,例如修改注释、测试用例、开发文档等:
chore,docs,test等作为提交类型。Summary by CodeRabbit
transformed属性到StoryboardFunction接口,以支持编译函数,提高执行性能。nativeMode函数,扩展了故事板功能注册。