feat(): support segues #4510
feat(): support segues #4510
Conversation
WalkthroughThe changes in this pull request involve modifications to the rendering logic within the Changes
Poem
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 (
|
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (1)
packages/runtime/src/internal/fulfilStoryboard.ts (1)
51-93: Consider adding unit tests for the new functionsThe newly added functions
initializeSeguesForRoutesandinitializeSeguesForBricksare crucial for segue initialization. To ensure their correctness and prevent future regressions, it's advisable to add unit tests covering various scenarios.Would you like assistance in generating unit tests for these functions?
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- packages/runtime/src/internal/Renderer.ts (1 hunks)
- packages/runtime/src/internal/fulfilStoryboard.ts (2 hunks)
- packages/types/src/manifest.ts (1 hunks)
🧰 Additional context used
🪛 GitHub Check: CodeQL
packages/runtime/src/internal/fulfilStoryboard.ts
[failure] 118-118: Incomplete string escaping or encoding
This does not escape backslash characters in the input.
🪛 Biome
packages/runtime/src/internal/fulfilStoryboard.ts
[error] 105-105: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
[error] 107-107: The assignment should not be in an expression.
The use of assignments in expressions is confusing.
Expressions are often considered as side-effect free.(lint/suspicious/noAssignInExpressions)
🔇 Additional comments (2)
packages/types/src/manifest.ts (1)
929-929: LGTM: New segue action addedThe addition of the "segue.go" action to the
BuiltinBrickEventHandlerinterface aligns with the PR objective of introducing support for segues. This change expands the event handling capabilities without affecting existing functionality.Let's verify the implementation of this new action:
packages/runtime/src/internal/Renderer.ts (1)
353-353: Correctly passing 'iid' to control brick configurationThe addition of
iid: brickConf.iidensures that theiidproperty is included when rendering control nodes. This enhances the tracking and management of brick instances and aligns with the intended functionality.
| ? `\${${param.value.replace(/^\s*<%[~=]?\s|\s%>\s*$/g, "")}}` | ||
| : String(param.value).replace(/`/g, "\\`") |
There was a problem hiding this comment.
Escape backslashes to prevent potential string issues
The current implementation only escapes backticks in the string but does not escape backslashes. This could lead to incorrect string representations if param.value contains backslashes. Consider escaping backslashes as well.
Apply this diff to address the string escaping:
? `\${${param.value.replace(/^\s*<%[~=]?\s|\s%>\s*$/g, "")}}`
- : String(param.value).replace(/`/g, "\\`")
+ : String(param.value).replace(/\\/g, "\\\\").replace(/`/g, "\\`")📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| ? `\${${param.value.replace(/^\s*<%[~=]?\s|\s%>\s*$/g, "")}}` | |
| : String(param.value).replace(/`/g, "\\`") | |
| ? `\${${param.value.replace(/^\s*<%[~=]?\s|\s%>\s*$/g, "")}}` | |
| : String(param.value).replace(/\\/g, "\\\\").replace(/`/g, "\\`") |
🧰 Tools
🪛 GitHub Check: CodeQL
[failure] 118-118: Incomplete string escaping or encoding
This does not escape backslash characters in the input.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: Copilot Autofix powered by AI <62310815+github-advanced-security[bot]@users.noreply.github.com>
weareoutman
force-pushed
the
steve/v3-segue
branch
from
af5e88b to
0a9df55
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## v3 #4510 +/- ##
==========================================
- Coverage 95.21% 94.91% -0.31%
==========================================
Files 205 207 +2
Lines 8860 9081 +221
Branches 1682 1729 +47
==========================================
+ Hits 8436 8619 +183
- Misses 319 355 +36
- Partials 105 107 +2
|
next-core
|
||||||||||||||||||||||||
| Project |
next-core
|
| Run status |
|
| Run duration | 00m 26s |
| Commit |
|
| Committer | Shenwei Wang |
| View all properties for this run ↗︎ | |
| Test results | |
|---|---|
Failures
|
0
|
Flaky
|
0
|
Pending
|
0
|
Skipped
|
0
|
Passing
|
16
|
依赖检查
组件之间的依赖声明，是微服务组件架构下的重要信息，请确保其正确性。
请勾选以下两组选项其中之一：
或者：
提交信息检查
Git 提交信息将决定包的版本发布及自动生成的 CHANGELOG，请检查工作内容与提交信息是否相符，并在以下每组选项中都依次确认。
破坏性变更：
feat作为提交类型。BREAKING CHANGE: 你的变更说明。新特性：
feat作为提交类型。问题修复：
fix作为提交类型。杂项工作：
即所有对下游使用者无任何影响、且没有必要显示在 CHANGELOG 中的改动，例如修改注释、测试用例、开发文档等：
chore,docs,test等作为提交类型。Summary by CodeRabbit
New Features
"segue.go"for improved navigation handling.Improvements
Bug Fixes