Feat/set app preview: add app preview#1628
Conversation
WalkthroughAdds a preview type parameter across toolbar and preview flows, introducing page vs app preview branches and related HTTP fetches and code generation. Makes popover trigger configurable, updates toolbar UI to a two-option menu, adds a page list API, adjusts URL query building, switches many generator imports to alias paths, and removes axios mock. Changes
Sequence Diagram(s)sequenceDiagram
autonumber
actor U as User
participant TB as Toolbar (Main.vue)
participant API as previewPage()
participant UPD as usePreviewData.updatePreview
participant HTTP as HTTP helpers
participant FS as File Writer (setFiles)
U->>TB: Click "页面预览" or "应用预览"
TB->>API: previewPage({ previewType })
API->>UPD: updatePreview(previewType)
alt previewType === "page"
UPD->>UPD: getBasicData(...) ➜ app/meta/importMap
UPD->>HTTP: fetchBlockSchema(...)*
note right of HTTP: Aggregates nested & current page blocks
UPD->>UPD: build page ancestry files
UPD->>FS: write App.js, page files, meta files
else previewType === "app"
UPD->>HTTP: fetch app schema
UPD->>HTTP: fetchPageList(appId)
UPD->>UPD: generate routes and app code
UPD->>FS: write router, src files, import-map.json, App.js
end
UPD-->>API: done
API-->>TB: open preview URL (with previewType)
TB-->>U: Preview opens
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Poem
Pre-merge checks and finishing touches✅ Passed checks (3 passed)
✨ Finishing touches
🧪 Generate unit tests
Tip 👮 Agentic pre-merge checks are now available in preview!Pro plan users can now enable pre-merge checks in their settings to enforce checklists before merging PRs.
Please see the documentation for more information. Example: reviews:
pre_merge_checks:
custom_checks:
- name: "Undocumented Breaking Changes"
mode: "warning"
instructions: |
Pass/fail criteria: All breaking changes to public APIs, CLI flags, environment variables, configuration keys, database schemas, or HTTP/GraphQL endpoints must be documented in the "Breaking Change" section of the PR description and in CHANGELOG.md. Exclude purely internal or private changes (e.g., code not exported from package entry points or explicitly marked as internal).Please share your feedback with us on this Discord post. 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. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 7
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/vue-generator/src/templates/vue-template/templateFiles/src/http/index.js (1)
19-24: Propagate errors and guard against missing error.response in interceptor.On network errors
error.responsecan be undefined, and returning nothing swallows errors.Apply this diff:
http.interceptors.response.use(dataHandler, (error) => { - const response = error.response - if (response.status === 403 && response.headers && response.headers['x-login-url']) { + const response = error?.response + if (response?.status === 403 && response.headers && response.headers['x-login-url']) { // TODO 处理无权限时，重新登录再发送请求 } + return Promise.reject(error) })packages/vue-generator/src/templates/vue-template/templateFiles/src/lowcodeConfig/dataSource.js (1)
74-84: Do not swallow request/response errors in interceptors.
errorHandlerupdates state but never rejects; callers awaitingload()will not receive a rejection.Apply this diff:
const errorHandler = (error) => { if (config.errorHandler?.value) { createFn(config.errorHandler.value)(error) } dataSource.status = 'error' dataSource.error = error + return Promise.reject(error) }
🧹 Nitpick comments (3)
packages/vue-generator/src/templates/vue-template/templateFiles/src/lowcodeConfig/dataSource.js (1)
19-26: Avoid eval for user-supplied functions (security, stability).Prefer
new Function(...)with explicit args and a constrained scope; at minimum, document the risk and gate by trust level.packages/toolbars/preview/src/Main.vue (1)
3-9: Consider adding visual distinction between preview optionsThe two preview options look identical to users. Consider adding icons or different styling to help users distinguish between page and app preview modes.
You could enhance the UI with icons:
<div class="toolbar-preview-item"> - <span @click="preview('page')">页面预览</span> - <span @click="preview('app')">应用预览</span> + <span @click="preview('page')"> + <svg-icon name="page-icon"></svg-icon> + 页面预览 + </span> + <span @click="preview('app')"> + <svg-icon name="app-icon"></svg-icon> + 应用预览 + </span> </div>packages/design-core/src/preview/src/preview/usePreviewData.ts (1)
571-571: Hardcoded Element Plus CSS removal is brittleThe hardcoded string replacement for Element Plus CSS could break if the import format changes.
Use a more flexible pattern:
-srcFiles['main.js'] = srcFiles['main.js'].replace("import 'element-plus/dist/index.css'", '') +// Remove any element-plus CSS imports with various formats +srcFiles['main.js'] = srcFiles['main.js'].replace(/import\s+['"]element-plus\/[^'"]*\.css['"]\s*;?/g, '')
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (14)
packages/common/component/ToolbarBase.vue(1 hunks)packages/common/component/toolbar-built-in/ToolbarBaseIcon.vue(2 hunks)packages/common/js/preview.js(1 hunks)packages/design-core/src/preview/src/preview/http.js(1 hunks)packages/design-core/src/preview/src/preview/usePreviewData.ts(3 hunks)packages/toolbars/preview/src/Main.vue(4 hunks)packages/vue-generator/src/plugins/genI18nPlugin.js(2 hunks)packages/vue-generator/src/templates/vue-template/templateFiles/src/App.vue(1 hunks)packages/vue-generator/src/templates/vue-template/templateFiles/src/http/axios.js(0 hunks)packages/vue-generator/src/templates/vue-template/templateFiles/src/http/index.js(1 hunks)packages/vue-generator/src/templates/vue-template/templateFiles/src/lowcodeConfig/dataSource.js(2 hunks)packages/vue-generator/src/templates/vue-template/templateFiles/src/lowcodeConfig/lowcode.js(1 hunks)packages/vue-generator/src/templates/vue-template/templateFiles/src/lowcodeConfig/store.js(1 hunks)packages/vue-generator/src/templates/vue-template/templateFiles/src/main.js(1 hunks)
💤 Files with no reviewable changes (1)
- packages/vue-generator/src/templates/vue-template/templateFiles/src/http/axios.js
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-07-03T09:22:59.512Z
Learnt from: hexqi
PR: opentiny/tiny-engine#1501
File: mockServer/src/tool/Common.js:79-82
Timestamp: 2025-07-03T09:22:59.512Z
Learning: In the tiny-engine project, the mockServer code uses ES6 import syntax but is compiled to CommonJS output. This means CommonJS globals like `__dirname` are available at runtime, while ES6 module-specific features like `import.meta` would cause runtime errors.
Applied to files:
packages/design-core/src/preview/src/preview/usePreviewData.ts
📚 Learning: 2024-10-10T02:47:46.239Z
Learnt from: yy-wow
PR: opentiny/tiny-engine#850
File: packages/toolbars/preview/src/Main.vue:16-16
Timestamp: 2024-10-10T02:47:46.239Z
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.
Applied to files:
packages/toolbars/preview/src/Main.vue
📚 Learning: 2024-10-10T02:48:10.881Z
Learnt from: yy-wow
PR: opentiny/tiny-engine#850
File: packages/toolbars/preview/src/Main.vue:0-0
Timestamp: 2024-10-10T02:48:10.881Z
Learning: 在 `packages/toolbars/preview/src/Main.vue` 文件中，使用 `useNotify` 而不是 `console` 来记录错误日志。
Applied to files:
packages/toolbars/preview/src/Main.vue
🧬 Code graph analysis (3)
packages/vue-generator/src/plugins/genI18nPlugin.js (1)
packages/vue-generator/src/utils/generateImportStatement.js (1)
generateImportStatement(2-16)
packages/design-core/src/preview/src/preview/http.js (2)
packages/register/src/common.ts (1)
getMetaApi(20-30)packages/register/src/constants.ts (1)
META_SERVICE(1-23)
packages/design-core/src/preview/src/preview/usePreviewData.ts (5)
packages/register/src/common.ts (1)
getMetaApi(20-30)packages/design-core/src/preview/src/preview/http.js (4)
fetchBlockSchema(40-41)fetchBlockSchema(40-41)fetchPageList(48-48)fetchPageList(48-48)packages/vue-generator/src/generator/page.js (1)
generatePageCode(464-502)packages/design-core/src/preview/src/preview/generate.js (4)
processAppJsCode(153-167)processAppJsCode(153-167)res(158-158)result(21-21)packages/design-core/src/preview/src/preview/importMap.js (3)
importMap(16-16)getImportMap(40-47)getImportMap(40-47)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: push-check
🔇 Additional comments (14)
packages/vue-generator/src/templates/vue-template/templateFiles/src/App.vue (1)
8-8: Ensure '@' alias is configured in generated projects.Switching to '@/i18n/index.js' assumes the template’s bundler (e.g., Vite) defines '@' → 'src'. Please verify the generator also emits/updates the alias in the target app’s config to avoid unresolved imports.
packages/vue-generator/src/templates/vue-template/templateFiles/src/main.js (1)
14-16: LGTM: alias-based imports.The move to '@/router/index.js' and '@/App.vue' looks good and aligns with the template-wide change.
packages/vue-generator/src/plugins/genI18nPlugin.js (1)
61-62: LGTM: alias paths in generated index.js imports.The switch to '@/lowcodeConfig/lowcode.js' and '@/i18n/...’ is consistent with the alias migration.
packages/vue-generator/src/templates/vue-template/templateFiles/src/http/index.js (1)
13-14: LGTM: alias-based http module imports.Consistent with template changes; explicit .js extensions are fine with Vite ESM.
packages/vue-generator/src/templates/vue-template/templateFiles/src/lowcodeConfig/store.js (1)
1-1: LGTM: explicit index import.Using '@/stores/index.js' is clear and avoids resolver differences.
packages/vue-generator/src/templates/vue-template/templateFiles/src/lowcodeConfig/lowcode.js (1)
16-19: LGTM: alias imports for lowcode runtime deps.Matches the broader alias migration; no functional changes observed.
packages/vue-generator/src/templates/vue-template/templateFiles/src/lowcodeConfig/dataSource.js (3)
13-15: LGTM: alias-based imports for http and dataSources.Keeps generated code consistent with the new path strategy.
85-98: Guard http.mock usage or provide fallback.If the axios adapter doesn’t expose
mock, this will throw. Add a capability check.Apply this diff:
- if (import.meta.env?.VITE_APP_MOCK === 'mock') { - http.mock([ + if (import.meta.env?.VITE_APP_MOCK === 'mock' && typeof http.mock === 'function') { + http.mock([ { url: config.options?.uri, response() { return Promise.resolve([200, { data: config.data }]) } }, { url: '*', proxy: '*' } ]) }
85-85: LGTM: optional chaining on import.meta.env.Prevents crashes in non-Vite or non-browser contexts.
packages/common/component/ToolbarBase.vue (1)
36-39: LGTM! Clean prop addition for trigger controlThe trigger prop is properly defined with a sensible default value that maintains backward compatibility.
packages/design-core/src/preview/src/preview/http.js (1)
48-48: LGTM! Consistent with existing API patternsThe new
fetchPageListfunction follows the established pattern for API endpoints in this file.packages/common/component/toolbar-built-in/ToolbarBaseIcon.vue (1)
3-3: LGTM! Proper dynamic trigger binding implementationThe trigger prop is correctly defined and bound to the TinyPopover component, enabling flexible popover behavior control.
Also applies to: 41-44
packages/toolbars/preview/src/Main.vue (1)
32-32: LGTM! Clean parameter passing for preview typeThe preview function signature change and parameter passing are implemented correctly. Using
{ previewType }for the object literal is clean and follows ES6 best practices.Also applies to: 62-62
packages/design-core/src/preview/src/preview/usePreviewData.ts (1)
509-512: Potential null pointer exception in route generationIf
item.metais undefined, accessingitem.meta.isHomeoritem.meta.idwill throw an error.Add null checks:
pageSchema.forEach((item) => { - if ((item.meta?.isHome || item.meta?.isDefault) && !isGetHome) { - home.redirect = { name: `${item.meta.id}` } + if ((item.meta?.isHome || item.meta?.isDefault) && !isGetHome && item.meta?.id) { + home.redirect = { name: `${item.meta.id}` } isGetHome = true }⛔ Skipped due to learnings
Learnt from: gene9831 PR: opentiny/tiny-engine#830 File: packages/common/component/MetaChildItem.vue:50-56 Timestamp: 2024-10-15T02:45:17.168Z Learning: In `packages/common/component/MetaChildItem.vue`, when checking if `text` is an object in the computed property `title`, ensure that `text` is not `null` because `typeof null === 'object'` in JavaScript. Use checks like `text && typeof text === 'object'` to safely handle `null` values.
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (8)
designer-demo/useData.js (8)
49-69: Avoid Number(null) pitfalls in URL checks; compare as strings.Prevents accidental 0/NaN comparisons.
- (params.page_content.componentName === COMPONENT_NAME.Block && !pageId && Number(blockId) === params.id) || + (params.page_content.componentName === COMPONENT_NAME.Block && !pageId && blockId === String(params.id)) || @@ - (params.page_content.componentName === COMPONENT_NAME.Page && !blockId && Number(pageId) === params.id) + (params.page_content.componentName === COMPONENT_NAME.Page && !blockId && pageId === String(params.id))
165-166: Use constants for service tokens to avoid stringly-typed mistakes.- const { generatePageCode } = getMetaApi('engine.service.generateCode') + const { generatePageCode } = getMetaApi(META_SERVICE.GenerateCode) @@ - const { getAllNestedBlocksSchema, generatePageCode } = getMetaApi('engine.service.generateCode') + const { getAllNestedBlocksSchema, generatePageCode } = getMetaApi(META_SERVICE.GenerateCode) @@ - const { getAllNestedBlocksSchema, generateAppCode } = getMetaApi('engine.service.generateCode') + const { getAllNestedBlocksSchema, generateAppCode } = getMetaApi(META_SERVICE.GenerateCode)Also applies to: 378-381, 416-417
534-561: Harden route generation: sanitize router field and handle empty pages.- const pageSchema = (schema.pageSchema || []).sort((a, b) => a.meta?.router?.length - b.meta?.router?.length) + const pageSchema = (schema.pageSchema || []).sort( + (a, b) => (String(a.meta?.router || '').length - String(b.meta?.router || '').length) + ) @@ - const newNode = { - path: `/${item.meta.router}`, + const safeRouter = String(item.meta?.router || '').replace(/^\/+/, '') + const newNode = { + path: `/${safeRouter}`, component: item.fileName, name: `${item.meta.id}` } @@ - if (!isGetHome) { + if (!isGetHome && result.length) { isGetHome = true home.redirect = { name: result[0]?.name } + } else if (!result.length) { + home.redirect = undefined }
583-589: Alias replacement regex misses double quotes and over-captures.Safer replacement that handles both quotes:
- } else { - fileContent = fileContent.replace(/(from\s*')(@)(\/.*')/g, '$1.$3') - } + } else { + // replace "@/foo" -> "./foo" after from '<quote>' + fileContent = fileContent.replace(/(from\s*['"])@(?=\/)/g, '$1.') + }
589-602: App.vue patch is brittle; ensure component availability and script style.The template injects but the import only adds TinySelect; without script-setup or global registration, the component won’t render. Verify App.vue uses <script setup> or register TinySelect in components; otherwise switch tag to and register. Also ensure these replacements don’t fail if the exact source strings differ.
I can generate a safer AST-based transform or a regex guarded by presence checks.
606-619: Shadowing imported srcFiles with a local const is confusing; rename.- const newFileRes = fileRes.filter((item) => item.filePath.includes('src/')) - const srcFiles = newFileRes.reduce((prev, item) => { + const newFileRes = fileRes.filter((item) => item.filePath.includes('src/')) + const generatedFiles = newFileRes.reduce((prev, item) => { const fileName = item.filePath prev[fileName] = formatCode(item.fileContent, fileName) return prev }, {}) - srcFiles['import-map.json'] = JSON.stringify(importMap) + generatedFiles['import-map.json'] = JSON.stringify(importMap) const newFiles = store.getFiles() const appJsCode = processAppJsCode(newFiles['app.js'], JSON.parse(searchParams.get('styles') || '[]')) - srcFiles['app.js'] = appJsCode - srcFiles['main.js'] = `import app from './app.js' \n ${srcFiles['src/main.js']}` - srcFiles['main.js'] = srcFiles['main.js'].replace("import 'element-plus/dist/index.css'", '') - setFiles(srcFiles, 'src/main.js') + generatedFiles['app.js'] = appJsCode + generatedFiles['main.js'] = `import app from './app.js' \n ${generatedFiles['src/main.js']}` + generatedFiles['main.js'] = generatedFiles['main.js'].replace("import 'element-plus/dist/index.css'", '') + setFiles(generatedFiles, 'src/main.js')
254-299: Verify CSS vs JS links when adding utils packages.Using
item.content.cdnLinkas both script and css can inject a JS URL as stylesheet. Confirm utils provide a CSS link or split fields accordingly.
343-345: setFiles called immediately for basicFiles: ensure the returned promise is actually awaited upstream.You store the promise and rely on it in getBasicData’s Promise.all. That’s fine, but confirm setFiles is idempotent and safe to call again later.
Optionally rename basicFiles to basicFilesPromise for clarity.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
designer-demo/useData.js(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
designer-demo/useData.js (5)
packages/design-core/src/preview/src/preview/usePreviewData.ts (2)
previewState(32-42)usePreviewData(343-593)packages/vue-generator/src/generator/page.js (2)
generatePageCode(464-502)generateCode(516-535)packages/register/src/common.ts (1)
getMergeMeta(53-55)packages/register/src/constants.ts (1)
META_SERVICE(1-23)packages/design-core/src/preview/src/preview/srcFiles.js (1)
srcFiles(28-28)
🪛 Biome (2.1.2)
designer-demo/useData.js
[error] 23-33: interface are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 34-39: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 45-45: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 78-78: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 78-78: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 83-84: return type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 100-100: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 129-129: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 154-159: interface are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 159-163: interface are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 165-167: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 168-168: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 168-169: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 245-246: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 246-247: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 247-249: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 256-256: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 264-264: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 267-267: Const declarations must have an initialized value.
This variable needs to be initialized.
(parse)
[error] 267-267: Expected a semicolon or an implicit semicolon after a statement, but found none
An explicit or implicit semicolon is expected here...
...Which is required to end this statement
(parse)
[error] 268-268: Parenthesized expression didnt contain anything
Expected an expression here
(parse)
[error] 271-272: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 277-277: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 286-286: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 306-307: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 307-307: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 314-314: type annotation are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 338-343: interface are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 343-346: interface are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 349-351: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 357-357: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 358-358: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 364-365: Type annotations are a TypeScript only feature. Convert your file to a TypeScript file or remove the syntax.
TypeScript only syntax
(parse)
[error] 82-82: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
[error] 267-267: This variable is used before its declaration.
The variable is declared here:
(lint/correctness/noInvalidUseBeforeDeclaration)
…into feat/set-app-preview
There was a problem hiding this comment.
Actionable comments posted: 3
♻️ Duplicate comments (5)
packages/design-core/src/preview/src/preview/usePreviewData.ts (5)
337-344: Signature alignment restored (setImportMap) — good catch.The breaking change noted earlier is resolved: IUsePreviewData includes setImportMap and the hook destructures it. No further action needed.
566-582: Consider extracting routing helpers into a module.getRoutesAndImportSet/getRouterFile/buildTreeRoutes are sizeable and reusable; move to routeGenerator.ts for testability.
422-497: Validate API responses inside getPreGenerateInfo.Assume nothing from the network: verify shapes; degrade gracefully; log once.
- const getPreGenerateInfo = async () => { + const getPreGenerateInfo = async () => { const promises = [ getMetaApi(META_SERVICE.Http).get(`/app-center/v1/api/apps/schema/${appId}`), fetchPageList(appId) ] - - const [appData, pageList] = await Promise.all(promises) - const pageDetailList = pageList + try { + const [appData, pageList] = await Promise.all(promises) + if (!appData || typeof appData !== 'object') { + throw new Error('Invalid app schema received') + } + const pageDetailList = Array.isArray(pageList) ? pageList : [] @@ - const res = await generateAppCode(appSchema) - - const { genResult = [] } = res || {} + const res = await generateAppCode(appSchema) + if (!res || !Array.isArray(res.genResult)) { + throw new Error('Invalid generator output') + } + const { genResult = [] } = res @@ - return fileRes + return fileRes + } catch (e) { + console.error('[preview:app] getPreGenerateInfo failed:', e) + return [] + } }
584-605: Make import path replacement and App.vue injection more robust.The current regex only matches single-quoted static imports and may miss dynamic imports; App.vue string replaces are brittle.
- if (fileName === 'src/router/index.js') { + if (fileName === 'src/router/index.js') { fileContent = getRouterFile(appSchema) } else { - fileContent = fileContent.replace(/(from\s*')(@)(\/.*')/g, '$1.$3') + // Handle single/double quotes and dynamic imports + fileContent = fileContent + .replace(/(from\s*['"])(@)(\/[^'"]*['"])/g, '$1.$3') + .replace(/(import\s*\(\s*['"])(@)(\/[^'"]*['"]\s*\))/g, '$1.$3') } if (fileName === 'src/App.vue') { - fileContent = fileContent.replace( + // Avoid double-injection + if (!/tiny-select/.test(fileContent)) { + fileContent = fileContent.replace( '<router-view></router-view>', `<tiny-select v-model="currentRoute" placeholder="请选择路由" render-type="tree" :tree-op="{ data: $router.options.routes.filter(item => item.path !== '/') }" text-field="path" value-field="path" @change="routeChange"></tiny-select>\n<router-view></router-view>` - ) - fileContent = fileContent.replace( - `import { provide } from 'vue'`, - `import { Select as TinySelect } from '@opentiny/vue'\nimport { ref, provide, watchEffect } from 'vue'\nimport { useRoute, useRouter } from 'vue-router'` - ) - fileContent = fileContent.replace( - `provide(I18nInjectionKey, i18n)`, - `const route = useRoute()\nconst router = useRouter()\nconst currentRoute = ref()\n\nwatchEffect(() => {\n\tcurrentRoute.value = route.path\n})\n\nconst routeChange = () => {\n\trouter.push(currentRoute.value)\n}\nprovide(I18nInjectionKey, i18n)` - ) + ) + } + fileContent = fileContent + .replace( + /import\s*\{\s*provide\s*\}\s*from\s*['"]vue['"]/, + `import { Select as TinySelect } from '@opentiny/vue'\nimport { ref, provide, watchEffect } from 'vue'\nimport { useRoute, useRouter } from 'vue-router'` + ) + .replace( + /provide\(I18nInjectionKey,\s*i18n\)/, + `const route = useRoute()\nconst router = useRouter()\nconst currentRoute = ref()\n\nwatchEffect(() => { currentRoute.value = route.path })\n\nconst routeChange = () => { router.push(currentRoute.value) }\nprovide(I18nInjectionKey, i18n)` + ) }
415-626: Harden app preview: input validation, error handling, and awaiting setFiles.The app branch performs multiple async operations with unchecked inputs and no catch. JSON.parse on URL params can throw; appId may be null; failures bubble and leave the UI in an indeterminate state. Also, setFiles isn’t awaited.
- } else if (previewType === 'app') { - const appId = searchParams.get('id') + } else if (previewType === 'app') { + try { + const appId = searchParams.get('id') + if (!appId) { + throw new Error('Missing "id" query param for app preview') + } const { getAllNestedBlocksSchema, generateAppCode } = getMetaApi('engine.service.generateCode') - const importMap = await getImportMap(JSON.parse(searchParams.get('scripts') || '{}')) + const safeParse = (str, fb) => { try { return JSON.parse(str ?? '') } catch { return fb } } + const importMap = await getImportMap(safeParse(searchParams.get('scripts'), {})) @@ - const fileRes = await getPreGenerateInfo() + const fileRes = await getPreGenerateInfo() const newFileRes = fileRes.filter((item) => item.filePath.includes('src/')) - const srcFiles = newFileRes.reduce((prev, item) => { + const emittedSrcFiles = newFileRes.reduce((prev, item) => { const fileName = item.filePath prev[fileName] = formatCode(item.fileContent, fileName) return prev }, {}) - srcFiles['import-map.json'] = JSON.stringify(importMap) + emittedSrcFiles['import-map.json'] = JSON.stringify(importMap) const newFiles = store.getFiles() const enableTailwindCSS = getMergeMeta('engine.config')?.enableTailwindCSS - const appJsCode = processAppJsCode( - newFiles['app.js'], - JSON.parse(searchParams.get('styles') || '[]'), - enableTailwindCSS - ) - srcFiles['app.js'] = appJsCode - srcFiles['main.js'] = `import app from './app.js' \n ${srcFiles['src/main.js']}` - srcFiles['main.js'] = srcFiles['main.js'].replace("import 'element-plus/dist/index.css'", '') - setFiles(srcFiles, 'src/main.js') + const styles = safeParse(searchParams.get('styles'), []) + const appJsCode = processAppJsCode(newFiles['app.js'], styles, enableTailwindCSS) + emittedSrcFiles['app.js'] = appJsCode + emittedSrcFiles['main.js'] = `import app from './app.js'\n${emittedSrcFiles['src/main.js'] || ''}` + emittedSrcFiles['main.js'] = emittedSrcFiles['main.js'].replace(/import\s+['"]element-plus\/dist\/index\.css['"];?/g, '') + await setFiles(emittedSrcFiles, 'src/main.js') + } catch (error) { + console.error('Failed to generate app preview:', error) + await setFiles({ 'src/main.js': "console.error('App preview failed');" }, 'src/main.js') + } }Also await the page branch’s setFiles for consistency:
- setFiles(newFiles, 'App.vue') + await setFiles(newFiles, 'App.vue')
🧹 Nitpick comments (5)
packages/design-core/src/preview/src/preview/usePreviewData.ts (4)
535-564: Guard home redirect when no routes; stable sort.If pageSchema is empty, home.redirect.name becomes undefined.
- const pageSchema = (schema.pageSchema || []).sort((a, b) => a.meta?.router?.length - b.meta?.router?.length) + const pageSchema = (schema.pageSchema || []).slice().sort((a, b) => { + const al = (a.meta?.router || '').length + const bl = (b.meta?.router || '').length + return al - bl + }) @@ - if (!isGetHome) { - isGetHome = true - home.redirect = { name: result[0]?.name } - } + if (!isGetHome && result.length) { + isGetHome = true + home.redirect = { name: result[0].name } + }
607-626: Avoid shadowing imported srcFiles; remove Element Plus CSS robustly; await setFiles.Shadowing srcFiles impairs readability and risks confusion with the imported singleton. Also make CSS removal regex-agnostic.
- const newFileRes = fileRes.filter((item) => item.filePath.includes('src/')) - const srcFiles = newFileRes.reduce((prev, item) => { + const newFileRes = fileRes.filter((item) => item.filePath.includes('src/')) + const emittedSrcFiles = newFileRes.reduce((prev, item) => { const fileName = item.filePath prev[fileName] = formatCode(item.fileContent, fileName) return prev }, {}) - srcFiles['import-map.json'] = JSON.stringify(importMap) + emittedSrcFiles['import-map.json'] = JSON.stringify(importMap) @@ - srcFiles['app.js'] = appJsCode - srcFiles['main.js'] = `import app from './app.js' \n ${srcFiles['src/main.js']}` - srcFiles['main.js'] = srcFiles['main.js'].replace("import 'element-plus/dist/index.css'", '') - setFiles(srcFiles, 'src/main.js') + emittedSrcFiles['app.js'] = appJsCode + emittedSrcFiles['main.js'] = `import app from './app.js'\n${emittedSrcFiles['src/main.js'] || ''}` + emittedSrcFiles['main.js'] = emittedSrcFiles['main.js'].replace(/import\s+['"]element-plus\/dist\/index\.css['"];?/g, '') + await setFiles(emittedSrcFiles, 'src/main.js')
414-415: Await setFiles in page branch.Prevent race conditions in subsequent updates.
- setFiles(newFiles, 'App.vue') + await setFiles(newFiles, 'App.vue')
416-419: Guard JSON.parse of URLscriptsparam.JSON.parse(searchParams.get('scripts') || '{}') can throw for malformed query values — wrap in a try/catch or use a safeParse helper and default to {} before calling getImportMap.
Location: packages/design-core/src/preview/src/preview/usePreviewData.ts (≈ lines 416–419).packages/toolbars/preview/vite.config.ts (1)
33-35: Consider a plugin instead of manual injection (optional)If you prefer a maintained solution that handles multi‑entry and SSR builds, use vite-plugin-lib-inject-css and drop the banner hack. (github.com)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/design-core/src/preview/src/preview/usePreviewData.ts(2 hunks)packages/toolbars/preview/vite.config.ts(1 hunks)packages/vue-generator/src/templates/vue-template/templateFiles/src/main.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/vue-generator/src/templates/vue-template/templateFiles/src/main.js
🧰 Additional context used
🧠 Learnings (3)
📚 Learning: 2025-01-14T06:59:23.602Z
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/page-block-function/methods.ts:9-21
Timestamp: 2025-01-14T06:59:23.602Z
Learning: The code in packages/canvas/render/src/page-block-function/methods.ts is migrated code that should not be modified during the migration phase. Error handling improvements can be addressed in future PRs.
Applied to files:
packages/design-core/src/preview/src/preview/usePreviewData.ts
📚 Learning: 2024-10-30T02:38:05.165Z
Learnt from: yy-wow
PR: opentiny/tiny-engine#886
File: packages/toolbars/generate-code/src/http.js:39-40
Timestamp: 2024-10-30T02:38:05.165Z
Learning: In the function `fetchBlockSchema` in `packages/toolbars/generate-code/src/http.js`, input validation and error handling for the `blockName` parameter are handled by the user elsewhere, so it's acceptable not to perform parameter validation or URL encoding in this function within this codebase.
Applied to files:
packages/design-core/src/preview/src/preview/usePreviewData.ts
📚 Learning: 2025-07-03T09:22:59.512Z
Learnt from: hexqi
PR: opentiny/tiny-engine#1501
File: mockServer/src/tool/Common.js:79-82
Timestamp: 2025-07-03T09:22:59.512Z
Learning: In the tiny-engine project, the mockServer code uses ES6 import syntax but is compiled to CommonJS output. This means CommonJS globals like `__dirname` are available at runtime, while ES6 module-specific features like `import.meta` would cause runtime errors.
Applied to files:
packages/design-core/src/preview/src/preview/usePreviewData.ts
🧬 Code graph analysis (1)
packages/design-core/src/preview/src/preview/usePreviewData.ts (6)
packages/register/src/common.ts (2)
getMetaApi(20-30)getMergeMeta(53-55)packages/design-core/src/preview/src/preview/http.js (4)
fetchBlockSchema(40-41)fetchBlockSchema(40-41)fetchPageList(48-48)fetchPageList(48-48)packages/design-core/src/preview/src/preview/generate.js (3)
processAppJsCode(153-169)processAppJsCode(153-169)res(154-154)packages/design-core/src/preview/src/preview/importMap.js (3)
importMap(16-16)getImportMap(39-46)getImportMap(39-46)packages/vue-generator/src/plugins/genRouterPlugin.js (2)
home(57-59)isGetHome(60-60)packages/design-core/src/preview/src/preview/srcFiles.js (1)
srcFiles(28-28)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: push-check
🔇 Additional comments (2)
packages/design-core/src/preview/src/preview/usePreviewData.ts (1)
5-13: HTTP imports look consistent with the new flow.New fetchPageList import aligns with the app preview. No issues spotted here.
packages/toolbars/preview/vite.config.ts (1)
33-35: Scope CSS import to the entry chunk and make the CSS filename deterministic (packages/toolbars/preview/vite.config.ts)
- output.banner as a plain string is injected into every chunk; gate it to the entry and add a semicolon/newline. Stabilize the emitted CSS filename via build.lib.cssFileName (or update the import to the actual emitted filename).
- output: { - banner: 'import "./style.css"' - }, + output: { + banner: (chunk) => (chunk.isEntry ? 'import "./style.css";\n' : '') + },build: { sourcemap: true, lib: { entry: path.resolve(__dirname, './index.ts'), name: 'toolbar-preview', + cssFileName: 'style', fileName: (_format, entryName) => `${entryName}.js`, formats: ['es'] },I could not verify the repo's Vite version or whether build.lib.cssFileName is supported from the sandbox — confirm locally that the workspace uses a Vite version that supports cssFileName (or adjust the import to the actual emitted CSS filename).
packages/vue-generator/src/templates/vue-template/templateFiles/src/http/axios.js
Show resolved
Hide resolved
packages/vue-generator/src/templates/vue-template/templateFiles/src/lowcodeConfig/dataSource.js
Show resolved
Hide resolved
|
先行合入，遗留的下面意见继续修改： |
3 participants
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
用户搭建完以后并不清楚整个项目是否能正常运行，尤其牵扯到跨页面的时候只能出码。
What is the current behavior?
Issue Number: N/A
What is the new behavior?
添加了应用预览，使用户可以测试例如页面跳转，传递参数等完整的业务流程
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit