-
Notifications
You must be signed in to change notification settings - Fork 11
feat(): support app preview #4808
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
|
Note Other AI code review bot(s) detectedCodeRabbit has detected other AI code review bot(s) in this pull request and will avoid duplicating their findings in the review comments. This may lead to a less comprehensive review. Walkthrough本次变更:在 runtime API 报告与内部导出中引入并暴露未文档化的 Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Pre-merge checks and finishing touches❌ Failed checks (2 warnings)
✅ Passed checks (1 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration 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 ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
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 |
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 app preview functionality by adding conditional runtime creation and an AppPreviewer class that enables live preview capabilities. The feature is activated through the _experimental_app_preview_ URL parameter.
- Added conditional runtime creation based on preview mode
- Implemented AppPreviewer class with setup, update, and navigation methods
- Exported getBrickPackages function to support preview functionality
Reviewed Changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| packages/runtime/src/internal/secret_internals.ts | Exports getBrickPackages function for external access |
| packages/brick-container/src/bootstrap.ts | Implements app preview functionality with conditional bootstrap logic |
| etc/runtime.api.md | Updates API documentation to include getBrickPackages export |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| history.replaceState(null, "", url); | ||
| const runtime = doCreateRuntime(); | ||
| await loadCheckLogin(); | ||
| await runtime.bootstrap(data as any); |
Copilot
AI
Oct 14, 2025
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.
Avoid using 'as any' type assertion. Consider properly typing the data parameter or using a more specific type assertion.
| await runtime.bootstrap(data as any); | |
| await runtime.bootstrap(data); |
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: 1
📜 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 (3)
etc/runtime.api.md(2 hunks)packages/brick-container/src/bootstrap.ts(4 hunks)packages/runtime/src/internal/secret_internals.ts(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build (20.x)
- GitHub Check: Analyze (javascript-typescript)
| function doCreateRuntime() { | ||
| return createRuntime({ | ||
| hooks: { | ||
| auth, | ||
| checkPermissions, | ||
| flowApi, | ||
| checkInstalledApps, | ||
| menu, | ||
| images: { imagesFactory, widgetImagesFactory }, | ||
| messageDispatcher, | ||
| ...(isAppPreview | ||
| ? null | ||
| : { | ||
| fulfilStoryboard, | ||
| pageView: analytics.pageView, | ||
| }), | ||
| }, | ||
| }); |
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.
修复 ...null 展开导致的运行时异常
当 _experimental_app_preview_ 参数存在时,isAppPreview 为 true,表达式 ...(isAppPreview ? null : { … }) 会在对象字面量内尝试展开 null,从而抛出 TypeError: Cannot convert undefined or null to object,预览模式在创建 runtime 之前即崩溃。请将条件分支改为展开空对象,或在展开前先行判空。
- ...(isAppPreview
- ? null
- : {
- fulfilStoryboard,
- pageView: analytics.pageView,
- }),
+ ...(isAppPreview
+ ? {}
+ : {
+ fulfilStoryboard,
+ pageView: analytics.pageView,
+ }),🤖 Prompt for AI Agents
In packages/brick-container/src/bootstrap.ts around lines 124 to 141, the object
spread uses ...(isAppPreview ? null : { … }) which throws when isAppPreview is
true because null cannot be spread; replace the conditional to spread an empty
object instead (e.g. ...(isAppPreview ? {} : { fulfilStoryboard, pageView:
analytics.pageView })) or restructure to conditionally add the properties only
when isAppPreview is false, ensuring no null/undefined is ever passed to the
object spread.
next-core
|
||||||||||||||||||||||||||||
| Project |
next-core
|
| Branch Review |
steve/v3-app-preview
|
| Run status |
|
| Run duration | 00m 26s |
| Commit |
|
| Committer | Shenwei Wang |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
0
|
|
|
17
|
| View all changes introduced in this branch ↗︎ | |
0858f23 to
34186b9
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: 1
🧹 Nitpick comments (3)
packages/yo/src/plopfile.js (3)
421-421: 移除注释掉的代码。第 421 行的注释代码应该删除,保持代码整洁。
应用此差异删除注释代码:
let settings = {}; if (existsSync(settingsJson)) { - // settings = JSON.parse(await readFile(settingsJson, "utf-8")); const errors = [];
432-443: 考虑更谨慎地合并设置,避免覆盖用户自定义配置。当前使用
Object.assign会直接替换jest.runMode、jest.jestCommandLine等属性。如果用户已经自定义了这些设置,它们将会被覆盖丢失。建议更智能地合并配置,例如:
- 只在属性不存在时才添加
- 或者提供选项让用户决定是否覆盖
参考实现:
- Object.assign(settings, { - "jest.runMode": "on-demand", - "jest.jestCommandLine": "npx test-next", - "jest.useJest30": true, - "jest.nodeEnv": { - NODE_ENV: "test", - }, - "jest.virtualFolders": folders.map((folder) => ({ - name: folder, - rootPath: folder, - })), - }); + // Merge settings, only set if not already defined by user + settings["jest.runMode"] ??= "on-demand"; + settings["jest.jestCommandLine"] ??= "npx test-next"; + settings["jest.useJest30"] ??= true; + settings["jest.nodeEnv"] ??= { NODE_ENV: "test" }; + // Always update virtualFolders to reflect current workspace structure + settings["jest.virtualFolders"] = folders.map((folder) => ({ + name: folder, + rootPath: folder, + }));
444-444: 考虑为文件写入操作添加错误处理。
writeFile操作可能因权限问题或磁盘空间不足而失败。虽然当前函数的调用者可能会处理错误,但在此处添加 try-catch 可以提供更友好的错误消息。参考实现:
try { await writeFile(settingsJson, JSON.stringify(settings, null, 2) + "\n"); } catch (error) { return `Failed to write .vscode/settings.json: ${error.message}`; }
📜 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 ignored due to path filters (1)
yarn.lockis excluded by!**/yarn.lock,!**/*.lock
📒 Files selected for processing (2)
packages/yo/package.json(1 hunks)packages/yo/src/plopfile.js(4 hunks)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (2)
- GitHub Check: build (20.x)
- GitHub Check: Analyze (javascript-typescript)
🔇 Additional comments (5)
packages/yo/package.json (1)
24-24: LGTM!依赖添加正确,版本 3.3.1 是当前最新稳定版本,使用
^范围符合最佳实践。根据学习资料
packages/yo/src/plopfile.js (4)
3-5: LGTM!导入声明正确,添加了新功能所需的文件操作和 JSONC 解析功能。
127-130: LGTM!新增的 VSCode 测试设置选项遵循现有模式,实现正确。
395-397: LGTM!处理逻辑正确,与其他类型的处理保持一致。
407-488: 删除关于 rootDir 变量扩展的误导性评论
代码中使用的模板字符串会在运行时计算${rootDir}/${folder}并写入实际路径,无需额外处理。Likely an incorrect or invalid review comment.
0aff1b2 to
0c9ae17
Compare
依赖检查
组件之间的依赖声明,是微服务组件架构下的重要信息,请确保其正确性。
请勾选以下两组选项其中之一:
或者:
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG,请检查工作内容与提交信息是否相符,并在以下每组选项中都依次确认。
破坏性变更:
feat作为提交类型。BREAKING CHANGE: 你的变更说明。新特性:
feat作为提交类型。问题修复:
fix作为提交类型。杂项工作:
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动,例如修改注释、测试用例、开发文档等:
chore,docs,test等作为提交类型。Summary by CodeRabbit
新功能
重构
变更
Chores