-
Notifications
You must be signed in to change notification settings - Fork 376
feat: preview support auto update #1261
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
feat: preview support auto update #1261
Conversation
WalkthroughThe pull request introduces extensive modifications to the preview functionality. In the core preview script, several legacy utilities have been removed and replaced with new functions for schema management, dependency gathering, and message communication between the preview window and its parent. The changes span updates in component lifecycles, new Vue composition hooks for handling preview data, and API refinements for fetching schema and page details. Additionally, dependency adjustments in package manifests and streamlined preview invocations in plugin components further consolidate the preview flow. Changes
Sequence Diagram(s)sequenceDiagram
participant P as Preview Component
participant PC as Preview Communication Module
participant PD as Preview Data Module
participant PW as Parent Window
P->>PC: initCommunication()
PC->>PW: sendReadyMessage() & start heartbeat
PW-->>PC: PostMessage ("schema" data)
PC->>P: Trigger onSchemaReceived
P->>PD: loadInitialData & updateUrl
PD-->>P: Preview updated with new schema
Possibly related PRs
Poem
📜 Recent review detailsConfiguration used: CodeRabbit UI 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (3)
⏰ Context from checks skipped due to timeout of 90000ms (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. 🪧 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: 3
🧹 Nitpick comments (19)
packages/design-core/src/preview/src/preview/generate.js (1)
153-166
: Enhanced CSS dependency handling with better error checkingThe updated
processAppJsCode
function now includes proper validation and duplicate prevention, which improves robustness and efficiency.The implementation:
- Adds validation to check if
cssList
is a valid array- Returns original code early if no CSS files need processing
- Prevents duplicate CSS inclusions by checking if they already exist in the code
This effectively prevents potential issues with duplicate CSS imports while handling edge cases better.
packages/design-core/src/preview/src/Toolbar.vue (1)
35-46
: Better breadcrumb handling using Vue reactivityThis is a good enhancement that uses Vue's reactivity system to update breadcrumbs based on state changes.
The watch-based approach is more aligned with Vue's design patterns than the previous direct method. It automatically updates the breadcrumb whenever the current page changes, providing a more maintainable and reactive solution.
packages/design-core/src/preview/src/preview/usePreviewCommunication.ts (4)
1-4
: Use more specific types for better safety.Currently, the
onSchemaReceived
callback accepts adata: any
parameter. Relying onany
weakens type-safety and could mask potential bugs. Consider introducing a more explicit interface or type for the incoming schema payload to ensure better compile-time validations.
6-6
: Variable naming clarity.Using
onSchemaReceivedAction
is workable but consider a simpler name likeonSchemaReceivedHandler
oronSchemaChange
for clarity.
24-43
: Monitor potential performance overhead with frequent heartbeats.The heartbeat runs every second and attempts up to ten reconnection attempts. This is likely fine, but be aware if multiple parallel previews ever occur, as frequent retries might impact performance.
77-88
: Avoid shared global state if multiple previews could coexist.The composable currently stores
onSchemaReceivedAction
andloadInitialData
in top-level variables, which new calls could overwrite. For single-instance use, it’s fine, but if you ever need multiple concurrent previews, consider scoping these to instance-level variables within the composable.packages/design-core/src/preview/src/preview/Preview.vue (1)
76-76
: Refine the deferred assignment pattern.
cleanupCommunicationAction = cleanupCommunication
is clear once you get used to it. However, returning a named result directly fromusePreviewCommunication
and assigning it in a single destructuring step might reduce confusion.- const { initCommunication, cleanupCommunication } = usePreviewCommunication({ ... }) - cleanupCommunicationAction = cleanupCommunication + const { + initCommunication, + cleanupCommunication: cleanupCommunicationAction + } = usePreviewCommunication({ ... })packages/plugins/page/src/composable/usePage.ts (1)
468-479
: Ensure robust error handling for page fetching.
While this function handles concurrency viaPromise.all
, consider adding a top-level try/catch or logging mechanism to handle potential rejections fromfetchPageDetailIfNeeded
gracefully.packages/common/js/preview.js (5)
33-48
: Optimize merging of scripts and styles.
The approach is straightforward, but consider usingSet
or a uniqueness check to avoid duplicates if extended in the future.
90-100
: Add safety check before posting messages.
EnsurepreviewWindow
is still valid and not null before callingpostMessage
, to avoid runtime errors if the preview window closes unexpectedly.
102-120
: Validate event origin carefully.
Though there's a domain check, verifying the exact origin or whitelisting legitimate subdomains can further enhance security, preventing malicious messaging.
237-261
: Ensure consistent handling of environment-based preview URL.
The logic for dev vs. production paths is valid. Keep an eye on edge cases wherehref
might be truncated or unusual.
264-266
: Consolidate repeated logic if needed.
previewPage
largely mirrorsopen(...)
. If usage diverges, keep them separate. Otherwise, consider merging for simplicity.packages/design-core/src/preview/src/preview/usePreviewData.ts (6)
47-86
: Avoid frequentreplaceState
calls.
Revising URL parameters on every minor script/style change can be expensive. Consider deferring updates or using a throttle if these changes occur repeatedly.
98-153
: History support logic is robust.
When exploringhistory
, you fetch the correct version of the page or block. Check boundary conditions when no matchinghistory
is found in arrays.
175-204
: Generate code for pages and blocks.
getPageAncestryFiles
elegantly reusesgeneratePageCode
. Remember to handle edge cases whencomponentsMap
is missing or invalid.
299-325
: Transforming script blocks with Babel.
Replacing<script setup>
sections with compiled code is well-structured. Watch for performance overhead on large files.
327-379
: Consolidate or cache block lookups.
updatePreview
fetches nested blocks for each ancestor. If performance becomes an issue, consider caching these results.
381-390
: Loading initial data for preview.
Graceful usage: you setpreviewState
and immediately update. Potentially confirm if partial load states or errors need separate handling.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
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
(3 hunks)packages/toolbars/preview/src/Main.vue
(2 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/toolbars/preview/src/Main.vue (1)
Learnt from: yy-wow
PR: opentiny/tiny-engine#850
File: packages/toolbars/preview/src/Main.vue:16-16
Timestamp: 2025-03-28T12:31:55.927Z
Learning: In `packages/toolbars/preview/src/Main.vue`, within the `preview` function, the `getMergeMeta` method is used at lines 64 and 65 to retrieve `engine.config` configurations.
🧬 Code Definitions (2)
packages/plugins/page/src/composable/usePage.ts (1)
packages/common/js/preview.js (1)
currentPage
(73-73)
packages/design-core/src/preview/src/preview/usePreviewData.ts (3)
packages/design-core/src/preview/src/preview/http.js (12)
getPageById
(48-48)getPageById
(48-48)fetchPageHistory
(50-51)fetchPageHistory
(50-51)getBlockById
(49-49)getBlockById
(49-49)fetchImportMap
(39-42)fetchImportMap
(39-42)fetchAppSchema
(44-44)fetchAppSchema
(44-44)fetchMetaData
(31-37)fetchMetaData
(31-37)packages/design-core/src/preview/src/preview/importMap.js (2)
getImportMap
(31-38)getImportMap
(31-38)packages/design-core/src/preview/src/preview/generate.js (3)
res
(158-158)processAppJsCode
(153-167)processAppJsCode
(153-167)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (43)
packages/design-core/package.json (1)
107-107
: Addition of TypeScript types for Babel core is appropriate.The addition of
@types/babel__core
as a dev dependency is consistent with the existing Babel dependencies in the project. This will provide improved TypeScript type checking and IDE support when working with Babel-related code.packages/common/package.json (1)
42-42
: Good adjustment: Moving @vueuse/core to dependenciesMoving
@vueuse/core
from devDependencies to dependencies is appropriate since it appears to be used at runtime in the preview functionality, not just during development.packages/design-core/src/preview/src/preview/importMap.js (1)
31-35
: Improved function signature with direct parameterChanging the
getImportMap
function to accept ascripts
parameter directly is a good improvement over using URL parameters. This creates a more explicit API and decouples the function from the URL parsing logic.packages/design-core/src/preview/src/preview/http.js (1)
48-51
: Well-structured API endpoints for preview dataThe addition of these three API functions (
getPageById
,getBlockById
, andfetchPageHistory
) provides a clean interface for retrieving preview-related data. These endpoints follow a consistent pattern and properly utilize the existinggetMetaApi
utility.packages/plugins/block/src/BlockSetting.vue (3)
91-91
: Import statement cleanupThe unused imports have been removed, which improves code cleanliness.
93-93
: Updated preview function importThe import for the
previewPage
function aligns with the changes in the function usage below.
248-259
:❓ Verification inconclusive
Simplified preview function with cleaner API
The
previewHistory
function has been simplified to use a more direct approach with thepreviewPage
function.Please ensure all consumers of this updated preview function are aware of the parameter structure changes. Let's verify other usage of this function:
🏁 Script executed:
#!/bin/bash # Search for other usages of previewPage to ensure all callers provide the required parameters rg -A 2 'previewPage\(' --type vue --type jsLength of output: 74
Action Required: Validate All Consumers of the Updated
previewPage
FunctionThe simplified
previewHistory
function now directly invokespreviewPage
with a cleaner API. Please ensure that all consumers of this updated function provide the required parameters and adjust their calls if needed.To verify this, run the following updated command which includes Vue files by registering them with ripgrep:
#!/bin/bash # Search for all usages of previewPage in JS and Vue files rg --type-add "vue:*.vue" -A 2 'previewPage\('If this search reveals any usages that do not conform to the new parameter structure, please update those call sites accordingly.
packages/design-core/src/preview/src/Toolbar.vue (1)
17-23
: Added necessary imports for reactive breadcrumb updatesNew imports support the reactive breadcrumb update approach implemented below.
packages/plugins/page/src/PageHistory.vue (1)
54-63
: Streamlined preview history implementationThe
previewHistory
function has been simplified using the spread operator to include all item properties and focusing only on the essential parameters.The changes align with similar modifications in other files, creating a more consistent preview approach across the application. The addition of the
true
parameter topreviewPage
appears to be part of a coordinated update across multiple components.packages/design-core/src/preview/src/preview/usePreviewCommunication.ts (4)
20-23
: Single-instance usage looks good.Defining
heartbeatTimer
,retryTimes
, andloadInitialData
as top-level variables works well for this single-window scenario. No immediate issues noted.
44-55
: Graceful fallback when no opener is found.If
window.opener
doesn’t exist, you callloadInitialData()
to initialize the preview. This fallback approach looks sensible and helps prevent blocking the preview flow.
57-61
: Robust cleanup logic.Removing the message event listener and clearing the heartbeat timer prevent memory leaks. This is a proper cleanup routine.
63-75
: Conditional heartbeat usage is well-handled.Skipping the heartbeat when
isHistory
is detected avoids unnecessary overhead for historical previews. Good approach.packages/toolbars/preview/src/Main.vue (2)
14-15
: Clean and focused imports.The updated imports remove unnecessary references. Keeping only
previewPage
,useLayout
,useNotify
, andgetOptions
makes the code simpler and more readable.
61-61
:✅ Verification successful
Confirm removal of block-specific preview logic.
Switching entirely to
previewPage()
may drop the block preview functionality if that was previously needed. Verify whether removingpreviewBlock
is acceptable or if you still require block-level previews elsewhere.
🏁 Script executed:
#!/bin/bash # Searches for potential references to previewBlock across the repo fd -e vue -e ts -e js -e tsx -e jsx | xargs rg -A 3 "previewBlock"Length of output: 66
Confirmation: Removal of block-specific preview logic is acceptable.
After re-verifying the repository with an additional comprehensive search for
"previewBlock"
, no references were found anywhere in the codebase. This strongly suggests that the block-level preview functionality has been fully removed and that switching entirely topreviewPage()
is appropriate. Please ensure that any potential edge cases relying on block-level previews have been manually checked, but based on our search, the removal appears safe.packages/design-core/src/preview/src/preview/Preview.vue (6)
17-17
: Good modular design.The introduction of
usePreviewCommunication
andusePreviewData
fosters a clear separation of concerns for data handling and communication logic.Also applies to: 20-21
55-56
: Verify default theme fallback.Setting
data-theme
to'light'
if the query parameter is missing is sensible. Just confirm that'light'
is the correct default choice for your application.
58-58
: Clear composable usage.Destructuring
loadInitialData
,updateUrl
, andupdatePreview
from the same hook makes the code more maintainable.
60-69
: Timely cleanup for history previews.Calling
cleanupCommunicationAction()
immediately for history mode ensures the preview is not updated repeatedly. This usage appears well-structured.
71-75
: Well-aligned with the typed interface.Providing both
onSchemaReceived
andloadInitialData
meets the composable’s contract and keeps logic neatly organized.
78-79
: Proper lifecycle hooking.Registering
initCommunication
ononMounted
and cleaning up ononBeforeUnmount
ensures the communication syncs exactly with the component’s lifecycle. Nicely done.packages/plugins/page/src/composable/usePage.ts (2)
442-447
: ValidatecurrentPage
before usage.
Consider adding null or undefined checks forcurrentPage
orcurrentPage.id
to prevent potential runtime errors when searching withinfamilyPages
.
489-502
: Verify required properties in object mapping.
Each mapped property relies on the source object (item
). Ensure thatitem
always contains these fields (e.g.,depth
,group
) to avoid undefined references.packages/common/js/preview.js (7)
13-25
: Imports organization looks good.
No issues found. The dependencies from@vueuse/core
and@opentiny/tiny-engine-meta-register
align with the new preview feature.
28-31
: Check for potential referencing ofpreviewWindow
before initialization.
previewWindow
is declared at line 31. Make sure all references occur after it’s set, to avoid undefined usage in other modules or concurrency scenarios.
50-88
: Check fallback handling for block/page detection.
getSchemaParams
branches logic based onisBlock()
. Confirm that it covers all relevant states (e.g., partial or missing data).
121-122
: Good proactive instantiation.
CallingsetupMessageListener()
at module scope ensures the listener is ready as soon as the file is loaded.
124-147
: Clean up subscriptions when the window closes.
Your unsubscribe logic is correct. This prevents resource leaks if the user frequently opens and closes the preview.
149-176
: Throttle callback is a good practice.
UsinguseThrottleFn
on schema change events avoids spam updates to the preview window. Implementation looks solid.
208-236
: Sufficient data in query parameters.
getQueryParams
merges scripts and styles in the URL well. No major concerns.packages/design-core/src/preview/src/preview/usePreviewData.ts (13)
1-23
: Imports and dependencies are well-organized.
They accurately reflect the new composition-based data handling approach.
24-32
: InterfaceIPage
definition looks good.
Ensure that external references toIPage
handle optional or extra fields gracefully.
33-41
: Reactive state for preview pages.
previewState
is a neat approach for global state in preview contexts. Keep in mind concurrency if other modules mutate it simultaneously.
42-45
:IDeps
naming is concise.
This definition clearly expresses the intention of script/style dependencies.
88-96
: Recursively gathering pages.
getPageRecursively
is clear. Ensure that endpoints (getPageById
) handle non-existent parent IDs gracefully.
154-166
: Extend existinggetImportMap
logic carefully.
Merging local bundle dependencies with existing import maps is good. Keep an eye on possible naming collisions inscripts
.
168-173
: InterfaceIPanelType
is straightforward.
The optionalindex
property is a nice differentiation for main vs. ancillary files.
206-241
: Comprehensive approach for fetching basic data.
getBasicData
merges multiple requests: app schema, metadata, import map, and base files. Good structure.
270-280
: Removinglang="jsx"
ensures Repl support.
ThefixScriptLang
function resolves known constraints in the Vue Repl environment.
282-289
: InterfaceIMetaDataParams
is relevant.
Capturinghistory
as optional is correct.
291-294
:IUsePreviewData
fosters clarity.
It clarifies whatsetFiles
andstore
should be, ensuring type consistency.
296-298
: Initialize withsetFiles(srcFiles, 'src/Main.vue')
.
This is a practical approach to set your base environment.
391-396
: Final API shape is well-defined.
ExposingloadInitialData
,updateUrl
, andupdatePreview
ensures easy external usage and maintainability.
packages/design-core/src/preview/src/preview/usePreviewCommunication.ts
Outdated
Show resolved
Hide resolved
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
♻️ Duplicate comments (2)
packages/design-core/src/preview/src/preview/usePreviewCommunication.ts (1)
11-17
:⚠️ Potential issueStrengthen origin checking for security.
The condition
event.origin.includes(window.location.hostname)
may incorrectly match partial or deceptive domains. A stricter equality check or a parsed hostname comparison would provide a safer way to validate the message origin.-if (event.origin === window.location.origin || event.origin.includes(window.location.hostname)) { +try { + const parsedOrigin = new URL(event.origin) + const parsedHost = new URL(window.location.href) + if (parsedOrigin.origin === window.location.origin || parsedOrigin.hostname === parsedHost.hostname) { + // Proceed with message handlingpackages/common/js/preview.js (1)
195-224
:⚠️ Potential issueMitigate potential memory leaks.
Window event listeners for the history preview should be removed once the preview is closed. Consider cleaning up with an appropriate removeEventListener call.
const handleHistoryPreview = (params, url) => { let historyPreviewWindow = null const handlePreviewReady = (event) => { if (event.origin === window.location.origin || event.origin.includes(window.location.hostname)) { const { event: eventType, source } = event.data || {} if (source === 'preview' && eventType === 'onMounted' && historyPreviewWindow) { const { scripts, styles, ancestors = [], ...rest } = params historyPreviewWindow.postMessage( { source: 'designer', type: 'schema', data: deepClone({ currentPage: rest, ancestors, scripts, styles }) }, '*' ) + // Remove the event listener after processing the message + window.removeEventListener('message', handlePreviewReady) } } } window.addEventListener('message', handlePreviewReady) historyPreviewWindow = window.open(url, '_blank') + + // Also handle the case when the window is closed + if (historyPreviewWindow) { + historyPreviewWindow.addEventListener('beforeunload', () => { + window.removeEventListener('message', handlePreviewReady) + }) + } }
🧹 Nitpick comments (1)
packages/design-core/src/preview/src/preview/usePreviewCommunication.ts (1)
1-4
: Improve type safety for data parameters.Using
any
type for data is not type-safe and could lead to runtime errors if the data structure is unexpected.interface PreviewCommunicationOptions { - onSchemaReceived: (data: any) => Promise<void> + onSchemaReceived: (data: PreviewSchemaData) => Promise<void> loadInitialData: () => Promise<void> } +interface PreviewSchemaData { + currentPage: { + page_content: any; // Consider defining a more specific type + [key: string]: any; + }; + ancestors: any[]; + scripts: Record<string, string>; + styles: any[]; +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
packages/common/js/preview.js
(1 hunks)packages/design-core/src/preview/src/preview/usePreviewCommunication.ts
(1 hunks)packages/toolbars/preview/meta.js
(1 hunks)packages/toolbars/preview/src/Main.vue
(3 hunks)
🧰 Additional context used
🧠 Learnings (1)
packages/toolbars/preview/src/Main.vue (1)
Learnt from: yy-wow
PR: opentiny/tiny-engine#850
File: packages/toolbars/preview/src/Main.vue:16-16
Timestamp: 2025-03-28T12:31:55.927Z
Learning: In `packages/toolbars/preview/src/Main.vue`, within the `preview` function, the `getMergeMeta` method is used at lines 64 and 65 to retrieve `engine.config` configurations.
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (12)
packages/design-core/src/preview/src/preview/usePreviewCommunication.ts (1)
70-81
: LGTM: Function implementation looks good.The
usePreviewCommunication
function appropriately sets up and returns the communication functions.packages/toolbars/preview/meta.js (1)
9-10
: LGTM: Appropriate addition of previewUrl property.Adding the
previewUrl
property allows for customization of the preview URL, supporting the new message-based communication implementation.packages/toolbars/preview/src/Main.vue (3)
5-5
: LGTM: Icon reference with optional chaining.The addition of optional chaining operators makes the icon reference more robust against undefined values.
61-61
:✅ Verification successful
Verify that previewPage works without parameters.
The function call has been simplified to call
previewPage()
without any parameters. The previous implementation likely constructed parameters for this function.
🏁 Script executed:
#!/bin/bash # Check how previewPage is defined and used elsewhere rg -A 3 "previewPage\s*=.*" packages/common/js/preview.jsLength of output: 152
Verified:
previewPage()
correctly utilizes default parameters.
The function’s default parameter values (i.e.,params = {}
andisHistory = false
) ensure that callingpreviewPage()
without arguments behaves as expected. No additional changes are needed here.
13-15
:❓ Verification inconclusive
Verify imports after simplification.
The imports have been significantly reduced, removing
previewBlock
,useBlock
,useCanvas
, andusePage
. Ensure that the simplification doesn't break any existing functionality.
🏁 Script executed:
#!/bin/bash # Check for any remaining references to removed imports rg "previewBlock|useBlock|useCanvas|usePage" packages/toolbars/preview/srcLength of output: 76
Action: Confirm simplified imports do not impact functionality
- The recent search using
rg
in thepackages/toolbars/preview/src
directory did not reveal any lingering references to the removed imports (previewBlock
,useBlock
,useCanvas
,usePage
).- Given that the command produced no output (which might indicate low-quality inference), please perform a manual verification of the affected functionality to ensure nothing is inadvertently broken.
packages/common/js/preview.js (7)
13-26
: LGTM: Imports properly updated.The imports have been updated to support the new schema management and preview communication features.
36-51
: LGTM: Good dependency consolidation.The
getScriptAndStyleDeps
function effectively consolidates dependencies from multiple hooks, providing a cleaner interface for script and style management.
53-91
: LGTM: Well-structured schema parameter function.The
getSchemaParams
function handles both block and page scenarios appropriately, providing a consistent data structure for both cases.
136-164
: LGTM: Good use of throttling for schema updates.Using
useThrottleFn
for schema change updates is an excellent choice to prevent excessive updates during rapid changes.
225-252
: LGTM: Comprehensive query parameter construction.The
getQueryParams
function effectively constructs the URL parameters needed for the preview, including support for history previews.
254-292
: LGTM: Well-implemented preview window management.The
open
function intelligently handles both new and existing preview windows, supporting custom URLs and properly setting up listeners.
294-296
: LGTM: Clean export of the previewPage function.The simplified
previewPage
function provides a clean interface for opening preview windows.
packages/design-core/src/preview/src/preview/usePreviewCommunication.ts
Outdated
Show resolved
Hide resolved
packages/design-core/src/preview/src/preview/usePreviewCommunication.ts
Outdated
Show resolved
Hide resolved
525ad0f
to
8c4aadf
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: 0
🧹 Nitpick comments (7)
packages/design-core/src/preview/src/preview/http.js (1)
48-51
: Add JSDoc comments for new API functionsThe newly added functions provide a good API for retrieving page, block, and history data directly instead of using URL parameters. However, they lack documentation compared to other functions in this file.
Consider adding JSDoc comments for consistency and better developer experience:
+/** + * Get page details by ID + * @param {string} id - The page ID + * @returns {Promise} - Promise resolving to page details + */ export const getPageById = async (id) => getMetaApi(META_SERVICE.Http).get(`/app-center/api/pages/detail/${id}`) +/** + * Get block details by ID + * @param {string} id - The block ID + * @returns {Promise} - Promise resolving to block details + */ export const getBlockById = async (id) => getMetaApi(META_SERVICE.Http).get(`/material-center/api/block/detail/${id}`) +/** + * Fetch page history by page ID + * @param {string} pageId - The page ID + * @returns {Promise} - Promise resolving to page history + */ export const fetchPageHistory = (pageId) => getMetaApi(META_SERVICE.Http).get(`/app-center/api/pages/histories?page=${pageId}`)packages/design-core/src/preview/src/Toolbar.vue (1)
37-46
: Add null checks for better robustnessThe watch function correctly updates breadcrumbs based on currentPage changes, but lacks null/undefined checking which could cause runtime errors.
Add null checks to prevent potential errors:
watch( () => previewState.currentPage, (newVal) => { + if (!newVal) return; if (newVal?.page_content?.componentName === constants.COMPONENT_NAME.Block) { setBreadcrumbBlock([newVal?.name_cn || newVal?.page_content?.fileName]) } else { setBreadcrumbPage([newVal?.name]) } } )
Also, consider setting the initial breadcrumb state using an immediate watch:
watch( () => previewState.currentPage, (newVal) => { if (!newVal) return; if (newVal?.page_content?.componentName === constants.COMPONENT_NAME.Block) { setBreadcrumbBlock([newVal?.name_cn || newVal?.page_content?.fileName]) } else { setBreadcrumbPage([newVal?.name]) } }, + { immediate: true } )
packages/design-core/src/preview/src/preview/Preview.vue (1)
58-69
: Good implementation of the preview data management.The use of a dedicated hook for preview data management improves separation of concerns. The schema handling logic with cleanup for history mode is well-implemented.
However, the cleanup function pattern could be improved for clarity.
Consider using a more descriptive name than
cleanupCommunicationAction
to better reflect its purpose:-let cleanupCommunicationAction = null -const onSchemaReceivedAction = async (data) => { +let communicationCleanupFunction = null +const handleSchemaReceived = async (data) => { updateUrl(data.currentPage, { scripts: data.scripts, styles: data.styles }) const isHistory = new URLSearchParams(location.search).get('history') // 如果是历史预览,则不需要实时预览,接收到消息之后直接取消监听(需要监听到第一次消息接受页面信息) if (isHistory) { - cleanupCommunicationAction() + communicationCleanupFunction() } await updatePreview(data) }packages/plugins/page/src/composable/usePage.ts (1)
481-506
: Enhanced family page handling.The refactored
getFamily
function now properly maps family pages to extract only the necessary properties and updates page content consistently.However, there's a potential performance improvement opportunity.
Consider memoizing the result of
getAncestorsRecursively
for frequently accessed pages:+const familyPagesCache = new Map(); const getFamily = async (currentPage) => { if (pageSettingState.pages.length === 0) { await getPageList() } + // Check if we have cached results for this page + if (familyPagesCache.has(currentPage.id)) { + const cachedFamilyPages = familyPagesCache.get(currentPage.id); + // Still need to update content with latest changes + updatePageContent(cachedFamilyPages, currentPage); + return cachedFamilyPages; + } const familyPages = getAncestorsRecursively(currentPage.id) .filter((item) => item.isPage) .reverse() .map((item) => ({ id: item.id, page_content: item.page_content, name: item.name, parentId: item.parentId, route: item.route, isPage: item.isPage, isBody: item.isBody, isHome: item.isHome, group: item.group, isDefault: item.isDefault, depth: item.depth })) await handlePageDetail(familyPages) updatePageContent(familyPages, currentPage) + + // Cache the result for future calls + familyPagesCache.set(currentPage.id, familyPages); return familyPages }packages/common/js/preview.js (1)
197-228
: Well-implemented history preview mode.The history preview implementation correctly handles one-time communication and cleans up event listeners appropriately. This prevents unnecessary updates and resources consumption.
Consider adding a timeout mechanism for removing the event listener if the window never sends an onMounted event:
const handleHistoryPreview = (params, url) => { let historyPreviewWindow = null + let timeoutId = null + const handlePreviewReady = (event) => { if (event.origin === window.location.origin || event.origin.includes(window.location.hostname)) { const { event: eventType, source } = event.data || {} if (source === 'preview' && eventType === 'onMounted' && historyPreviewWindow) { const { scripts, styles, ancestors = [], ...rest } = params historyPreviewWindow.postMessage( { source: 'designer', type: 'schema', data: deepClone({ currentPage: rest, ancestors, scripts, styles }) }, previewWindow.origin || window.location.origin ) // 历史页面不需要实时更新预览,发送完消息后移除监听 window.removeEventListener('message', handlePreviewReady) + clearTimeout(timeoutId) } } } window.addEventListener('message', handlePreviewReady) + // Set timeout to remove event listener after 30 seconds if no message received + timeoutId = setTimeout(() => { + window.removeEventListener('message', handlePreviewReady) + console.warn('History preview window did not respond within timeout period') + }, 30000) historyPreviewWindow = window.open(url, '_blank') }packages/design-core/src/preview/src/preview/usePreviewData.ts (2)
181-240
: Robust page ancestry file generation.The
getPageAncestryFiles
function effectively handles the generation of Vue component files for ancestor pages with proper naming and indexing.Consider adding error handling for the code generation:
const getPageAncestryFiles = ( appData: IGetPageAncestryFilesParams['appData'], params: IGetPageAncestryFilesParams['params'] ) => { const familyPages: IPanelType[] = [] const ancestors = params.ancestors // @ts-ignore const { generatePageCode } = getMetaApi('engine.service.generateCode') + + // Check if code generation service is available + if (!generatePageCode) { + console.error('Code generation service not available') + return familyPages + } // Rest of the function remains unchanged // ... }
296-396
: Well-structured preview data hook.The
usePreviewData
hook provides a clean API with appropriate methods for loading data and updating the preview. The code transformation for script tags is a good solution for compatibility issues.Consider implementing error handling and retry logic for API calls:
const loadInitialData = async () => { + try { const { currentPage, ancestors } = await getPageOrBlockByApi() previewState.currentPage = currentPage previewState.ancestors = ancestors if (currentPage) { updatePreview({ currentPage, ancestors }) } + } catch (error) { + console.error('Failed to load initial preview data:', error) + // Consider implementing a retry mechanism or user notification + } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
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)
🚧 Files skipped from review as they are similar to previous changes (7)
- packages/toolbars/preview/meta.js
- packages/design-core/package.json
- packages/plugins/block/src/BlockSetting.vue
- packages/plugins/page/src/PageHistory.vue
- packages/design-core/src/preview/src/preview/importMap.js
- packages/design-core/src/preview/src/preview/usePreviewCommunication.ts
- packages/common/package.json
🧰 Additional context used
🧠 Learnings (1)
packages/toolbars/preview/src/Main.vue (1)
Learnt from: yy-wow
PR: opentiny/tiny-engine#850
File: packages/toolbars/preview/src/Main.vue:16-16
Timestamp: 2025-03-28T12:31:55.927Z
Learning: In `packages/toolbars/preview/src/Main.vue`, within the `preview` function, the `getMergeMeta` method is used at lines 64 and 65 to retrieve `engine.config` configurations.
🧬 Code Definitions (2)
packages/plugins/page/src/composable/usePage.ts (1)
packages/common/js/preview.js (1)
currentPage
(76-76)
packages/design-core/src/preview/src/preview/usePreviewData.ts (3)
packages/design-core/src/preview/src/preview/http.js (10)
getPageById
(48-48)getPageById
(48-48)fetchPageHistory
(50-51)fetchPageHistory
(50-51)getBlockById
(49-49)getBlockById
(49-49)fetchAppSchema
(44-44)fetchAppSchema
(44-44)fetchMetaData
(31-37)fetchMetaData
(31-37)packages/design-core/src/preview/src/preview/importMap.js (2)
getImportMap
(31-38)getImportMap
(31-38)packages/design-core/src/preview/src/preview/generate.js (3)
res
(158-158)processAppJsCode
(153-167)processAppJsCode
(153-167)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (24)
packages/design-core/src/preview/src/preview/generate.js (1)
153-167
: Improved CSS processing with defensive programmingThe refactoring of
processAppJsCode
is well done, adding input validation and preventing duplicate CSS references. These changes improve robustness and prevent potential issues.The changes include:
- Validation that
cssList
is a valid non-empty array- Prevention of duplicate CSS references by checking if they already exist
- Proper code structure and spacing with newlines
packages/design-core/src/preview/src/Toolbar.vue (1)
17-23
: LGTM: Addition of reactive importsThe new imports support the move from URL parameter-based approach to a reactive state-based approach, which aligns with the PR objectives.
packages/toolbars/preview/src/Main.vue (2)
14-15
: Simplified imports align with the new approachThe import changes reflect the transition to a more streamlined preview system.
61-61
:❓ Verification inconclusive
Verify that previewPage handles all preview cases
The function has been significantly simplified, removing parameter construction and conditional logic for block vs. page previews. This simplification is good, but ensure that
previewPage
now handles all preview scenarios internally.
🏁 Script executed:
#!/bin/bash # Check the implementation of previewPage to ensure it handles both block and page preview cases echo "Checking implementation of previewPage function:" rg -A 15 "export const previewPage" --glob "*.js" echo "\nChecking any references to block preview functionality:" rg "previewBlock" --glob "*.{js,vue,ts}"Length of output: 507
Action Required: Confirm Unified Preview Handling Implementation
The implementation of
previewPage
inpackages/common/js/preview.js
now simply callsopen(params, isHistory)
without any conditional logic specifically for block previews. No references to a separatepreviewBlock
function were found, indicating that all preview scenarios are expected to be handled through this unified function. Please manually verify that any preview requests (both block and page) are fully accommodated by this strategy.
- File:
packages/common/js/preview.js
- Current implementation:
export const previewPage = (params = {}, isHistory = false) => { open(params, isHistory) }- Observation: No separate
previewBlock
or related conditional handling exists in the codebase.- Next Steps: Ensure that external calls to
previewPage
(such as frompackages/toolbars/preview/src/Main.vue
) pass the proper parameters so that all preview cases (block and page) are supported.packages/design-core/src/preview/src/preview/Preview.vue (4)
17-21
: Well-structured imports improve maintainability.The addition of Vue lifecycle hooks and new preview utility imports creates a more modular approach to component management. This is a good architectural improvement.
55-56
: Simplified parameter handling.Using URLSearchParams is a cleaner approach for query parameter handling than manual parsing. This improves code readability and maintainability.
71-77
: Good communication setup with proper dependency injection.The pattern of injecting dependencies into the communication hook follows good practices and makes testing easier.
76-79
: Proper lifecycle management for communication.The use of lifecycle hooks ensures proper initialization and cleanup of communication resources, preventing potential memory leaks.
packages/plugins/page/src/composable/usePage.ts (2)
442-447
: Improved page content update logic.The function now correctly updates the page content by finding the matching page in the family pages array. This is a more robust approach than direct assignment.
468-479
: Simplified handlePageDetail function signature.Removing the
currentPage
parameter simplifies the function and makes it more focused on handling the pages array. Good refactoring to reduce unnecessary parameters.packages/common/js/preview.js (10)
13-26
: Well-organized imports.Good restructuring of imports to include all necessary hooks and utilities for the enhanced preview functionality.
31-35
: Good communication channel implementation.The use of BroadcastChannel for preview communication is a modern approach that improves cross-window messaging reliability.
36-51
: Clean dependency management.The
getScriptAndStyleDeps
function effectively consolidates dependencies from multiple sources into structured objects, making them easier to manage.
53-91
: Well-structured schema parameter handling.The
getSchemaParams
function intelligently differentiates between block and page schemas and gathers all necessary data. Good separation of concerns and code organization.
94-103
: Secure message passing implementation.The
sendSchemaUpdate
function properly handles posting messages to the preview window with the appropriate origin rather than using '*' which is a security improvement.
107-122
: Thorough event listener management.Excellent implementation of setup and cleanup for schema change listeners. The code properly handles subscription lifecycle to prevent memory leaks and ensure resources are released when no longer needed.
Also applies to: 135-164
167-192
: Robust message listener with improved security.The message listener implementation includes proper origin validation using URL parsing, which is more secure than simple string comparisons. Good practice for cross-window communication.
230-257
: Comprehensive query parameter handling.The
getQueryParams
function properly constructs query parameters with appropriate encoding for complex data structures like scripts and styles.
259-297
: Improved preview window management.The
open
function now handles various scenarios including history previews and focusing existing windows. The addition of custom preview URL support through configuration enhances flexibility.
299-301
: Simplified preview page function.The refactored
previewPage
function with history mode support simplifies the API while maintaining functionality.packages/design-core/src/preview/src/preview/usePreviewData.ts (4)
24-40
: Well-defined TypeScript interfaces.Good use of TypeScript interfaces for type safety. The reactive state pattern for managing current page and ancestors is clean and maintainable.
47-86
: Smart URL update logic.The
updateUrl
function intelligently decides when to update the URL, preventing unnecessary history entries. The conditional logic based on component type (Page vs Block) is well-implemented.
88-153
: Thorough data fetching and history handling.The recursive page fetching and history state management are well-implemented. The code correctly handles both page and block preview scenarios with appropriate data structures.
304-322
: Careful script processing.The Babel transformation for script tags is well-implemented to ensure compatibility with Vue SFC requirements. The handling of empty script blocks is a good defensive programming practice.
8c4aadf
to
ee7d1b1
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: 2
🧹 Nitpick comments (10)
packages/design-core/src/preview/src/preview/http.js (1)
48-51
: Well-structured API functions for data retrievalThese new functions follow the established pattern in the codebase while moving away from URL parameter parsing. They provide clean, dedicated endpoints for retrieving page and block data, supporting the PR's goal of enhancing preview functionality with real-time updates.
Consider adding try/catch blocks for error handling to improve robustness:
-export const getPageById = async (id) => getMetaApi(META_SERVICE.Http).get(`/app-center/api/pages/detail/${id}`) +export const getPageById = async (id) => { + try { + return await getMetaApi(META_SERVICE.Http).get(`/app-center/api/pages/detail/${id}`) + } catch (error) { + console.error('Error fetching page by ID:', error) + throw error + } +}packages/plugins/block/src/BlockSetting.vue (2)
90-90
: Import cleanup should be completedThe PR summary indicated that
useBlock
should be removed from imports, but it remains in this line. The function is still used in the file, but for consistency with other changes in this PR, consider extracting it separately if needed.
247-259
: Simplified preview function implementationThe function has been properly streamlined to use the new postMessage-based preview system mentioned in the PR objectives, which helps resolve the URL parameter truncation issue.
However, the boolean parameter
true
lacks context:previewPage( { page_content: item.content, id: item.blockId || item.block_id, history: item.id, name: item.label }, - true + true // isBlock: indicates this is a block preview rather than a page )packages/design-core/src/preview/src/Toolbar.vue (1)
37-46
: Improved reactive breadcrumb updatesThe watch function elegantly replaces the previous URL parameter parsing approach, establishing a reactive connection to the preview state that will update automatically when content changes - directly supporting the PR's goal of real-time updates.
Consider adding a null check to prevent potential errors if
newVal
is undefined:watch( () => previewState.currentPage, (newVal) => { + if (!newVal) return; if (newVal?.page_content?.componentName === constants.COMPONENT_NAME.Block) { setBreadcrumbBlock([newVal?.name_cn || newVal?.page_content?.fileName]) } else { setBreadcrumbPage([newVal?.name]) } } )
packages/design-core/src/preview/src/preview/generate.js (1)
154-167
: Improved CSS dependency handlingThe function now properly validates inputs and avoids duplicate CSS imports, enhancing robustness. This supports the PR's goal of improving preview functionality.
Consider adding a comment explaining what
addCss
does for better code maintainability:export const processAppJsCode = (code, cssList) => { if (!Array.isArray(cssList) || !cssList.length) { return code } let res = `${code}\n` + // Add CSS dependencies using addCss function, avoiding duplicates cssList.forEach((css) => { if (!code.includes(`addCss('${css}')`)) { res += `addCss('${css}')\n` } }) return res }
packages/design-core/src/preview/src/preview/usePreviewData.ts (5)
23-23
: Address the TODO for TS TypesLine 23 references a TODO for extracting shared TypeScript definitions. It might be worth centralizing these types into a dedicated file (e.g.,
types.ts
) to promote reusability across the codebase.Do you want me to open a new issue or prepare a PR to create these shared TS definitions?
24-32
: Replaceany
with More Specific Types
page_content
declares a broad{ [key: string]: any }
shape. Consider replacing this with more precise types to enhance type safety and maintainability. This helps catch possible shape mismatches at compile-time.interface IPage { id: number name: string parentId: number | string - page_content: { - [key: string]: any - componentName: string - } + page_content: { + componentName: string + // Replace the line below with a stricter type definition + // e.g. props?: Record<string, unknown>, children?: IBlockContent[], etc. + } }
47-86
: Avoid Overly Large Query ParametersThe
updateUrl
function appendsscripts
andstyles
directly as JSON strings in the URL. This may cause overly lengthy query parameters, leading to potential URL-length limitations or performance concerns. Consider a shorter identifier-based approach.
327-379
: Consider Breaking DownupdatePreview
into Smaller Functions
updatePreview
handles multiple responsibilities (meta data retrieval, import map updates, block scanning, code generation, etc.). Splitting it into smaller, focused helpers aids readability and testing.
381-389
: Improve Error Recovery inloadInitialData
Currently, if
getPageOrBlockByApi
fails or returns empty data, there’s no fallback behavior besides settingpreviewState
tonull
. A more robust approach could handle user notifications or retries.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
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)
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/toolbars/preview/meta.js
- packages/common/package.json
- packages/design-core/src/preview/src/preview/importMap.js
- packages/design-core/package.json
- packages/plugins/page/src/PageHistory.vue
- packages/design-core/src/preview/src/preview/usePreviewCommunication.ts
🧰 Additional context used
🧠 Learnings (1)
packages/toolbars/preview/src/Main.vue (1)
Learnt from: yy-wow
PR: opentiny/tiny-engine#850
File: packages/toolbars/preview/src/Main.vue:16-16
Timestamp: 2025-04-08T06:42:07.303Z
Learning: In `packages/toolbars/preview/src/Main.vue`, within the `preview` function, the `getMergeMeta` method is used at lines 64 and 65 to retrieve `engine.config` configurations.
🧬 Code Definitions (3)
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
(233-233)
packages/common/js/preview.js (3)
packages/canvas/container/src/container.ts (1)
getSchema
(94-94)packages/register/src/common.ts (2)
getMetaApi
(54-64)getMergeMeta
(179-181)packages/register/src/constants.ts (1)
META_SERVICE
(1-22)
packages/design-core/src/preview/src/preview/usePreviewData.ts (3)
packages/design-core/src/preview/src/preview/http.js (12)
getPageById
(48-48)getPageById
(48-48)fetchPageHistory
(50-51)fetchPageHistory
(50-51)getBlockById
(49-49)getBlockById
(49-49)fetchAppSchema
(44-44)fetchAppSchema
(44-44)fetchMetaData
(31-37)fetchMetaData
(31-37)fetchBlockSchema
(45-46)fetchBlockSchema
(45-46)packages/design-core/src/preview/src/constant/index.js (1)
PanelType
(13-15)packages/design-core/src/preview/src/preview/srcFiles.js (1)
srcFiles
(27-27)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (25)
packages/design-core/src/preview/src/Toolbar.vue (1)
17-23
: Good addition of necessary importsThese imports properly support the new reactive approach to breadcrumb updates, removing the dependency on URL parameters.
packages/toolbars/preview/src/Main.vue (3)
5-5
: Improved icon fallback handling.Added a fallback to use
options?.icon
whenoptions.icon?.default
is undefined, which makes the component more robust against different icon formats.
14-15
: Simplified imports for streamlined preview approach.The imports have been streamlined to remove unnecessary dependencies (
previewBlock
,useBlock
,useCanvas
,usePage
), which aligns with the simplified preview approach implemented in this PR.
61-61
:✅ Verification successful
Preview functionality now uses postMessage communication.
The preview method has been simplified to call
previewPage()
without parameters, which aligns with PR objectives to switch to postMessage for parameter passing. This approach prevents URL parameter truncation for large pages.
🏁 Script executed:
#!/bin/bash # Verify that previewPage is now implemented to handle all cases internally echo "Checking how previewPage is implemented in the common/js/preview.js file" rg -A 5 "export const previewPage" packages/common/js/preview.jsLength of output: 314
Verified:
previewPage()
now properly handles default parameters internallyThe call to
previewPage()
inpackages/toolbars/preview/src/Main.vue
has been simplified to omit parameters, aligning with the PR's objective of using postMessage communication for parameter passing. Our verification confirms that the implementation inpackages/common/js/preview.js
uses default values, ensuring that all cases are now handled internally without risking URL parameter truncation for large pages.packages/design-core/src/preview/src/preview/Preview.vue (5)
17-21
: Added Vue composition API and new preview hooks.The component now implements Vue's composition API lifecycle hooks (
onMounted
,onBeforeUnmount
) and introduces new custom hooks (usePreviewCommunication
,usePreviewData
) that support the real-time preview updates as required by the PR.
55-56
: Simplified query parameter handling.Replaced complex query parameter parsing with standard
URLSearchParams
and directly set the theme attribute on the document, improving code readability and maintenance.
71-77
: Added communication system with cleanup.Implemented a robust communication system with proper cleanup through
usePreviewCommunication
, ensuring that resources are properly released when the component is unmounted. This prevents memory leaks and ensures clean state transitions.
78-79
: Added proper lifecycle management.Added lifecycle hooks to initialize communication on mount and clean up on unmount, ensuring that resources are properly managed throughout the component's lifecycle.
58-69
:✅ Verification successful
Implemented real-time preview data management.
Added structured data handling with the
usePreviewData
hook and implementedonSchemaReceivedAction
to handle incoming schema updates. This supports the PR objective of enabling real-time updates when changes are made in the editing interface.
🏁 Script executed:
#!/bin/bash # Check implementation of usePreviewData hook echo "Checking usePreviewData implementation" fd -t f "usePreviewData.ts" packages/design-core --exec cat {}Length of output: 10875
Real-time preview management implementation verified
The hook and its usage in the preview component have been confirmed. The
usePreviewData
hook is implemented as expected, and theonSchemaReceivedAction
correctly manages URL updates and conditionally cleans up listeners when handling historical previews.No further modifications are necessary here.
packages/plugins/page/src/composable/usePage.ts (3)
442-447
: Refactored page content update to work with familyPages.Modified
updatePageContent
to find and update the current page schema within a collection of family pages, which supports the new approach of transferring complete schema data between designer and preview.
468-479
: Simplified page detail handling.Simplified the
handlePageDetail
function to focus solely on fetching page details and updating parent IDs without immediately updating page content. This separation of concerns improves code maintainability.
481-506
: Enhanced family page handling for preview.Significantly enhanced the
getFamily
function to:
- Extract specific properties from ancestor pages
- Handle page details with proper error handling
- Update page content with current changes
This change enables the preview system to access all necessary page data for rendering the preview with proper context and inheritance.
packages/common/js/preview.js (11)
13-29
: Added necessary imports and utilities for enhanced preview.Added imports from
@vueuse/core
for throttling and other meta-register hooks needed for the new communication approach. ThedeepClone
utility ensures schema data is properly isolated during transfers.
30-35
: Added cross-window communication infrastructure.Established global state to maintain a reference to the preview window and created a
BroadcastChannel
for communication between designer and preview windows, which is essential for the new real-time preview feature.
36-51
: Added dependency collection for preview rendering.Implemented
getScriptAndStyleDeps
to gather all necessary script and style dependencies from materials and utilities, ensuring the preview has access to all required resources for proper rendering.
53-91
: Implemented robust schema parameter collection.Created
getSchemaParams
to gather all necessary data for preview rendering, handling both block and page preview scenarios. This function:
- Detects the current context (block or page)
- Retrieves appropriate schema and metadata
- Assembles dependencies and ancestor data
- Returns a complete data package for preview
This approach ensures all necessary data is available without relying on URL parameters, addressing the PR objective of preventing truncation issues.
93-103
: Implemented secure schema update mechanism.Created
sendSchemaUpdate
to safely transmit schema data to the preview window usingpostMessage
, with proper origin validation usingpreviewWindow.origin || window.location.origin
instead of the insecure wildcard origin.
105-164
: Added schema change subscription with cleanup.Implemented a comprehensive system to:
- Subscribe to schema changes with proper throttling
- Clean up listeners when no longer needed
- Handle various schema change events (direct changes, imports, initializations)
This system enables the real-time preview updates required by the PR objectives while preventing memory leaks through proper cleanup.
167-192
: Implemented secure cross-window message handling.Created
setupMessageListener
to safely process messages from the preview window with:
- Proper origin validation using URL parsing
- Connection management for preview window references
- Initial data loading on preview mount
The implementation includes proper URL parsing for origin validation, addressing a security concern from previous reviews.
197-228
: Added history preview support with proper cleanup.Implemented
handleHistoryPreview
to support viewing historical page versions with:
- One-time data transmission to prevent ongoing updates
- Proper event listener cleanup after sending data
- Secure origin validation
This implementation addresses a memory leak concern from previous reviews by adding proper event listener cleanup.
230-257
: Enhanced query parameter management.Created
getQueryParams
to build query strings with all necessary parameters, including:
- Current tenant and theme information
- Framework and platform information
- Resources and dependencies
- Page/block identifiers
- History state when applicable
This function provides a consistent way to build preview URLs with all required context.
259-297
: Improved preview window management.Enhanced the
open
function to:
- Support custom preview URLs from configuration
- Handle history preview mode
- Focus existing preview windows instead of opening duplicates
- Set up schema change listeners for real-time updates
This implementation provides a flexible, configurable approach to opening previews while maintaining the connection for real-time updates.
299-301
: Simplified preview API with support for history mode.Updated the
previewPage
API to provide a clean interface that accepts optional parameters and a history flag, making it easy to use from various parts of the application while supporting all required features.packages/design-core/src/preview/src/preview/usePreviewData.ts (2)
1-19
: Overall Implementation Looks SolidThe overall file structure and the approach to managing reactive preview data with Vue composables appear consistent and readable. Great job on setting up the file and its imports!
88-96
: Guard Against Cyclical Page References
getPageRecursively
may run into an infinite loop if the page hierarchy has circular parent-child references. Introduce a visited set or check for recursion depth to avoid potential unbounded recursion.Here is a sample approach:
const visitedPages = new Set(); const getPageRecursively = async (id: string): Promise<IPage[]> => { if (visitedPages.has(id)) { // Prevent infinite recursion return []; } visitedPages.add(id); const page = await getPageById(id); ... }
ee7d1b1
to
49561d8
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: 6
🔭 Outside diff range comments (1)
packages/toolbars/preview/src/Main.vue (1)
30-63
:⚠️ Potential issueErrors thrown by
previewPage()
are not caughtAll preparatory hooks are wrapped with try/catch, but the main call to
previewPage()
is outside that block.
A failed window opening or a rejected promise (e.g. popup blocked) will surface as an uncaught error.- previewPage() + try { + await previewPage() + } catch (err) { + useNotify({ type: 'error', message: `Preview failed: ${err}` }) + return + }
♻️ Duplicate comments (2)
packages/design-core/src/preview/src/preview/usePreviewData.ts (2)
98-147
: Network failures are still not handled
Previous reviews already pointed this out; the new implementation again fetches pages/blocks without any error handling.
Wrap every remote call intry/catch
and provide fallback UI or logging.
304-323
: Unsafe Babel transformation of user contentThe JSX → JS transform runs on arbitrary page/block scripts. Without sanitisation this opens the door to XSS when the transformed code is executed in the REPL iframe.
At minimum, validate the source against a deny‑list (e.g.
import
,eval
) or transform inside a Web Worker with a restricted context.
🧹 Nitpick comments (4)
packages/design-core/src/preview/src/preview/Preview.vue (2)
55-57
: Theme selection is locked to the initial URL — consider reacting to runtime changes
queryParams
is instantiated once and its value never refreshed.
If the designer subsequently updates the URL (e.g. viaupdateUrl
) with a differenttheme
, the preview will not pick it up until a full reload.-const queryParams = new URLSearchParams(location.search) -document.documentElement?.setAttribute?.('data-theme', queryParams.get('theme') || 'light') +const setTheme = () => { + const params = new URLSearchParams(location.search) + document.documentElement?.setAttribute?.( + 'data-theme', + params.get('theme') || 'light' + ) +} +setTheme() +// react to future url updates +window.addEventListener('popstate', setTheme)
71-79
: Potential memory‑leak whenhistory
mode closes the channel
cleanupCommunicationAction
is reassigned after the callback that uses it is created, which is OK, butonSchemaReceivedAction
never null‑checks before invoking.
IfinitCommunication
fails or was never called, callingcleanupCommunicationAction()
will throw.-if (isHistory) { - cleanupCommunicationAction() +if (isHistory && typeof cleanupCommunicationAction === 'function') { + cleanupCommunicationAction() }packages/design-core/src/preview/src/preview/usePreviewData.ts (1)
154-166
:getImportMap
refetches import‑map on every schema changeBecause
fetchImportMap()
is awaited inside the function, each call makes a network round‑trip.
Consider caching the promise or result the first time and reusing it:-let getImportMap = async (...) => { - if (import.meta.env.VITE_LOCAL_BUNDLE_DEPS === 'true') { - const mapJSON = await fetchImportMap() +const importMapCache = { value: null } +const getImportMap = async (scripts = {}) => { + if (import.meta.env.VITE_LOCAL_BUNDLE_DEPS === 'true') { + if (!importMapCache.value) { + importMapCache.value = await fetchImportMap().catch(() => ({ imports: {} })) + } + const mapJSON = importMapCache.valuepackages/common/js/preview.js (1)
240-244
: Embedding large JSON blobs in the query string undermines the “long‑URL” fix
&scripts=${JSON.stringify(scripts)}&styles=${JSON.stringify(styles)}
can easily exceed the
URL length limits that originally motivated the switch topostMessage
. Consider:
- Moving script/style dependency data into the initial
postMessage
payload alongside the
schema (they are already available there), and- Replacing these query params with a short opaque token or omitting them entirely.
This keeps the preview URL compact and future‑proof.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
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)
🚧 Files skipped from review as they are similar to previous changes (11)
- packages/design-core/package.json
- packages/plugins/page/src/PageHistory.vue
- packages/toolbars/preview/meta.js
- packages/common/package.json
- packages/design-core/src/preview/src/preview/http.js
- packages/design-core/src/preview/src/Toolbar.vue
- packages/design-core/src/preview/src/preview/importMap.js
- packages/design-core/src/preview/src/preview/generate.js
- packages/design-core/src/preview/src/preview/usePreviewCommunication.ts
- packages/plugins/page/src/composable/usePage.ts
- packages/plugins/block/src/BlockSetting.vue
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/design-core/src/preview/src/preview/usePreviewData.ts (6)
packages/design-core/src/preview/src/preview/http.js (14)
getPageById
(48-48)getPageById
(48-48)fetchPageHistory
(50-51)fetchPageHistory
(50-51)getBlockById
(49-49)getBlockById
(49-49)fetchImportMap
(39-42)fetchImportMap
(39-42)fetchAppSchema
(44-44)fetchAppSchema
(44-44)fetchMetaData
(31-37)fetchMetaData
(31-37)fetchBlockSchema
(45-46)fetchBlockSchema
(45-46)packages/design-core/src/preview/src/preview/importMap.js (2)
getImportMap
(31-38)getImportMap
(31-38)packages/register/src/common.ts (1)
getMetaApi
(54-64)packages/design-core/src/preview/src/constant/index.js (1)
PanelType
(13-15)packages/design-core/src/preview/src/preview/srcFiles.js (1)
srcFiles
(27-27)packages/design-core/src/preview/src/preview/generate.js (3)
res
(158-158)processAppJsCode
(153-167)processAppJsCode
(153-167)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (1)
packages/design-core/src/preview/src/preview/usePreviewData.ts (1)
55-64
: 🛠️ Refactor suggestionURL comparison may trigger unnecessary history churn
styles
andscripts
are compared withJSON.stringify(...) !== searchParams.get(...)
.
URLSearchParams
returns URL‑encoded strings, so the comparison often fails even when logically equal, causingreplaceState
to run on every update.Suggest decoding the existing value first or comparing parsed objects.
-const styles = searchParams.get('styles') -... -if (deps.styles && JSON.stringify(deps.styles) !== styles) { +const styles = JSON.parse(searchParams.get('styles') || '[]') +... +if (deps.styles && JSON.stringify(deps.styles) !== JSON.stringify(styles)) {Likely an incorrect or invalid review 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.
LGTM!
然后有一些点可以考虑下能否优化:
1.热更新白屏时间长(接近10s左右),有点影响体验,可以考虑:
- 更新schema能否实现不刷新页面,依赖不重新加载
- 部分场景可以考虑优化减少热更新次数:
- 未锁定场景,会先修改刷新,再恢复刷新,未锁定能否不发送message
- 修改属性后,已经热更新,再点击保存,没有schema变化,能否不重新热更新加载
- 路由特性有父页面场景,单页视图之前在预览没有支持,后面可以考虑支持(设计器子页面嵌套视图切换到单页视图,预览还是显示单页视图)
49561d8
to
35f25a3
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: 0
♻️ Duplicate comments (7)
packages/design-core/src/preview/src/preview/usePreviewData.ts (3)
98-153
: Add error handling for API calls.The function
getPageOrBlockByApi
makes multiple API calls without error handling, which could cause silent failures in the preview functionality.const getPageOrBlockByApi = async (): Promise<{ currentPage: IPage | null; ancestors: IPage[] }> => { const searchParams = new URLSearchParams(location.search) const pageId = searchParams.get('pageid') const blockId = searchParams.get('blockid') const history = searchParams.get('history') + try { if (pageId) { let ancestors = (await getPageRecursively(pageId)).reverse() let currentPage = await getPageById(pageId) if (history) { const historyList: IPage[] = await fetchPageHistory(pageId) currentPage = historyList.find((item) => item.id === Number(history)) ancestors = ancestors.map((item) => { if (item.id === Number(pageId)) { return { ...item, page_content: currentPage.page_content } } return item }) } return { currentPage, ancestors } } if (blockId) { const block = await getBlockById(blockId) let pageContent = block.content let name = block.name || block.name_cn || block.label if (history) { const historyContent = (block.histories || []).find((item: { id: number }) => item.id === Number(history)) pageContent = historyContent?.content || pageContent name = historyContent?.name || name } return { currentPage: { ...block, page_content: pageContent, name }, ancestors: [] } } return { currentPage: null, ancestors: [] } + } catch (error) { + console.error('Failed to fetch page or block data:', error) + return { + currentPage: null, + ancestors: [] + } + } }
304-322
: Validate/sanitize code before Babel transformation.The code transforms arbitrary script content without validation, which could pose a security risk if the source is untrusted.
Consider adding input validation before transformation or ensuring the code source is trusted.
369-369
:⚠️ Potential issueFix incorrect styles parameter parsing.
The
processAppJsCode
function expects an array of CSS files, but you're parsing the styles parameter as an object.- const appJsCode = processAppJsCode(newFiles['app.js'], JSON.parse(searchParams.get('styles') || '{}')) + const appJsCode = processAppJsCode(newFiles['app.js'], JSON.parse(searchParams.get('styles') || '[]'))packages/common/js/preview.js (4)
34-35
: BroadcastChannel is never closed - potential resource leak.The
previewChannel
instance is created but never closed, which could lead to resource leaks.Add a function to close the channel when all preview windows are closed:
+const closePreviewChannel = () => { + try { + previewChannel.close() + } catch (error) { + console.error('Failed to close preview channel:', error) + } +} +// Update line 127 to call this function when appropriate +if (!previewWindow || previewWindow.closed) { + cleanupSchemaChangeListener() + closePreviewChannel() + return +}
94-103
: Avoid using insecure origin in postMessage.Using
previewWindow.origin
can be undefined, causing security risks by sending messages to any origin.const sendSchemaUpdate = (data) => { + let targetOrigin = window.location.origin + try { + // May throw if preview is cross-origin + targetOrigin = new URL(previewWindow.location.href).origin + } catch (error) { + console.warn('[preview] Fallback to same-origin target for postMessage') + } previewWindow.postMessage( { source: 'designer', type: 'schema', data }, - previewWindow.origin || window.location.origin + targetOrigin ) }
168-175
: Add error handling for URL parsing.The code attempts to create URL objects without error handling, which could break if the origin is invalid.
window.addEventListener('message', async (event) => { - const parsedOrigin = new URL(event.origin) - const parsedHost = new URL(window.location.href) + let parsedOrigin, parsedHost + try { + parsedOrigin = new URL(event.origin) + parsedHost = new URL(window.location.href) + } catch (error) { + console.warn('[preview] Invalid origin or URL parsing error:', error) + return + } // 确保消息来源安全 if (parsedOrigin.origin === parsedHost.origin || parsedOrigin.host === parsedHost.host) {
200-218
: Fix insecure origin checking in handleHistoryPreview.Using
includes()
for origin checking is insecure, andpreviewWindow.origin
may be undefined.- if (event.origin === window.location.origin || event.origin.includes(window.location.hostname)) { + if (event.origin === window.location.origin) { const { event: eventType, source } = event.data || {} if (source === 'preview' && eventType === 'onMounted' && historyPreviewWindow) { const { scripts, styles, ancestors = [], ...rest } = params historyPreviewWindow.postMessage( { source: 'designer', type: 'schema', data: deepClone({ currentPage: rest, ancestors, scripts, styles }) }, - previewWindow.origin || window.location.origin + window.location.origin )
🧹 Nitpick comments (4)
packages/design-core/src/preview/src/preview/usePreviewData.ts (3)
23-32
: Add a proper TypeScript comment for the TODO line.The TODO comment at line 23 is written in Chinese and lacks context about what needs to be extracted into common TS types. Consider adding a more descriptive English comment with specific action items.
-// TODO: 后面可以是否可以抽取公共的 ts 类型定义 +// TODO: Extract these interfaces into common TS type definitions for reuse across preview-related components
339-341
: Fix TypeScript ignore comment.Avoid using
@ts-ignore
as it hides potential type issues. Instead, properly type the return value fromgetMetaApi
.- // @ts-ignore - const { getAllNestedBlocksSchema, generatePageCode } = getMetaApi('engine.service.generateCode') + const codeService = getMetaApi('engine.service.generateCode') as { + getAllNestedBlocksSchema: (content: any, fetchFn: any, blockSet: Set<any>) => Promise<any[]>, + generatePageCode: (content: any, componentsMap: any[], options: any, nextPage?: string) => string + } + const { getAllNestedBlocksSchema, generatePageCode } = codeService
47-86
: Refactor URL update logic for better readability.The
updateUrl
function contains complex nested conditionals and unclear logic flow. Consider refactoring for clarity.const updateUrl = (params: IPage, deps: IDeps) => { const searchParams = new URLSearchParams(location.search) const pageId = searchParams.get('pageid') const blockId = searchParams.get('blockid') const scripts = searchParams.get('scripts') const styles = searchParams.get('styles') - let shouldUpdate = false + + // Update dependency parameters if changed + const updateIfChanged = (key: string, value: any, currentValue: string | null) => { + if (value && JSON.stringify(value) !== currentValue) { + searchParams.set(key, JSON.stringify(value)) + return true + } + return false + } + + let shouldUpdate = updateIfChanged('scripts', deps.scripts, scripts) || + updateIfChanged('styles', deps.styles, styles) - if (deps.scripts && JSON.stringify(deps.scripts) !== scripts) { - searchParams.set('scripts', JSON.stringify(deps.scripts)) - shouldUpdate = true - } - - if (deps.styles && JSON.stringify(deps.styles) !== styles) { - searchParams.set('styles', JSON.stringify(deps.styles)) - shouldUpdate = true - } + // Check if we need to update ID parameters + const isBlock = params.page_content.componentName === COMPONENT_NAME.Block + const isPage = params.page_content.componentName === COMPONENT_NAME.Page + const currentIdMatch = (isBlock && !pageId && Number(blockId) === params.id) || + (isPage && !blockId && Number(pageId) === params.id) + if ( !shouldUpdate && - // 如果当前页面是区块,且没有pageid,且blockid与当前区块id相同,则不更新url - ((params.page_content.componentName === COMPONENT_NAME.Block && !pageId && Number(blockId) === params.id) || - // 如果当前页面是页面,且没有blockid,且pageid与当前页面id相同,则不更新url - (params.page_content.componentName === COMPONENT_NAME.Page && !blockId && Number(pageId) === params.id)) + currentIdMatch ) { return } - if (params.page_content.componentName === COMPONENT_NAME.Block) { + if (isBlock) { searchParams.delete('pageid') searchParams.set('blockid', String(params.id)) } - if (params.page_content.componentName === COMPONENT_NAME.Page) { + if (isPage) { searchParams.delete('blockid') searchParams.set('pageid', String(params.id)) } window.history.replaceState({}, '', `?${searchParams.toString()}`) }packages/common/js/preview.js (1)
267-276
: Document the customPreviewUrl functionality.The
customPreviewUrl
parameter provides powerful functionality but lacks documentation about its usage patterns.Add JSDoc comments explaining the functionality and providing examples:
+/** + * Determines the preview URL based on configuration + * + * The customPreviewUrl can be: + * 1. A string: Direct URL to use for preview (query params will be appended) + * 2. A function: Receives default URL and query string, returns custom URL + * Example: (defaultUrl, query) => `https://custom-domain.com/preview?${query}&extra=param` + */ // 从预览组件配置获取自定义URL const customPreviewUrl = getMergeMeta('engine.toolbars.preview')?.options?.previewUrl const defaultPreviewUrl = isDevelopEnv ? `./preview.html` : `${href.endsWith('/') ? href : `${href}/`}preview`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
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)
✅ Files skipped from review due to trivial changes (1)
- packages/design-core/package.json
🚧 Files skipped from review as they are similar to previous changes (12)
- packages/toolbars/preview/meta.js
- packages/design-core/src/preview/src/preview/importMap.js
- packages/plugins/block/src/BlockSetting.vue
- packages/common/package.json
- packages/design-core/src/preview/src/preview/http.js
- packages/plugins/page/src/PageHistory.vue
- packages/design-core/src/preview/src/preview/usePreviewCommunication.ts
- packages/design-core/src/preview/src/Toolbar.vue
- packages/design-core/src/preview/src/preview/generate.js
- packages/design-core/src/preview/src/preview/Preview.vue
- packages/plugins/page/src/composable/usePage.ts
- packages/toolbars/preview/src/Main.vue
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/design-core/src/preview/src/preview/usePreviewData.ts (6)
packages/design-core/src/preview/src/preview/http.js (14)
getPageById
(48-48)getPageById
(48-48)fetchPageHistory
(50-51)fetchPageHistory
(50-51)getBlockById
(49-49)getBlockById
(49-49)fetchImportMap
(39-42)fetchImportMap
(39-42)fetchAppSchema
(44-44)fetchAppSchema
(44-44)fetchMetaData
(31-37)fetchMetaData
(31-37)fetchBlockSchema
(45-46)fetchBlockSchema
(45-46)packages/design-core/src/preview/src/preview/importMap.js (2)
getImportMap
(31-38)getImportMap
(31-38)packages/register/src/common.ts (1)
getMetaApi
(54-64)packages/design-core/src/preview/src/constant/index.js (1)
PanelType
(13-15)packages/design-core/src/preview/src/preview/srcFiles.js (1)
srcFiles
(27-27)packages/design-core/src/preview/src/preview/generate.js (3)
res
(158-158)processAppJsCode
(153-167)processAppJsCode
(153-167)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (1)
packages/common/js/preview.js (1)
242-243
:❓ Verification inconclusive
Optimize parameter passing to prevent URL length issues.
Adding large JSON stringified objects to the URL can cause truncation and failures with long parameters.
Consider using session storage or localStorage instead of URL parameters for scripts and styles:
- query += `&platform=${platform}&scripts=${JSON.stringify(scripts)}&styles=${JSON.stringify(styles)}` + query += `&platform=${platform}&previewId=${Date.now()}` + + // Store large objects in storage instead of URL + sessionStorage.setItem(`tiny-engine-preview-deps-${Date.now()}`, JSON.stringify({ + scripts, + styles + }))Run this script to check the potential size of the URL parameters:
🏁 Script executed:
#!/bin/bash # Check if current implementation could lead to excessively long URLs # Find and analyze scripts object structure rg -A 5 "getScriptAndStyleDeps" --type js echo "---" # Find usages of JSON.stringify with scripts/styles rg "JSON\.stringify\(.*scripts" --type js echo "---" # Check if there are any existing implementations using storage rg "sessionStorage|localStorage" --type js | grep -i "preview"Length of output: 1949
🏁 Script executed:
#!/bin/bash # Display the context around the query construction in preview.js rg -n -C5 "let query" --type js packages/common/js/preview.jsLength of output: 610
Avoid embedding large JSON in URLs; store dependencies separately
Including
JSON.stringify(scripts)
andJSON.stringify(styles)
directly in the query string can easily push the URL past browser limits. Instead, generate a smallpreviewId
and persist the full payload in session (or local) storage.Locations to update:
- packages/common/js/preview.js: around lines 240–243 (where
query
is built)Suggested diff:
- query += `&platform=${platform}&scripts=${JSON.stringify(scripts)}&styles=${JSON.stringify(styles)}` + // Use a short previewId in the URL and store full deps in sessionStorage + const previewId = Date.now().toString() + query += `&platform=${platform}&previewId=${previewId}` + + sessionStorage.setItem( + `tiny-engine-preview-deps-${previewId}`, + JSON.stringify({ scripts, styles }) + )Please validate that the serialized size of your
scripts
andstyles
payload would exceed safe URL lengths in your use cases, and adjust storage expiration or cleanup as needed.
点保存会触发预览页面热更新是因为点保存的时候,手动重新加载了页面,可能需要明确下点保存的时候,为什么要重新加载页面,如果没有必要,那么我理解之间在保存页面插件就可以优化了。 单页/多页视图的场景:后续可以逐步优化。 |
Point save will trigger the hot update of the preview page because when point saves, the page is manually reloaded. It may be necessary to clarify why the page needs to be reloaded when point saves. If it is not necessary, then I understand that the page can be optimized by the save page plugin. Scenarios for single page/multi-page view: they can be gradually optimized later. |
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
♻️ Duplicate comments (3)
packages/design-core/src/preview/src/preview/usePreviewData.ts (3)
90-155
: Add Error Handling for API CallsThe recursive page fetching and block retrieval functions lack error handling. If API calls fail, the Promise rejections could cause the entire preview to fail silently.
#!/bin/bash # Verify if there are other instances of API calls with proper error handling in the codebase # Check for try/catch patterns around API calls echo "Looking for error handling patterns around API calls:" rg --type ts --type js "try\s*\{[^}]*\b(get|post|put|delete|fetch)\b[^}]*\}\s*catch" -A 2 -B 2
306-324
: Validate / Sanitize Dynamically Transformed ScriptsBabel-transforming user-provided code without validation poses a security risk, especially if the content comes from user inputs.
#!/bin/bash # Check if there are any sanitization or validation patterns before code transformation # Look for sanitization patterns echo "Checking for code sanitization before transformation:" rg --type ts --type js "(sanitize|validate|escape|filter).*\b(transform|babel|eval|compile)\b" -A 2 -B 2 # Look for security validation functions echo "Checking for security validation functions:" rg --type ts --type js "security.*\b(check|validate|sanitize)\b" -A 2 -B 2
373-377
:⚠️ Potential issue
styles
query param expected to be an object but set as an arrayIn
processAppJsCode
, the function expects an array for thecssList
parameter, butsearchParams.get('styles') || '{}'
may parse into an object instead of an array.-const appJsCode = processAppJsCode(newFiles['app.js'], JSON.parse(searchParams.get('styles') || '{}')) +const appJsCode = processAppJsCode(newFiles['app.js'], JSON.parse(searchParams.get('styles') || '[]'))
🧹 Nitpick comments (5)
packages/design-core/src/preview/src/preview/usePreviewData.ts (5)
67-75
: Simplify complex conditional logicThe condition for determining whether to update the URL is overly complex and difficult to understand at a glance.
Consider extracting these conditions into named helper functions for better readability:
- if ( - !shouldUpdate && - // 如果当前页面是区块,且没有pageid,且blockid与当前区块id相同,则不更新url - ((params.page_content.componentName === COMPONENT_NAME.Block && !pageId && Number(blockId) === params.id) || - // 如果当前页面是页面,且没有blockid,且pageid与当前页面id相同,则不更新url - (params.page_content.componentName === COMPONENT_NAME.Page && !blockId && Number(pageId) === params.id)) - ) { + const isCurrentBlock = () => + params.page_content.componentName === COMPONENT_NAME.Block && + !pageId && + Number(blockId) === params.id; + + const isCurrentPage = () => + params.page_content.componentName === COMPONENT_NAME.Page && + !blockId && + Number(pageId) === params.id; + + if (!shouldUpdate && (isCurrentBlock() || isCurrentPage())) { return }
23-23
: Implement the suggested type extractionThe TODO comment indicates a need to extract common TS type definitions, which would improve code organization and reusability.
Consider creating a shared types file in a common location like
packages/design-core/src/preview/src/types/index.ts
that exports these interfaces for reuse across the codebase.
387-394
: Add loading state managementThe
loadInitialData
function doesn't handle loading states, which could lead to poor user experience when data fetching is slow.Consider adding loading state management:
+ const loadingState = reactive({ + isLoading: false, + error: null + }) const loadInitialData = async () => { + loadingState.isLoading = true + try { const { currentPage, ancestors } = await getPageOrBlockByApi() previewState.currentPage = currentPage previewState.ancestors = ancestors if (currentPage) { updatePreview({ currentPage, ancestors }) } + } catch (error) { + loadingState.error = error + console.error('Failed to load initial preview data:', error) + } finally { + loadingState.isLoading = false + } } return { loadInitialData, updateUrl, updatePreview, + loadingState }
335-339
: Optimize import map updates to prevent unnecessary re-rendersThe code unnecessarily stringifies the entire import map for comparison, which is inefficient for large objects.
- if (JSON.stringify(previewState.importMap) !== JSON.stringify(importMapData)) { + // Use a deeper equality check without full stringification or use a library like fast-deep-equal + const importMapChanged = !previewState.importMap.imports || + Object.keys(importMapData.imports).some(key => + !previewState.importMap.imports[key] || + previewState.importMap.imports[key] !== importMapData.imports[key] + ); + + if (importMapChanged) { store.setImportMap(importMapData) previewState.importMap = importMapData }
330-358
: Add progress reporting for complex multi-step operationsThe
updatePreview
function performs multiple async operations without reporting progress, which could lead to a poor user experience for complex pages with many blocks.Consider adding a progress reporting mechanism for the various async operations, particularly for block fetching which might take time for complex pages.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/design-core/src/preview/src/preview/usePreviewData.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/design-core/src/preview/src/preview/usePreviewData.ts (6)
packages/design-core/src/preview/src/preview/http.js (14)
getPageById
(48-48)getPageById
(48-48)fetchPageHistory
(50-51)fetchPageHistory
(50-51)getBlockById
(49-49)getBlockById
(49-49)fetchImportMap
(39-42)fetchImportMap
(39-42)fetchAppSchema
(44-44)fetchAppSchema
(44-44)fetchMetaData
(31-37)fetchMetaData
(31-37)fetchBlockSchema
(45-46)fetchBlockSchema
(45-46)packages/design-core/src/preview/src/preview/importMap.js (2)
getImportMap
(31-38)getImportMap
(31-38)packages/register/src/common.ts (1)
getMetaApi
(54-64)packages/design-core/src/preview/src/constant/index.js (1)
PanelType
(13-15)packages/design-core/src/preview/src/preview/srcFiles.js (1)
srcFiles
(27-27)packages/design-core/src/preview/src/preview/generate.js (3)
res
(158-158)processAppJsCode
(153-167)processAppJsCode
(153-167)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
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
♻️ Duplicate comments (6)
packages/common/js/preview.js (3)
91-100
:⚠️ Potential issueFix security vulnerability in postMessage origin.
The current implementation uses
previewWindow.origin || window.location.origin
as the target origin, but Window objects don't have anorigin
property. This will always fall back to using window.location.origin, which might allow sending messages to unintended recipients if the preview window is cross-origin.const sendSchemaUpdate = (data) => { + let targetOrigin = window.location.origin + try { + // May throw if preview is cross-origin + targetOrigin = new URL(previewWindow.location.href).origin + } catch (_) { + console.warn('[preview] Fallback to same-origin target for postMessage') + } previewWindow.postMessage( { source: 'designer', type: 'schema', data }, - previewWindow.origin || window.location.origin + targetOrigin ) }
165-182
:⚠️ Potential issueImprove origin validation to handle invalid origins and prevent subdomain attacks.
The current implementation creates URL objects without proper error handling and is vulnerable to subdomain attacks. For example, a malicious site on
evil.your-domain.com
would pass the current check againstyour-domain.com
.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) { + let parsedOrigin; + try { + parsedOrigin = new URL(event.origin); + } catch (error) { + // Invalid origin, ignore the message + return; + } + // 确保消息来源安全 - 只允许完全匹配的源 + if (parsedOrigin.origin === window.location.origin) {
202-220
:⚠️ Potential issueFix security issues in history preview message handling.
Similar to the issue with
sendSchemaUpdate
, the history preview handler has security issues with the target origin validation and postMessage target origin.- if (event.origin === window.location.origin || event.origin.includes(window.location.hostname)) { + if (event.origin === window.location.origin) { const { event: eventType, source } = event.data || {} if (source === 'preview' && eventType === 'onMounted' && historyPreviewWindow) { const { scripts, styles, ancestors = [], ...rest } = params historyPreviewWindow.postMessage( { source: 'designer', type: 'schema', data: deepClone({ currentPage: rest, ancestors, scripts, styles }) }, - previewWindow.origin || window.location.origin + window.location.origin )packages/design-core/src/preview/src/preview/usePreviewData.ts (3)
100-155
: 🛠️ Refactor suggestionAdd error handling for API calls.
The current implementation doesn't include error handling for API calls, which could lead to unhandled promise rejections if the API requests fail. This is particularly important for the page/block fetching functions since they're critical for the preview functionality.
const getPageOrBlockByApi = async (): Promise<{ currentPage: IPage | null; ancestors: IPage[] }> => { const searchParams = new URLSearchParams(location.search) const pageId = searchParams.get('pageid') const blockId = searchParams.get('blockid') const history = searchParams.get('history') + try { if (pageId) { let ancestors = (await getPageRecursively(pageId)).reverse() let currentPage = await getPageById(pageId) if (history) { const historyList: IPage[] = await fetchPageHistory(pageId) currentPage = historyList.find((item) => item.id === Number(history)) ancestors = ancestors.map((item) => { if (item.id === Number(pageId)) { return { ...item, page_content: currentPage.page_content } } return item }) } return { currentPage, ancestors } } if (blockId) { const block = await getBlockById(blockId) let pageContent = block.content let name = block.name || block.name_cn || block.label if (history) { const historyContent = (block.histories || []).find((item: { id: number }) => item.id === Number(history)) pageContent = historyContent?.content || pageContent name = historyContent?.name || name } return { currentPage: { ...block, page_content: pageContent, name }, ancestors: [] } } return { currentPage: null, ancestors: [] } + } catch (error) { + console.error('Failed to fetch page or block data:', error) + return { + currentPage: null, + ancestors: [] + } + } }The same error handling should be applied to other API calls in the file.
305-323
:⚠️ Potential issueAdd security measures and error handling for code transformation.
The Babel transformation doesn't have proper error handling and could be a security risk if processing untrusted code. Consider adding validation and error handling to make this more robust.
- const newPanelValue = panelValue.replace(/<script\s*setup\s*>([\s\S]*)<\/script>/, (match, p1) => { + let newPanelValue; + try { + newPanelValue = panelValue.replace(/<script\s*setup\s*>([\s\S]*)<\/script>/, (match, p1) => { if (!p1) { // eslint-disable-next-line no-useless-escape return '<script setup></script>' } + // Basic validation to reject potentially malicious code + if (/eval\s*\(|Function\s*\(|new\s+Function/i.test(p1)) { + console.warn('Potentially unsafe code detected, skipping transformation') + return match // Return original without transformation + } + const transformedScript = transformSync(p1, { babelrc: false, plugins: [[vueJsx, { pragma: 'h' }]], sourceMaps: false, configFile: false }) const res = `<script setup>${transformedScript?.code || ''}` // eslint-disable-next-line no-useless-escape const endTag = '</script>' return `${res}${endTag}` }) + } catch (error) { + console.error('Error transforming script:', error) + newPanelValue = panelValue // Fallback to original on error + }
372-372
:⚠️ Potential issueFix type mismatch with styles parameter.
The
processAppJsCode
function expects an array forstyles
, but you're passing'[]'
as a default which is inconsistent with how the styles are collected and passed in other parts of the code.- const appJsCode = processAppJsCode(newFiles['app.js'], JSON.parse(searchParams.get('styles') || '[]')) + const appJsCode = processAppJsCode( + newFiles['app.js'], + JSON.parse(searchParams.get('styles') || '[]') + )Make sure this is consistent with how styles are collected and passed in the preview.js file. Either use an array consistently or update the processAppJsCode function to handle both array and object types.
🧹 Nitpick comments (2)
packages/common/js/preview.js (1)
242-244
: Consider alternative approach for passing large style and script data.Encoding potentially large objects (scripts and styles) directly in the URL can exceed URL length limits in some browsers. Consider using sessionStorage or postMessage for passing this data instead.
Instead of passing this data via URL parameters:
- query += `&platform=${platform}&scripts=${JSON.stringify(scripts)}&styles=${JSON.stringify(styles)}` + query += `&platform=${platform}` + + // Store large data in sessionStorage with a unique key + const dataKey = `preview-data-${Date.now()}` + sessionStorage.setItem(dataKey, JSON.stringify({ scripts, styles })) + query += `&dataKey=${dataKey}`Then in the preview page, retrieve the data from sessionStorage using the key.
packages/design-core/src/preview/src/preview/usePreviewData.ts (1)
328-382
: Optimize preview update process for better performance.The current implementation regenerates all code for blocks and ancestors on every update, which could be inefficient for large applications. Consider implementing caching or partial updates for better performance.
// 根据新的参数更新预览 const updatePreview = async (params: { currentPage: IPage; ancestors: IPage[] }) => { + // Cache key based on current pages and ancestors to avoid redundant processing + const cacheKey = JSON.stringify({ + currentPageId: params.currentPage?.id, + ancestorIds: (params.ancestors || []).map(a => a.id) + }) + + // Check if we have a cached version and nothing has changed + if ( + updatePreview.lastCacheKey === cacheKey && + updatePreview.cachedFiles + ) { + // Use cached files if nothing has changed + setFiles(updatePreview.cachedFiles) + return + } const { appData, metaData, importMapData } = await getBasicData(basicFiles) previewState.currentPage = params.currentPage previewState.ancestors = params.ancestors if (JSON.stringify(previewState.importMap) !== JSON.stringify(importMapData)) { store.setImportMap(importMapData) previewState.importMap = importMapData } const blockSet = new Set() let blocks = [] const { getAllNestedBlocksSchema, generatePageCode } = getMetaApi('engine.service.generateCode') if (params.ancestors?.length) { const promises = params.ancestors.map((item) => getAllNestedBlocksSchema(item.page_content, fetchBlockSchema, blockSet) ) blocks = (await Promise.all(promises)).flat() } const currentPageBlocks = await getAllNestedBlocksSchema( params.currentPage?.page_content || {}, fetchBlockSchema, blockSet ) blocks = blocks.concat(currentPageBlocks) const pageCode = [ ...getPageAncestryFiles(appData, params), ...(blocks || []).map((blockSchema) => { return { panelName: `${blockSchema.fileName}.vue`, panelValue: generatePageCode(blockSchema, appData?.componentsMap || [], { blockRelativePath: './' }) || '', panelType: 'vue' } }) ] 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 pageCode.map(fixScriptLang).forEach((item) => assignFiles(item, newFiles)) const metaFiles = generateMetaFiles(metaData) Object.assign(newFiles, metaFiles) setFiles(newFiles) + + // Cache the results + updatePreview.lastCacheKey = cacheKey + updatePreview.cachedFiles = { ...newFiles } } + +// Initialize cache storage on the function itself +updatePreview.lastCacheKey = '' +updatePreview.cachedFiles = nullThis optimization adds caching based on page and ancestor IDs, avoiding redundant processing when the same preview is requested multiple times.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/common/js/preview.js
(1 hunks)packages/design-core/src/preview/src/preview/usePreviewData.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/design-core/src/preview/src/preview/usePreviewData.ts (7)
packages/design-core/src/preview/src/preview/http.js (14)
getPageById
(48-48)getPageById
(48-48)fetchPageHistory
(50-51)fetchPageHistory
(50-51)getBlockById
(49-49)getBlockById
(49-49)fetchImportMap
(39-42)fetchImportMap
(39-42)fetchAppSchema
(44-44)fetchAppSchema
(44-44)fetchMetaData
(31-37)fetchMetaData
(31-37)fetchBlockSchema
(45-46)fetchBlockSchema
(45-46)packages/design-core/src/preview/src/preview/importMap.js (2)
getImportMap
(31-38)getImportMap
(31-38)packages/register/src/common.ts (1)
getMetaApi
(54-64)packages/vue-generator/src/generator/page.js (1)
generatePageCode
(464-502)packages/design-core/src/preview/src/constant/index.js (1)
PanelType
(13-15)packages/design-core/src/preview/src/preview/srcFiles.js (1)
srcFiles
(27-27)packages/design-core/src/preview/src/preview/generate.js (3)
res
(158-158)processAppJsCode
(153-167)processAppJsCode
(153-167)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
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
♻️ Duplicate comments (9)
packages/common/js/preview.js (5)
91-100
:⚠️ Potential issueFix incorrect use of
previewWindow.origin
.Window objects don't have an
origin
property. This evaluates toundefined
, causing potential cross-origin messaging issues.Apply this fix:
const sendSchemaUpdate = (data) => { + let targetOrigin = window.location.origin + try { + // May throw if preview is cross-origin + targetOrigin = new URL(previewWindow.location.href).origin + } catch (_) { + console.warn('[preview] Fallback to same-origin target for postMessage') + } previewWindow.postMessage( { source: 'designer', type: 'schema', data }, - previewWindow.origin || window.location.origin + targetOrigin ) }
166-170
:⚠️ Potential issueWrap URL creation in try/catch to handle invalid origins.
If
event.origin
is "null" (from file: or about: URLs),new URL(event.origin)
will throw an error, breaking message handling.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) { + let parsedOrigin; + try { + parsedOrigin = new URL(event.origin) + const parsedHost = new URL(window.location.href) + // 确保消息来源安全 + if (parsedOrigin.origin === parsedHost.origin || parsedOrigin.host === parsedHost.host) {
185-195
:⚠️ Potential issueBroadcastChannel is closed immediately after use.
Creating and immediately closing the BroadcastChannel prevents bidirectional communication. This channel should remain open to receive responses.
// 创建 BroadcastChannel 实例用于通信 const previewChannel = new BroadcastChannel('tiny-engine-preview-channel') // 可能是刷新,需要重新建立连接 previewChannel.postMessage({ event: 'connect', source: 'designer' }) -previewChannel.close()
203-221
:⚠️ Potential issueFix history preview origin check and messaging.
The origin check is insecure, and the messaging uses the same incorrect origin pattern as before.
- if (event.origin === window.location.origin || event.origin.includes(window.location.hostname)) { + let targetOrigin = window.location.origin; + try { + 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 || {} if (source === 'preview' && eventType === 'onMounted' && historyPreviewWindow) { const { scripts, styles, ancestors = [], ...rest } = params historyPreviewWindow.postMessage( { source: 'designer', type: 'schema', data: deepClone({ currentPage: rest, ancestors, scripts, styles }) }, - previewWindow.origin || window.location.origin + targetOrigin )
270-282
:⚠️ Potential issueValidate custom preview URLs to prevent open redirect vulnerabilities.
The current implementation allows custom preview URLs without validation, which could enable open redirect attacks.
if (customPreviewUrl) { // 如果配置了自定义预览URL,则使用自定义URL + let customUrl = ''; + + // Get the URL from the function or use the string directly + if (typeof customPreviewUrl === 'function') { + customUrl = customPreviewUrl(defaultPreviewUrl, query); + } else { + customUrl = `${customPreviewUrl}?${query}`; + } + + // Validate the URL to prevent open redirect + try { + const urlObj = new URL(customUrl, window.location.origin); + // Only allow same-origin URLs or specific trusted domains + if (urlObj.origin === window.location.origin || + /^https:\/\/(.*\.)?trusted-domain\.com$/.test(urlObj.origin)) { + openUrl = customUrl; + } else { + console.warn('[preview] Rejected potentially unsafe custom preview URL:', customUrl); + openUrl = `${defaultPreviewUrl}?${query}`; + } + } catch (_) { + console.warn('[preview] Invalid custom preview URL:', customUrl); + openUrl = `${defaultPreviewUrl}?${query}`; + } - openUrl = - typeof customPreviewUrl === 'function' - ? customPreviewUrl(defaultPreviewUrl, query) - : `${customPreviewUrl}?${query}`packages/design-core/src/preview/src/preview/usePreviewData.ts (4)
305-323
: 🛠️ Refactor suggestionAdd validation for dynamically transformed scripts.
Babel-transforming user-provided code without validation can pose security risks. Consider adding validation logic.
const newPanelValue = panelValue.replace(/<script\s*setup\s*>([\s\S]*)<\/script>/, (match, p1) => { if (!p1) { // eslint-disable-next-line no-useless-escape return '<script setup></script>' } + // Basic validation - reject potentially harmful code patterns + const maliciousPatterns = [ + /eval\s*\(/, // eval() calls + /Function\s*\(/, // Function constructor + /__proto__/, // prototype pollution attempts + /document\s*\.\s*cookie/ // cookie access + ]; + + if (maliciousPatterns.some(pattern => pattern.test(p1))) { + console.warn('Potentially unsafe script content detected and skipped'); + return '<script setup>console.error("Script content rejected for security reasons");</script>'; + } const transformedScript = transformSync(p1, { babelrc: false, plugins: [[vueJsx, { pragma: 'h' }]], sourceMaps: false, configFile: false }) const res = `<script setup>${transformedScript?.code || ''}` // eslint-disable-next-line no-useless-escape const endTag = '</script>' return `${res}${endTag}` })
189-190
: 🛠️ Refactor suggestionRemove implicit any typing for meta API.
Using
getMetaApi
without type checking can lead to runtime errors if the API is not available or has an unexpected shape.- const { generatePageCode } = getMetaApi('engine.service.generateCode') + const engineService = getMetaApi('engine.service.generateCode') as { + generatePageCode: ( + content: any, + componentsMap: Array<Record<string, string>>, + options: { blockRelativePath: string }, + nextPage?: string | null + ) => string + } + const { generatePageCode } = engineService
263-263
:⚠️ Potential issueFix inconsistent scripts default value.
Default value for scripts is
'{}'
(empty object), but later code (line 373) expects an array for styles.- getImportMap(JSON.parse(searchParams.get('scripts') || '{}')), + getImportMap(JSON.parse(searchParams.get('scripts') || '{}')),It's important to check that the default value for
styles
on line 373 is consistent with this pattern.
100-155
: 🛠️ Refactor suggestionAdd error handling for API calls.
The current implementation doesn't handle API failures, which could lead to unhandled exceptions and poor user experience.
const getPageOrBlockByApi = async (): Promise<{ currentPage: IPage | null; ancestors: IPage[] }> => { const searchParams = new URLSearchParams(location.search) const pageId = searchParams.get('pageid') const blockId = searchParams.get('blockid') const history = searchParams.get('history') + try { if (pageId) { let ancestors = (await getPageRecursively(pageId)).reverse() let currentPage = await getPageById(pageId) if (history) { const historyList: IPage[] = await fetchPageHistory(pageId) currentPage = historyList.find((item) => item.id === Number(history)) ancestors = ancestors.map((item) => { if (item.id === Number(pageId)) { return { ...item, page_content: currentPage.page_content } } return item }) } return { currentPage, ancestors } } if (blockId) { const block = await getBlockById(blockId) let pageContent = block.content let name = block.name || block.name_cn || block.label if (history) { const historyContent = (block.histories || []).find((item: { id: number }) => item.id === Number(history)) pageContent = historyContent?.content || pageContent name = historyContent?.name || name } return { currentPage: { ...block, page_content: pageContent, name }, ancestors: [] } } + } catch (error) { + console.error('Failed to fetch or parse page/block data:', error); + } return { currentPage: null, ancestors: [] } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/common/js/preview.js
(1 hunks)packages/design-core/src/preview/src/preview/usePreviewData.ts
(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
packages/design-core/src/preview/src/preview/usePreviewData.ts (7)
packages/design-core/src/preview/src/preview/http.js (14)
getPageById
(48-48)getPageById
(48-48)fetchPageHistory
(50-51)fetchPageHistory
(50-51)getBlockById
(49-49)getBlockById
(49-49)fetchImportMap
(39-42)fetchImportMap
(39-42)fetchAppSchema
(44-44)fetchAppSchema
(44-44)fetchMetaData
(31-37)fetchMetaData
(31-37)fetchBlockSchema
(45-46)fetchBlockSchema
(45-46)packages/design-core/src/preview/src/preview/importMap.js (2)
getImportMap
(31-38)getImportMap
(31-38)packages/register/src/common.ts (1)
getMetaApi
(54-64)packages/vue-generator/src/generator/page.js (1)
generatePageCode
(464-502)packages/design-core/src/preview/src/constant/index.js (1)
PanelType
(13-15)packages/design-core/src/preview/src/preview/srcFiles.js (1)
srcFiles
(27-27)packages/design-core/src/preview/src/preview/generate.js (3)
res
(158-158)processAppJsCode
(153-167)processAppJsCode
(153-167)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (5)
packages/common/js/preview.js (3)
13-24
: Good modularization of dependencies.The imports are well organized, breaking down meta register functionality into individual imports which makes the code more maintainable and aids tree-shaking.
123-126
: Good resource management.Setting
previewWindow = null
after detecting it's closed properly prevents memory leaks and dangling references, which is a good practice.
302-304
: Consolidated preview functions improve API simplicity.Refactoring to use a single
previewPage
function with anisHistory
flag simplifies the API and makes it more intuitive.packages/design-core/src/preview/src/preview/usePreviewData.ts (2)
23-42
: Well-structured reactive state with typed interfaces.The use of TypeScript interfaces for page structure and reactive state follows Vue 3 best practices and improves code maintainability.
385-393
: Good initial loading pattern with state management.The
loadInitialData
function properly separates data fetching from state updates, which is a good pattern for composable functions in Vue.
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?
【新Feature 描述】
配置跳转 URL 使用说明:
a. 直接配置 url:
http://tiny-engine-preview.com
适用场景:仅修改跳转 url,不修改 query 查询字符串部分
示例:
配置完成之后,会增加 query,然后跳转到配置的 url,比如:
http://tiny-engine-preview.com?tenant=1&id=1&...
b. 使用配置函数:
适用场景:需要增加自定义修改 query。可定制性高
示例:
【测试用例】
✅ 1. 编辑页面——打开预览页面,能够显示当前编辑页面的数据
✅ 2. 编辑页面——打开预览页面,修改页面数据,能够实时更新
✅ 3. 编辑区块——打开预览页面,修改区块数据,能够实时更新
✅ 4. 编辑页面——打开预览页面,新增函数工具类,绑定工具类,预览页面能够实时更新
✅ 5. 切换页面/区块,预览页面能够实时跟随切换、同时 url 参数的 pageid 和 blockid 会跟着变化
✅ 6. 删除页面/区块、新增页面/区块,预览页面能够跟随切换到删除后面的其他区块、页面
✅ 7. 直接打开 URL,URL 中包含 pageid,预览页面能够显示对应页面数据
✅ 8. 直接打开 URL,URL 中包含 blockid,预览页面能够显示对应区块数据
✅ 9. 包含多级超大页面,预览页面预期能够更新
✅ 10. 父级页面包含多个区块,预览页面能够正常显示
✅ 11. 从页面历史中打开预览页面,预览页面能够显示历史页面预览内容,且不会实时更新
✅ 12. 从区块历史中打开预览页面,预览页面能够显示历史区块预览内容,且不会实时更新
✅ 13. 刷新设计器页面,然后再次刷新预览页面,预期能够正常显示,修改设计器页面,预览页面能够实时更新
✅ 14. 已经有预览页面实例,再次点击预览,不再打开新的预览页面,而是打开当前的预览页面
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Summary by CodeRabbit
New Features
previewUrl
property to extend preview configuration options.Refactor
Chores
@vueuse/core
to runtime dependencies and added type definitions for Babel core.Documentation