Fix router page placeholder and useblock error, switch block render error #990
Conversation
…s lost accidentally
…ndering with error pageContext, result in page render error
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (3)
Please check the settings in the CodeRabbit UI or the You can disable this status message by setting the WalkthroughThis pull request introduces modifications across several files in the canvas rendering and routing system. The changes primarily focus on refining context and state management, particularly in how pages are rendered and how their schemas are handled. Key modifications include updates to page context logic, routing view placeholders, schema setting mechanisms, and the introduction of an Changes
Sequence DiagramsequenceDiagram
participant Renderer
participant PageContext
participant SchemaManager
participant RouterView
Renderer->>PageContext: Set context based on props
PageContext-->>Renderer: Return active page context
Renderer->>SchemaManager: Update schema and methods
SchemaManager-->>Renderer: Confirm schema update
Renderer->>RouterView: Render with updated context
Possibly related PRs
Suggested labels
Suggested reviewers
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 (
|
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/canvas/render/src/render.ts (1)
251-251: Consider verifying the newkeyusage for sub-routes/pages.
Using the entireancestorsarray as the key value could be too broad. Ifancestorschanges frequently, it might cause unnecessary unmounts/remounts for child components.Also applies to: 255-255
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
packages/canvas/render/src/RenderMain.ts(4 hunks)packages/canvas/render/src/builtin/CanvasRouterView.vue(1 hunks)packages/canvas/render/src/material-function/page-getter.ts(0 hunks)packages/canvas/render/src/page-block-function/schema.ts(2 hunks)packages/canvas/render/src/render.ts(2 hunks)packages/plugins/block/src/composable/useBlock.js(2 hunks)
💤 Files with no reviewable changes (1)
- packages/canvas/render/src/material-function/page-getter.ts
✅ Files skipped from review due to trivial changes (1)
- packages/canvas/render/src/builtin/CanvasRouterView.vue
🔇 Additional comments (9)
packages/canvas/render/src/page-block-function/schema.ts (2)
33-33: Ensure correct handling of the optionalpageId.
This new parameter could introduce subtle issues if it’s not consistently passed or validated.
76-76: Confirm thatpageIdis correctly utilized when setting page-level CSS.
SincepageIdis optional, confirm no null or undefined usage insetPagecss.packages/canvas/render/src/RenderMain.ts (5)
141-142: Verify the condition for usingactivePageContext.
Replacing!pageIdFromPathwithprops.entrymay alter how page contexts are determined. Ensure this change aligns with all scenarios, especially nested or child pages.
146-148: Check the updated page activation logic.
The new assignments topageContext.pageIdandpageContext.activerely on eitherprops.pageIdor the absence ofpageIdFromPath. Confirm that these conditions correctly reflect the intended active page.Also applies to: 158-158
169-177: Verify the dynamicuseSchemacalls for nested contexts.
Here we differentiate between “active” and “inactive” schema references. Confirm that re-initializing schema, state, and methods does not conflict with existing contexts.Also applies to: 185-185
202-202: Validate method updates in watch handler.
The watch onschema.methodscallssetCurrentMethodwith a forced re-initialization. Make sure no unexpected side effects occur with nested or shared methods.
209-224: Review the watcher forprops.activeandprops.renderSchema.
Whenactivebecomes false, the call tosetCurrentSchema(renderSchema, props.pageId)might override the current schema unexpectedly. Check if this is intentional or if additional guard conditions are needed.Also applies to: 225-225, 227-229
packages/canvas/render/src/render.ts (1)
232-235: Nice addition ofancestors.
Injecting and exposingancestorsprovides valuable context for hierarchical rendering. Ensure thatancestorsupdates do not trigger excessive re-renders.Also applies to: 239-239
packages/plugins/block/src/composable/useBlock.js (1)
34-34: Confirm references to the updated publish topic.
Renaming the publish topic fromhistoryChangedtolocationHistoryChangedmay require updates in other files to avoid broken subscriptions.
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
能正常切换到区块
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Release Notes
New Features
Bug Fixes
Refactor
Chores