Skip to content

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

Merged
merged 9 commits into from
Apr 27, 2025

Conversation

chilingling
Copy link
Member

@chilingling chilingling commented Mar 28, 2025

English | 简体中文

PR

PR Checklist

Please check if your PR fulfills the following requirements:

  • The commit message follows our Commit Message Guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)
  • Built its own designer, fully self-validated

PR Type

What kind of change does this PR introduce?

  • Bugfix
  • Feature
  • Code style update (formatting, local variables)
  • Refactoring (no functional changes, no api changes)
  • Build related changes
  • CI related changes
  • Documentation content changes
  • Other... Please describe:

Background and solution

What is the current behavior?

【背景】

  1. 当页面超大时,通过 URL 进行传递参数给预览页面可能会造成信息截断。造成预览失败的 bug。
  2. 页面预览没有实时更新的能力,用户体验不好。

Issue Number: N/A

What is the new behavior?

【新Feature 描述】

  1. 通信方式修改:修改设计页面与预览页面传参方式,通过 postMessage 进行传参。
  2. 增加实时预览能力:如果是预览当前编辑页,则通过监听 schema 变化实时传递参数给预览页面,实现实时预览的能力。
  3. 允许用户配置预览页面的跳转 url。
    配置跳转 URL 使用说明:
    a. 直接配置 url: http://tiny-engine-preview.com
    适用场景:仅修改跳转 url,不修改 query 查询字符串部分
    示例:
import { Preview } from '@opentiny/tiny-engine'
export default {
   toolbars: [
     [Preview, { options: { ...Preview.options,  previewUrl: 'http://tiny-engine-preview.com' } }]
   ]
}

配置完成之后,会增加 query,然后跳转到配置的 url,比如:http://tiny-engine-preview.com?tenant=1&id=1&...

b. 使用配置函数:
适用场景:需要增加自定义修改 query。可定制性高
示例:

import { Preview } from '@opentiny/tiny-engine'
export default {
   toolbars: [
     [
       Preview,
       {
          options: { 
              ...Preview.options,
              previewUrl: (originUrl, query) => {
                return `http://tiny-engine-preview.com?test=1&${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?

  • Yes
  • No

Other information

Summary by CodeRabbit

Summary by CodeRabbit

  • New Features

    • Enhanced preview functionality with event-driven schema synchronization and cross-window communication for improved live previews.
    • Added dynamic loading and updating of preview data, including scripts, styles, and page ancestry, with support for history mode.
    • Introduced a new previewUrl property to extend preview configuration options.
  • Refactor

    • Unified preview handling by consolidating block and page previews into a single streamlined process.
    • Simplified breadcrumb updates with reactive watchers and modularized preview data and communication logic.
    • Removed deprecated utilities and replaced URL parameter parsing with robust API-based data fetching.
    • Refactored preview component to use composables for data and communication management, improving maintainability.
    • Improved code generation logic to avoid duplicate CSS injections.
    • Reorganized page content update logic for better handling of page families.
    • Streamlined preview calls in toolbar and plugins by removing unnecessary parameters and conditional logic.
  • Chores

    • Moved @vueuse/core to runtime dependencies and added type definitions for Babel core.
    • Cleaned up unused imports and optimized preview invocation across plugins and toolbars.
  • Documentation

    • Added new frontend API documentation for preview configuration, including URL customization and hot reload toggling.
    • Updated documentation index and catalog to include the new preview API article.

Sorry, something went wrong.

Copy link
Contributor

coderabbitai bot commented Mar 28, 2025

Walkthrough

The 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

File(s) Change Summary
packages/common/js/preview.js
packages/design-core/src/preview/src/preview/Preview.vue
packages/design-core/src/preview/src/preview/usePreviewCommunication.ts
packages/design-core/src/preview/src/preview/usePreviewData.ts
packages/design-core/src/preview/src/preview/http.js
packages/design-core/src/preview/src/preview/importMap.js
Added new functions for schema handling, preview data management, message listening, and API methods; removed obsolete utilities to streamline communication with the preview window.
packages/design-core/src/preview/src/Toolbar.vue
packages/toolbars/preview/src/Main.vue
Updated reactive breadcrumb logic and simplified preview invocations by removing conditional block/page differentiation.
packages/design-core/src/preview/src/preview/generate.js
packages/common/package.json
packages/design-core/package.json
Revised CSS generation in JS to prevent duplicate calls; adjusted dependency management by moving @vueuse/core to regular dependencies and adding @types/babel__core as a dev dependency.
packages/plugins/block/src/BlockSetting.vue
packages/plugins/page/src/PageHistory.vue
packages/plugins/page/src/composable/usePage.ts
Modified preview function calls from previewBlock to previewPage, simplified parameter structures, and restructured page data updating for family pages.
packages/toolbars/preview/meta.js Added a new previewUrl property to the toolbar preview options configuration.

Sequence Diagram(s)

Loading
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

  • feat: preview support auto update #1327: Enhances preview window management and schema synchronization with robust communication and update mechanisms, closely related to the preview.js modifications in this PR.

Poem

I'm a rabbit with a hop so bright,
Skipping through code in the deep of night.
Schemas and previews now dance and play,
With messages sent in a seamless way.
Old code hops off, making room anew,
In this debugging warren, fresh and true.
Hoppy coding to all, from me to you! 🐇✨


📜 Recent review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 41aa847 and cd2736c.

📒 Files selected for processing (3)
  • docs/README.md (1 hunks)
  • docs/api/frontend-api/preview-api.md (1 hunks)
  • docs/catalog.json (1 hunks)
✅ Files skipped from review due to trivial changes (3)
  • docs/README.md
  • docs/api/frontend-api/preview-api.md
  • docs/catalog.json
⏰ Context from checks skipped due to timeout of 90000ms (1)
  • GitHub Check: push-check

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.

❤️ Share
🪧 Tips

Chat

There are 3 ways to chat with CodeRabbit:

  • Review comments: Directly reply to a review comment made by CodeRabbit. Example:
    • I pushed a fix in commit <commit_id>, please review it.
    • Generate unit testing code for this file.
    • Open a follow-up GitHub issue for this discussion.
  • Files and specific lines of code (under the "Files changed" tab): Tag @coderabbitai in a new review comment at the desired location with your query. Examples:
    • @coderabbitai generate unit testing code for this file.
    • @coderabbitai modularize this function.
  • PR comments: Tag @coderabbitai in a new PR comment to ask questions about the PR branch. For the best results, please provide a very specific query, as very limited context is provided in this mode. Examples:
    • @coderabbitai gather interesting stats about this repository and render them as a table. Additionally, render a pie chart showing the language distribution in the codebase.
    • @coderabbitai read src/utils.ts and generate unit testing code.
    • @coderabbitai read the files in the src/scheduler package and generate a class diagram using mermaid and a README in the markdown format.
    • @coderabbitai help me debug CodeRabbit configuration file.

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)

  • @coderabbitai pause to pause the reviews on a PR.
  • @coderabbitai resume to resume the paused reviews.
  • @coderabbitai review to trigger an incremental review. This is useful when automatic reviews are disabled for the repository.
  • @coderabbitai full review to do a full review from scratch and review all the files again.
  • @coderabbitai summary to regenerate the summary of the PR.
  • @coderabbitai generate docstrings to generate docstrings for this PR.
  • @coderabbitai generate sequence diagram to generate a sequence diagram of the changes in this PR.
  • @coderabbitai resolve resolve all the CodeRabbit review comments.
  • @coderabbitai configuration to show the current CodeRabbit configuration for the repository.
  • @coderabbitai help to get help.

Other keywords and placeholders

  • Add @coderabbitai ignore anywhere in the PR description to prevent this PR from being reviewed.
  • Add @coderabbitai summary to generate the high-level summary at a specific location in the PR description.
  • Add @coderabbitai anywhere in the PR title to generate the title automatically.

CodeRabbit Configuration File (.coderabbit.yaml)

  • You can programmatically configure CodeRabbit by adding a .coderabbit.yaml file to the root of your repository.
  • Please see the configuration documentation for more information.
  • If your editor has YAML language server enabled, you can add the path at the top of this file to enable auto-completion and validation: # yaml-language-server: $schema=https://coderabbit.ai/integrations/schema.v2.json

Documentation and Community

  • Visit our Documentation for detailed information on how to use CodeRabbit.
  • Join our Discord Community to get help, request features, and share feedback.
  • Follow us on X/Twitter for updates and announcements.

@github-actions github-actions bot added the enhancement New feature or request label Mar 28, 2025
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 checking

The updated processAppJsCode function now includes proper validation and duplicate prevention, which improves robustness and efficiency.

The implementation:

  1. Adds validation to check if cssList is a valid array
  2. Returns original code early if no CSS files need processing
  3. 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 reactivity

This 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 a data: any parameter. Relying on any 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 like onSchemaReceivedHandler or onSchemaChange 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 and loadInitialData 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 from usePreviewCommunication 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 via Promise.all, consider adding a top-level try/catch or logging mechanism to handle potential rejections from fetchPageDetailIfNeeded gracefully.

packages/common/js/preview.js (5)

33-48: Optimize merging of scripts and styles.
The approach is straightforward, but consider using Set or a uniqueness check to avoid duplicates if extended in the future.


90-100: Add safety check before posting messages.
Ensure previewWindow is still valid and not null before calling postMessage, 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 where href might be truncated or unusual.


264-266: Consolidate repeated logic if needed.
previewPage largely mirrors open(...). If usage diverges, keep them separate. Otherwise, consider merging for simplicity.

packages/design-core/src/preview/src/preview/usePreviewData.ts (6)

47-86: Avoid frequent replaceState 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 exploring history, you fetch the correct version of the page or block. Check boundary conditions when no matching history is found in arrays.


175-204: Generate code for pages and blocks.
getPageAncestryFiles elegantly reuses generatePageCode. Remember to handle edge cases when componentsMap 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 set previewState 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

📥 Commits

Reviewing files that changed from the base of the PR and between bec6f7c and 4a81dca.

📒 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 dependencies

Moving @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 parameter

Changing the getImportMap function to accept a scripts 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 data

The addition of these three API functions (getPageById, getBlockById, and fetchPageHistory) provides a clean interface for retrieving preview-related data. These endpoints follow a consistent pattern and properly utilize the existing getMetaApi utility.

packages/plugins/block/src/BlockSetting.vue (3)

91-91: Import statement cleanup

The unused imports have been removed, which improves code cleanliness.


93-93: Updated preview function import

The 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 the previewPage 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 js

Length of output: 74


Action Required: Validate All Consumers of the Updated previewPage Function

The simplified previewHistory function now directly invokes previewPage 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 updates

New imports support the reactive breadcrumb update approach implemented below.

packages/plugins/page/src/PageHistory.vue (1)

54-63: Streamlined preview history implementation

The 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 to previewPage 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, and loadInitialData 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 call loadInitialData() 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, and getOptions 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 removing previewBlock 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 to previewPage() 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 and usePreviewData 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, and updatePreview 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 and loadInitialData meets the composable’s contract and keeps logic neatly organized.


78-79: Proper lifecycle hooking.

Registering initCommunication on onMounted and cleaning up on onBeforeUnmount ensures the communication syncs exactly with the component’s lifecycle. Nicely done.

packages/plugins/page/src/composable/usePage.ts (2)

442-447: Validate currentPage before usage.
Consider adding null or undefined checks for currentPage or currentPage.id to prevent potential runtime errors when searching within familyPages.


489-502: Verify required properties in object mapping.
Each mapped property relies on the source object (item). Ensure that item 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 of previewWindow 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 on isBlock(). Confirm that it covers all relevant states (e.g., partial or missing data).


121-122: Good proactive instantiation.
Calling setupMessageListener() 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.
Using useThrottleFn 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: Interface IPage definition looks good.
Ensure that external references to IPage 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 existing getImportMap logic carefully.
Merging local bundle dependencies with existing import maps is good. Keep an eye on possible naming collisions in scripts.


168-173: Interface IPanelType is straightforward.
The optional index 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: Removing lang="jsx" ensures Repl support.
The fixScriptLang function resolves known constraints in the Vue Repl environment.


282-289: Interface IMetaDataParams is relevant.
Capturing history as optional is correct.


291-294: IUsePreviewData fosters clarity.
It clarifies what setFiles and store should be, ensuring type consistency.


296-298: Initialize with setFiles(srcFiles, 'src/Main.vue').
This is a practical approach to set your base environment.


391-396: Final API shape is well-defined.
Exposing loadInitialData, updateUrl, and updatePreview ensures easy external usage and maintainability.

Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 issue

Strengthen 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 handling
packages/common/js/preview.js (1)

195-224: ⚠️ Potential issue

Mitigate 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

📥 Commits

Reviewing files that changed from the base of the PR and between 4a81dca and 525ad0f.

📒 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.js

Length of output: 152


Verified: previewPage() correctly utilizes default parameters.
The function’s default parameter values (i.e., params = {} and isHistory = false) ensure that calling previewPage() 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, and usePage. 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/src

Length of output: 76


Action: Confirm simplified imports do not impact functionality

  • The recent search using rg in the packages/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.

@chilingling chilingling force-pushed the feat/previewSupportAutoUpdate branch from 525ad0f to 8c4aadf Compare March 29, 2025 06:54
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 functions

The 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 robustness

The 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

📥 Commits

Reviewing files that changed from the base of the PR and between 525ad0f and 8c4aadf.

📒 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 programming

The 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:

  1. Validation that cssList is a valid non-empty array
  2. Prevention of duplicate CSS references by checking if they already exist
  3. Proper code structure and spacing with newlines
packages/design-core/src/preview/src/Toolbar.vue (1)

17-23: LGTM: Addition of reactive imports

The 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 approach

The 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 in packages/common/js/preview.js now simply calls open(params, isHistory) without any conditional logic specifically for block previews. No references to a separate previewBlock 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 from packages/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.

@chilingling chilingling force-pushed the feat/previewSupportAutoUpdate branch from 8c4aadf to ee7d1b1 Compare April 8, 2025 12:33
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 retrieval

These 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 completed

The 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 implementation

The 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 updates

The 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 handling

The 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 Types

Line 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: Replace any 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 Parameters

The updateUrl function appends scripts and styles 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 Down updatePreview 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 in loadInitialData

Currently, if getPageOrBlockByApi fails or returns empty data, there’s no fallback behavior besides setting previewState to null. A more robust approach could handle user notifications or retries.

📜 Review details

Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 8c4aadf and ee7d1b1.

📒 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 imports

These 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 when options.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.js

Length of output: 314


Verified: previewPage() now properly handles default parameters internally

The call to previewPage() in packages/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 in packages/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 implemented onSchemaReceivedAction 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 the onSchemaReceivedAction 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:

  1. Extract specific properties from ancestor pages
  2. Handle page details with proper error handling
  3. 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. The deepClone 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:

  1. Detects the current context (block or page)
  2. Retrieves appropriate schema and metadata
  3. Assembles dependencies and ancestor data
  4. 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 using postMessage, with proper origin validation using previewWindow.origin || window.location.origin instead of the insecure wildcard origin.


105-164: Added schema change subscription with cleanup.

Implemented a comprehensive system to:

  1. Subscribe to schema changes with proper throttling
  2. Clean up listeners when no longer needed
  3. 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:

  1. Proper origin validation using URL parsing
  2. Connection management for preview window references
  3. 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:

  1. One-time data transmission to prevent ongoing updates
  2. Proper event listener cleanup after sending data
  3. 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:

  1. Current tenant and theme information
  2. Framework and platform information
  3. Resources and dependencies
  4. Page/block identifiers
  5. 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:

  1. Support custom preview URLs from configuration
  2. Handle history preview mode
  3. Focus existing preview windows instead of opening duplicates
  4. 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 Solid

The 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);
  ...
}

@hexqi hexqi added this to the v2.5.0 milestone Apr 16, 2025
@chilingling chilingling force-pushed the feat/previewSupportAutoUpdate branch from ee7d1b1 to 49561d8 Compare April 19, 2025 08:33
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 issue

Errors thrown by previewPage() are not caught

All 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 in try/catch and provide fallback UI or logging.


304-323: Unsafe Babel transformation of user content

The 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. via updateUrl) with a different theme, 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 when history mode closes the channel

cleanupCommunicationAction is reassigned after the callback that uses it is created, which is OK, but onSchemaReceivedAction never null‑checks before invoking.
If initCommunication fails or was never called, calling cleanupCommunicationAction() 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 change

Because 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.value
packages/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 to postMessage. 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

📥 Commits

Reviewing files that changed from the base of the PR and between ee7d1b1 and 49561d8.

📒 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 suggestion

URL comparison may trigger unnecessary history churn

styles and scripts are compared with JSON.stringify(...) !== searchParams.get(...).
URLSearchParams returns URL‑encoded strings, so the comparison often fails even when logically equal, causing replaceState 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.

Copy link
Collaborator

@hexqi hexqi left a 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变化,能否不重新热更新加载
  1. 路由特性有父页面场景,单页视图之前在预览没有支持,后面可以考虑支持(设计器子页面嵌套视图切换到单页视图,预览还是显示单页视图)

Verified

This commit was signed with the committer’s verified signature.

Verified

This commit was signed with the committer’s verified signature.

Verified

This commit was signed with the committer’s verified signature.
@chilingling chilingling force-pushed the feat/previewSupportAutoUpdate branch from 49561d8 to 35f25a3 Compare April 22, 2025 06:19
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 issue

Fix 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, and previewWindow.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 from getMetaApi.

-    // @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

📥 Commits

Reviewing files that changed from the base of the PR and between 49561d8 and 35f25a3.

📒 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.js

Length of output: 610


Avoid embedding large JSON in URLs; store dependencies separately

Including JSON.stringify(scripts) and JSON.stringify(styles) directly in the query string can easily push the URL past browser limits. Instead, generate a small previewId 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 and styles payload would exceed safe URL lengths in your use cases, and adjust storage expiration or cleanup as needed.

@chilingling
Copy link
Member Author

LGTM! 然后有一些点可以考虑下能否优化:

1.热更新白屏时间长(接近10s左右),有点影响体验,可以考虑:

  • 更新schema能否实现不刷新页面,依赖不重新加载

  • 部分场景可以考虑优化减少热更新次数:

    • 未锁定场景,会先修改刷新,再恢复刷新,未锁定能否不发送message
    • 修改属性后,已经热更新,再点击保存,没有schema变化,能否不重新热更新加载
  1. 路由特性有父页面场景,单页视图之前在预览没有支持,后面可以考虑支持(设计器子页面嵌套视图切换到单页视图,预览还是显示单页视图)
  1. 热更新白屏时间长的原因:每一个微小的改动都会导致 vue-repl sand box 的重新构建(每个文件都重新编译、所有 importMap 都重新请求)。后续可以考虑升级版本再尝试优化。
  2. 触发热更新的场景:
    • schema 变化
    • 页面/区块之间的切换

点保存会触发预览页面热更新是因为点保存的时候,手动重新加载了页面,可能需要明确下点保存的时候,为什么要重新加载页面,如果没有必要,那么我理解之间在保存页面插件就可以优化了。
未锁定场景:同理,这里我理解也不太应该直接改动 schema,后续需要优化。

单页/多页视图的场景:后续可以逐步优化。

@Issues-translate-bot
Copy link

Bot detected the issue body's language is not English, translate it automatically.


LGTM! Then there are some points to consider whether it can be optimized:

  1. The hot update time is long (nearly 10 seconds), which affects the experience a bit, so you can consider:
  • Can update schema not refresh the page, dependency not reload

  • Some scenarios can consider optimizing and reducing the number of hot updates:

  • In the scene that is not locked, the refresh will be modified and then the refresh will be restored. Can the message not be sent if it is not locked

  • After modifying the attribute, it has been hot updated. Click Save again. There is no schema change. Can you not re-update and load it?

  1. The routing feature has a parent page scene. The single page view was not supported before preview. You can consider supporting it later (the designer subpage nested view switches to single page view, preview or display single page view)
  1. The reason for the long time of hot updates to white screens: Every small change will cause the rebuild of vue-repl sand box (every file is recompiled and all importMaps are re-requested). You can consider upgrading the version and then try to optimize it later.
  2. Scenarios that trigger hot updates:
    • schema changes
    • Switch between pages/blocks

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.
Unlocked scenario: Similarly, I understand that schema should not be changed directly here, and it needs to be optimized later.

Scenarios for single page/multi-page view: they can be gradually optimized later.

Verified

This commit was signed with the committer’s verified signature.
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 Calls

The 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 Scripts

Babel-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 array

In processAppJsCode, the function expects an array for the cssList parameter, but searchParams.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 logic

The 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 extraction

The 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 management

The 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-renders

The 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 operations

The 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

📥 Commits

Reviewing files that changed from the base of the PR and between 35f25a3 and 22b180c.

📒 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

Verified

This commit was signed with the committer’s verified signature.
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 issue

Fix security vulnerability in postMessage origin.

The current implementation uses previewWindow.origin || window.location.origin as the target origin, but Window objects don't have an origin 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 issue

Improve 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 against your-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 issue

Fix 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 suggestion

Add 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 issue

Add 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 issue

Fix type mismatch with styles parameter.

The processAppJsCode function expects an array for styles, 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 = null

This 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

📥 Commits

Reviewing files that changed from the base of the PR and between 22b180c and 12a25c8.

📒 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

Verified

This commit was signed with the committer’s verified signature.
hexqi
hexqi previously approved these changes Apr 24, 2025

Verified

This commit was signed with the committer’s verified signature.
Copy link
Contributor

@coderabbitai coderabbitai bot left a 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 issue

Fix incorrect use of previewWindow.origin.

Window objects don't have an origin property. This evaluates to undefined, 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 issue

Wrap 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 issue

BroadcastChannel 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 issue

Fix 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 issue

Validate 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 suggestion

Add 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 suggestion

Remove 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 issue

Fix 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 suggestion

Add 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

📥 Commits

Reviewing files that changed from the base of the PR and between 105e511 and 41aa847.

📒 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 an isHistory 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.

Verified

This commit was signed with the committer’s verified signature.
@github-actions github-actions bot added the documentation Improvements or additions to documentation label Apr 27, 2025

Verified

This commit was signed with the committer’s verified signature.
@hexqi hexqi merged commit c3ae3b0 into opentiny:develop Apr 27, 2025
4 checks passed
@chilingling chilingling mentioned this pull request Apr 28, 2025
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants