feat: add runtime rendering solution #1661
Conversation
Majority is the same as builtin components in package/canvas/renderer. But CanvasRouterLink.vue, CanvasRouterView.vue, and CanvasCollection.vue had changed.
Referenced the implementation of tiny-schema-renderer repo and canvas/render
There are some bugs in lifecycle execution(onMount, onBeforeUpdate, onUpdated, onErrorCaptured, onUnmounted)
Support homepage setting, nested routes, and default navigation to subpages for nested routes
Mainly based on the vue-generator code. Currently in a mixed state of JavaScript and TypeScript, which will be improved later.
Support using destructured npm package imports
In useAppSchema.ts, replace the actual requests in fetchAppSchema(appId) and fetchBlocks() with mock files to use mock files for testing.
WalkthroughAdds a new runtime-renderer package (Vue runtime app, router, stores, parser, renderer, builtins, CSS scoping), a runtime-deploy toolbar and common deploy utilities, a designer-demo runtime bootstrap and dependency, dev/build alias and TS path entries, and integrates the toolbar into design-core, layout, and registry. Changes
Sequence Diagram(s)sequenceDiagram
participant Browser
participant runtime.html
participant runtime.js
participant Renderer as initRuntimeRenderer()
participant Schema as useAppSchema
participant Router as createAppRouter()
participant Stores as createStores
participant VueApp as Vue App
Browser->>runtime.html: GET /runtime.html
runtime.html->>runtime.js: load module /src/runtime.js
runtime.js->>Renderer: initRuntimeRenderer()
Renderer->>Schema: fetchAppSchema(), fetchBlocks()
Schema->>Schema: load bundles, components, utils, data sources, CSS
Renderer->>Router: createAppRouter()
Renderer->>Stores: generateStoresConfig() → createStores(pinia)
Renderer->>VueApp: create app, use pinia/router/i18n, mount #app
sequenceDiagram
participant User
participant Toolbar as RuntimeDeploy (Main.vue)
participant Layout as useLayout
participant Deploy as runtimeDeploy()
participant Window as window.open
User->>Toolbar: click deploy
Toolbar->>Layout: isEmptyPage()
alt page empty
Layout-->>Toolbar: true
Toolbar->>User: show warning and abort
else page not empty
Layout-->>Toolbar: false
Toolbar->>Deploy: runtimeDeploy()
Deploy->>Deploy: compute URL (tenant,id,pageId,hash,deployUrl)
Deploy->>Window: window.open(url, "tiny-engine-runtime")
Window-->>User: runtime opens or popup blocked (notify)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes
Poem
Pre-merge checks and finishing touches❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
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: 48
♻️ Duplicate comments (2)
packages/runtime-renderer/src/renderer/page-function/css-handler.ts (1)
90-104: See earlier comment on global state race condition.This function mutates the global
enableScopedflag, which is the source of the race condition flagged in lines 19-20. Refer to that comment for the recommended fix.packages/runtime-renderer/src/types/schema.ts (1)
238-245: Same API response pattern as AppSchema.This interface has the same design issues as
AppSchema(lines 2-22): mixing response metadata with data, and redundant error fields. Consider applying the same refactoring approach to separateApiResponse<T>fromBlockData.
🧹 Nitpick comments (62)
packages/runtime-renderer/src/mock/blocks.json (2)
329-332: Remove the empty dependency object.The empty
componentsobject serves no purpose and clutters the configuration.Apply this diff:
"dependencies": { "scripts": [ - { - "components": {} - }, { "package": "@opentiny/vue", "components": { "TinyButton": "Button" } } ],
322-325: Replace magic number with a descriptive log message.The
console.log(123456)uses a magic number that provides no context. For test/debug mock data, a descriptive message would be more helpful.Consider:
"onClickNew1": { "type": "JSFunction", - "value": "function onClickNew1(event) {\n this.state.stateblock1.button++\n console.log(123456)\n}\n" + "value": "function onClickNew1(event) {\n this.state.stateblock1.button++\n console.log('Button counter incremented:', this.state.stateblock1.button)\n}\n" }packages/runtime-renderer/src/App.vue (1)
2-2: Consider whether forced remounting is necessary.Using
:key="$route.fullPath"forces a full component remount on every route change, which destroys component state and prevents Vue Router's built-in transition/keep-alive optimizations. This pattern is typically only needed when:
- Child components don't properly react to route param changes
- You need to reset component state between routes with identical components
For most runtime rendering scenarios, the default router-view behavior (remounting only when the component changes) is more performant and preserves user interactions better.
If forced remounting is required for your schema-driven rendering, consider documenting why in a comment. Otherwise, try removing the key binding:
- <router-view :key="$route.fullPath" /> + <router-view />Verify that route navigation and component updates still work correctly after this change.
packages/runtime-renderer/src/renderer/builtin/CanvasText.vue (1)
1-18: LGTM! Consider optional modernization.The component implementation is correct with proper prop typing and defaults. The class binding correctly combines static and dynamic classes.
For consistency with other new components in this PR (like PageRenderer.vue which uses script setup), you could optionally modernize to Composition API:
<template> <span :class="['canvas-text', className]">{{ text }}</span> </template> <script setup lang="ts"> defineProps<{ text?: string className?: string }>() withDefaults(defineProps<{ text?: string; className?: string }>(), { text: '', className: '' }) </script>However, the current Options API implementation is perfectly valid and functional.
packages/runtime-renderer/src/types/config.ts (1)
25-30: Consider usingunknowninstead ofanyfor better type safety.The use of
Record<string, any>eliminates compile-time type checking. While this is pragmatic for dynamic runtime stores from schemas, consider usingRecord<string, unknown>to maintain some type safety and require explicit type assertions at usage sites.Apply this diff if you want stricter typing:
export interface StoreConfig { id: string - state: Record<string, any> - actions: Record<string, any> - getters: Record<string, any> + state: Record<string, unknown> + actions: Record<string, unknown> + getters: Record<string, unknown> }packages/runtime-renderer/src/renderer/builtin/CanvasRouterLink.vue (2)
1-13: Replacejavascript:void(0)with#for cleaner markup.The
href="javascript:void(0)"is a dated practice. Since you're callingpreventDefault()in the click handler, usinghref="#"is cleaner and more conventional.Apply this diff:
<a - href="javascript:void(0)" + href="#" @click="handleClick"
44-49: Consider adding error handling for failed navigation.The
handleClickfunction correctly prevents default and usesrouter.push, but navigation can fail (e.g., navigation guards rejecting). Consider adding error handling for better resilience.const handleClick = (event: Event) => { event.preventDefault() - if (props?.to) { - router.push(props.to) + if (props.to) { + router.push(props.to).catch((err) => { + console.warn('Navigation failed:', err) + }) } }packages/runtime-renderer/src/stores/index.ts (1)
28-45: Add validation and error handling for store creation.The
createStoresfunction should validate configurations and handle potential failures:
- Line 40: Chinese comment should be translated.
- Missing validation for required config fields (id, state, etc.).
- No error handling if
defineStoreor store instantiation fails.- Consider logging when stores are created for debugging.
Apply this diff:
export const createStores = (storesConfig: StoreConfig[], pinia: Pinia) => { const stores = shallowReactive<Record<string, any>>({}) storesConfig.forEach((config) => { + // Validate config + if (!config.id) { + console.error('Store config missing required id field:', config) + return + } + + try { - // 使用 defineStore 创建 Pinia store + // Create Pinia store using defineStore const useStore = defineStore(config.id, { state: () => config.state, - getters: config.getters, - actions: config.actions }) - // 使用useStore创建 store 实例并绑定到 pinia + // Create store instance and bind to Pinia stores[config.id] = useStore(pinia) + } catch (error) { + console.error(`Failed to create store: ${config.id}`, error) + } }) return stores }packages/runtime-renderer/src/utils/data-utils.ts (4)
1-2: Consider direct named export for clarity.The destructuring-with-rename pattern
export const { parseFunction: generateFunction } = commonUtilsis less discoverable than a direct re-export. IfgenerateFunctionis intended as a domain-specific alias, consider documenting why the rename is necessary or using a more explicit pattern.Apply this diff for a clearer approach:
-import { utils as commonUtils } from '@opentiny/tiny-engine-utils' -export const { parseFunction: generateFunction } = commonUtils +import { utils as commonUtils } from '@opentiny/tiny-engine-utils' +export const generateFunction = commonUtils.parseFunction
4-6: Add type annotations for better type safety.The
resetfunction lacks type annotations, reducing type safety and IDE support.Apply this diff:
-export const reset = (obj) => { +export const reset = (obj: Record<string, any>) => { Object.keys(obj).forEach((key) => delete obj[key]) }
8-12: Add type annotations to newFn wrapper.While the wrapper prevents ESLint errors, it lacks type safety for its arguments.
Apply this diff:
-export const newFn = (...argv) => { +export const newFn = (...argv: string[]) => { const Fn = Function return new Fn(...argv) }
25-30: Add type annotations for parameter objects.The function lacks type information for its parameters, reducing type safety.
Apply this diff:
-export const getDeletedKeys = (objA, objB) => { +export const getDeletedKeys = (objA: Record<string, any>, objB: Record<string, any>): string[] => { const keyA = Object.keys(objA) const keyB = new Set(Object.keys(objB)) return keyA.filter((item) => !keyB.has(item)) }packages/runtime-renderer/src/app-function/http/axios.js (2)
118-136: Mock state management lacks cleanup guarantees.The
disableMock()method at line 131-136 callsmock.restore()but doesn't handle potential errors during restoration. Additionally, there's no way to query or clear accumulated proxy configurations.Apply this diff:
disableMock() { if (mock) { - mock.restore() + try { + mock.restore() + } catch (error) { + console.error('[axios-wrapper] Failed to restore mock adapter:', error) + } } mock = undefined },Consider adding a method to clear/reset mock configurations:
resetMock() { if (mock) { mock.reset() } },
16-143: Convert to TypeScript for type safety.This file is JavaScript (.js) but the project uses TypeScript. Converting to TypeScript would provide:
- Type safety for the config parameter and return type
- Better IDE support
- Documentation through types
- Compile-time error catching
Consider renaming to
axios.tsand adding type definitions:import axios, { AxiosInstance, AxiosRequestConfig } from 'axios' import MockAdapter from 'axios-mock-adapter' interface HttpClient { request: AxiosInstance['request'] get: AxiosInstance['get'] // ... other methods mock(config: MockConfig[]): MockAdapter disableMock(): void isMock(): boolean } export default (config: AxiosRequestConfig): HttpClient => { // implementation }packages/runtime-renderer/src/renderer/parser/parser.ts (2)
233-236: Use strict equality for condition checks.Line 234 uses loose equality (
== null) which matches bothnullandundefined. While this might be intentional, it's better to be explicit for maintainability.Apply this diff for clarity:
export const parseCondition = (condition, scope, ctx) => { - // eslint-disable-next-line no-eq-null - return condition == null ? true : parseData(condition, scope, ctx) + return condition === null || condition === undefined ? true : parseData(condition, scope, ctx) }
282-299: v-model binding generation has fragile event name detection.The regex
/^on[A-Z]/at line 286 checks for event handlers, but it doesn't validate that the event name actually corresponds to the model value property. This could miss edge cases or incorrectly identify handlers.Improve the detection logic:
// 处理 v-model 双向绑定 const propsEntries = Object.entries(data) const modelValue = propsEntries.find(([_key, value]) => value?.type === 'JSExpression' && value?.model === true) - const hasUpdateModelValue = propsEntries.find( - ([key]) => /^on[A-Z]/.test(key) && key.startsWith(`onUpdate:${modelValue?.[0]}`) - ) + const hasUpdateModelValue = modelValue && propsEntries.find( + ([key]) => key === `onUpdate:${modelValue[0]}` + ) if (modelValue && !hasUpdateModelValue) { // 添加 onUpdate:modelKey 事件 - res[`onUpdate:${modelValue?.[0]}`] = parseData( + res[`onUpdate:${modelValue[0]}`] = parseData( { type: 'JSFunction', value: `(value) => ${modelValue[1].value}=value` }, scope, ctx ) }packages/runtime-renderer/src/renderer/builtin/CanvasPlaceholder.vue (1)
14-16: Consider internationalization for the placeholder text.The hardcoded Chinese default text '请将元素拖放到这里' may limit usability for non-Chinese users. Consider using the i18n mechanism already present in the codebase (based on learnings mentioning tiny-engine-i18n-host).
For example, you could use a translated key:
placeholder: { type: String, - default: '请将元素拖放到这里' + default: () => i18n.global.t('canvas.placeholder.dragHere') }Or make the component i18n-aware by importing and using the translation service.
packages/runtime-renderer/src/app-function/http/config.js (1)
13-15: Consider making HTTP configuration runtime-customizable.The hardcoded
withCredentials: falseis a safe default, but applications may need to customize this (e.g., for CORS requests with cookies). Consider exposing this as a configurable option that can be overridden at runtime initialization.Example refactor:
-export default { - withCredentials: false +export const createHttpConfig = (overrides = {}) => ({ + withCredentials: false, + ...overrides +}) + +export default createHttpConfig()packages/runtime-renderer/src/renderer/builtin/CanvasCollection.vue (3)
2-2: AddinheritAttrs: falsefor explicit attribute control.When using
v-bind="$attrs"on a non-root or specific element, it's best practice to setinheritAttrs: falsein the component options to prevent Vue from automatically applying attributes to the root element. This makes the attribute forwarding behavior explicit and predictable.Apply this diff:
export default { + inheritAttrs: false, components: { CanvasPlaceholder },
9-27: Consider adding TypeScript type annotations.The script uses
lang="ts"but lacks type annotations for props and component options. Consider adding proper TypeScript interfaces or usingdefineComponentfrom Vue for better type safety.Example refactor:
<script lang="ts"> import { defineComponent, PropType } from 'vue' import CanvasPlaceholder from './CanvasPlaceholder.vue' export default defineComponent({ name: 'CanvasCollection', components: { CanvasPlaceholder }, props: { tag: { type: String as PropType<string>, default: 'div' }, schema: { type: Object as PropType<Record<string, any>>, default: () => ({}) }, dataSource: { type: [String, Array, Number] as PropType<string | any[] | number> } } }) </script>
20-24: Remove or document unused props in CanvasCollection.vue
Bothschema(line 20) anddataSource(line 24) are declared but never referenced in the component’s template or script, nor bound by any parent. Drop them or add documentation explaining their intended use.packages/runtime-renderer/src/renderer/builtin/CanvasImg.vue (1)
12-15: Consider using Object type for style prop.The
styleprop is defined as String, which limits flexibility and type safety. Vue's style binding typically accepts Object or Array types for better validation and developer experience.Apply this diff to improve type safety:
style: { - type: String, - default: '' + type: [Object, String], + default: () => ({}) }packages/runtime-renderer/src/app-function/http/index.js (1)
22-22: Incomplete re-login flow.The TODO comment indicates a missing authentication re-login implementation.
Do you want me to generate a re-login handler implementation or open a new issue to track this task?
packages/runtime-renderer/src/renderer/builtin/CanvasSlot.vue (2)
13-20: Propsnameandparamsare declared but never used.The component defines
nameandparamsprops but does not reference them in the template or pass them explicitly to child components. If these props are intended for future use or external APIs, consider documenting their purpose.
3-3: Consider disabling attribute inheritance to prevent prop duplication.The component uses
v-bind="$attrs"to forward attributes to CanvasPlaceholder. However, by default Vue inherits all attributes (includingnameandparamsprops) to the root element (the<slot>), and then forwards them again via$attrs. This may cause duplicate attributes on CanvasPlaceholder.Apply this diff to explicitly control attribute inheritance:
<script> import CanvasPlaceholder from './CanvasPlaceholder.vue' export default { + inheritAttrs: false, components: { CanvasPlaceholder },Also applies to: 7-21
packages/runtime-renderer/src/renderer/builtin/CanvasBox.vue (1)
7-16: Consider migrating to<script setup>for consistency.The component uses Options API. For a new codebase feature, consider using
<script setup>with Composition API for better TypeScript support, reduced boilerplate, and alignment with modern Vue 3 patterns.Example refactor:
<script setup lang="ts"> defineOptions({ inheritAttrs: false }) interface Props { tag?: string } withDefaults(defineProps<Props>(), { tag: 'div' }) </script>packages/runtime-renderer/package.json (3)
23-23: Consider upgradingaxios-mock-adapterto latest stable version.The package specifies
axios-mock-adapter: ^1.19.0, but version 2.1.0 is available and includes maintenance fixes and improvements. Review the changelog for any breaking changes before upgrading.Based on learnings.
- "axios-mock-adapter": "^1.19.0", + "axios-mock-adapter": "^2.1.0",
29-31: Vite and plugin versions are older than other packages in the monorepo.This package uses
vite: ^4.0.0and@vitejs/plugin-vue: ^4.0.0, while the runtime-deploy toolbar package uses Vite 5.x and plugin-vue 5.x. Consider aligning versions across the monorepo for consistency.
19-19: Upgrade@vue/babel-plugin-jsxto v1.5.0.
In packages/runtime-renderer/package.json (line 19), change"@vue/babel-plugin-jsx": "^1.2.5"→"^1.5.0". Release notes list only feature and bugfix updates since v1.2.5 with no breaking changes in v1.3.x or v1.4.x, so this bump is safe and ensures you get the latest fixes and improvements.packages/runtime-renderer/index.ts (2)
26-27: Consider parallelizing independent async operations.
fetchAppSchemaandfetchBlocksappear to be independent operations that could run in parallel to reduce initialization time.- await fetchAppSchema(appId) - await fetchBlocks() + await Promise.all([ + fetchAppSchema(appId), + fetchBlocks() + ])
22-39: Consider adding user-facing error handling.While
useAppSchemamay handle fetch errors internally,initRuntimeRendererhas no try-catch or fallback UI. If initialization fails, users see a blank screen. Consider wrapping the initialization in error handling and displaying an error message.Example:
export const initRuntimeRenderer = async () => { try { // ... existing initialization code } catch (error) { console.error('Failed to initialize runtime renderer:', error) document.getElementById('app')!.innerHTML = ` <div style="padding: 2rem; color: red;"> <h2>Failed to load application</h2> <p>${error instanceof Error ? error.message : 'Unknown error'}</p> </div> ` throw error } }packages/runtime-renderer/src/renderer/page-function/state.ts (3)
15-17: Consider performance implications of clearing all keys.When
clearis true, this iterates and deletes all keys. For large state objects, this could be slow. Consider whetherObject.keys(state).forEach((key) => delete state[key])is necessary when you could reassign the reactive reference or use a more efficient clearing mechanism.However, given Vue 3's reactive system, direct deletion is correct to maintain reactivity. The current approach is acceptable for typical use cases.
19-23: getDeletedKeys performs only shallow key comparison
The implementation inpackages/runtime-renderer/src/utils/data-utils.ts(and its mirror inpackages/canvas/render/src/data-utils.ts) filters only top-level property names viaObject.keys, so it doesn’t handle nested objects, distinguishundefinedvsnull, or compare array contents. If you need those cases covered, implement deep-diff logic or explicitly document that deletions are shallow.
28-39: Use explicit null/undefined checks for accessor.value (page-function/state.ts:29–34)
isStateAccessorcorrectly filters accessor objects, butif (accessor?.getter?.value)(and similarly forsetter) treats empty strings ('') as falsy and will skip them. Replace these truthiness guards with explicit checks—e.g.if (accessor.getter.value != null)—to ensure empty-string values aren’t inadvertently ignored.packages/toolbars/runtime-deploy/src/Main.vue (2)
4-4: Hardcoded Chinese text should be internationalized.The content text "在线部署运行" and message "请先创建页面" (Line 35) are hardcoded. For better i18n support, use the i18n system that appears to be available in the project (tiny-engine-i18n-host).
Consider extracting strings to i18n keys:
- content="在线部署运行" + :content="t('toolbar.runtimeDeploy')"- message: '请先创建页面' + message: t('toolbar.createPageFirst')
30-42: WrapruntimeDeploy()in a try/catch and await its promiseTo surface deployment errors and prevent unhandled rejections, update the call to:
const deploy = async () => { // 检查页面状态，确保有内容 if (useLayout().isEmptyPage()) { useNotify({ type: 'warning', message: '请先创建页面' }) return } - runtimeDeploy() + try { + await runtimeDeploy() + } catch (error) { + useNotify({ + type: 'error', + message: '部署失败，请重试' + }) + console.error('Runtime deploy failed:', error) + } }packages/toolbars/runtime-deploy/vite.config.ts (1)
20-36: Vite config looks correct for a library build.The configuration appropriately:
- Enables sourcemaps for debugging
- Externalizes Vue and @opentiny dependencies to avoid bundling peer dependencies
- Uses ES format for modern module consumption
- Disables publicDir for library builds
One minor observation: Line 23 has an empty
resolve: {}object. If there are no resolve options needed, consider removing it for cleanliness.publicDir: false, - resolve: {}, build: {packages/runtime-renderer/src/components/Loading.vue (1)
46-59: Animation keyframes are correct but could use will-change hint.The rotation animation is smooth, but for better performance, especially on mobile devices, add
will-change: transformto the.loadingclass to hint to the browser that this element will be animated..loading { width: 60px; height: 60px; margin: 0 auto; position: relative; animation: load 3s linear infinite; + will-change: transform; }packages/runtime-renderer/src/renderer/page-function/blockContext.ts (1)
47-47: JSON.parse/stringify is inefficient for deep cloning.Using
JSON.parse(JSON.stringify(data))has limitations:
- Doesn't handle functions, undefined, Symbols, or circular references
- Performance overhead for large objects
Consider using a dedicated cloning library like
structuredClone(native in modern browsers/Node) or lodash'scloneDeep.- const newSchema = JSON.parse(JSON.stringify(data)) + const newSchema = structuredClone(data)Note:
structuredCloneis available in Node 17+ and modern browsers. If supporting older environments, keep JSON approach or add a polyfill.packages/runtime-renderer/src/renderer/useContext.ts (2)
15-17: Consider adding generic type parameter for better type safety.The
Contextinterface currently allows any values. While this provides flexibility, consider making the factory generic to allow consumers to specify their context shape:-interface Context { +interface Context<T = any> { - [key: string]: any + [key: string]: T }Then update the factory signature:
export default <T = any>(): UseContextReturn<T> => { ... }This would enable type-safe context usage while maintaining backward compatibility via the default
anytype.
30-35: LGTM with optional refactor for clear logic.The
setContextimplementation correctly merges new context values. The clear parameter properly removes existing keys before assignment.For slightly cleaner code, consider this refactor:
const setContext = (ctx: Context, clear?: boolean) => { if (clear) { - Object.keys(context).forEach((key) => delete context[key]) + Object.keys(context).forEach((key) => { + delete context[key] + }) } Object.assign(context, ctx) }Or use a more functional approach:
const setContext = (ctx: Context, clear?: boolean) => { if (clear) { for (const key in context) { delete context[key] } } Object.assign(context, ctx) }packages/runtime-renderer/src/components/NotFound.vue (2)
11-37: Add internationalization and improve accessibility.The template has hardcoded Chinese text and lacks proper accessibility attributes:
- I18n: Text like "页面未找到", "抱歉，您访问的页面不存在或已被删除", etc., should be externalized for internationalization, especially given the PR context mentions tiny-engine-i18n-host as a workspace dependency.
- Accessibility: The SVG icon lacks a title or
aria-label, and interactive elements could benefit from better ARIA attributes.For i18n, consider using the i18n system:
<h1>{{ t('notFound.title') }}</h1> <p>{{ t('notFound.description') }}</p>For accessibility:
<svg viewBox="0 0 200 200" width="120" height="120"> + <title>404 错误图标</title> <circle cx="100" cy="100" r="80" fill="#f0f0f0" stroke="#ddd" stroke-width="2" /> <text x="100" y="110" text-anchor="middle" font-size="48" fill="#999">404</text> </svg>- <button @click="refresh" class="action-button secondary">刷新页面</button> + <button @click="refresh" class="action-button secondary" aria-label="刷新当前页面"> + 刷新页面 + </button>
49-51: Consider router-based navigation instead of full page reload.
window.location.reload()forces a full page refresh, losing all application state and causing a jarring user experience. In a Vue SPA, consider navigating to the home route or previous page instead:+import { useRouter } from 'vue-router' + const route = useRoute() +const router = useRouter() const refresh = () => { - window.location.reload() + // Navigate to home page or previous route + router.push('/') + // Or go back if there's history: + // router.back() }If a full reload is genuinely required (e.g., to reset service workers or clear cache), consider confirming with the user first.
packages/runtime-renderer/src/composables/useAppSchema.ts (2)
11-18: Consider propagating initialization errors.The error is caught and logged but not propagated to the caller. This means
initializeAppConfigand its caller won't know if utility initialization failed, potentially leading to silent failures at runtime when utilities are accessed.Consider one of these approaches:
- Re-throw the error after logging:
const initializeUtils = async (utils: Util[]) => { try { await initUtils(utils) } catch (error) { // eslint-disable-next-line no-console console.error('工具函数初始化失败:', error) + throw error } }
- Or return a success/failure indicator:
-const initializeUtils = async (utils: Util[]) => { +const initializeUtils = async (utils: Util[]): Promise<boolean> => { try { await initUtils(utils) + return true } catch (error) { // eslint-disable-next-line no-console console.error('工具函数初始化失败:', error) + return false } }
29-34: Remove unsafe type assertion.The
as anytype assertion on line 32 bypasses TypeScript's type checking. According to the schema types,I18nConfigvalues areRecord<string, any>, which should be compatible withmergeLocaleMessage.const initializeI18n = (i18nConfig: I18nConfig) => { if (!i18nConfig) return Object.entries(i18nConfig).forEach(([loc, msgs]) => { - i18n.global.mergeLocaleMessage(loc, msgs as any) + i18n.global.mergeLocaleMessage(loc, msgs) }) }If there's a legitimate type mismatch, consider adding proper type guards or updating the interface definition.
packages/runtime-renderer/src/renderer/page-function/accessor.ts (1)
20-21: Unused propsAccessorMap.The
propsAccessorMapis declared and exposed in the return object but never populated or used within this module. It appears to be reserved for future use with prop accessors (similar to howgenerateStateAccessorspopulatesstateAccessorMap).Consider either:
- Implementing a
generatePropsAccessorsfunction if prop accessors are needed- Removing
propsAccessorMapif it's not currently used elsewhere in the codebase- Adding a TODO comment explaining its future purpose
Run this script to check if
propsAccessorMapis used elsewhere:#!/bin/bash # Search for propsAccessorMap usage in the codebase rg -n "propsAccessorMap" --type ts --type tsx -C 2packages/runtime-renderer/src/router/index.ts (1)
82-89: Remove or guard debug code for production.Lines 82-89 assign the router to
window.__DEBUG_ROUTER__and log all routes to the console. This debug code should either be removed or guarded behind a development-only flag.export async function createAppRouter() { const routes = await createRouterConfig() const router = createRouter({ history: createWebHashHistory('/runtime.html'), routes }) - if (typeof window !== 'undefined') { + if (typeof window !== 'undefined' && import.meta.env.DEV) { window.__DEBUG_ROUTER__ = router // eslint-disable-next-line no-console console.log( '所有路由:', router.getRoutes().map((r) => ({ path: r.path, name: r.name, redirect: r.redirect })) ) } return router }Also add a TypeScript declaration for the debug property:
declare global { interface Window { __DEBUG_ROUTER__?: Router } }packages/runtime-renderer/src/renderer/render.ts (1)
218-244: Minor duplication: Mapper and block schema binding.Lines 229-231 and 234-237 both add
bindProps.schema = schemafor different component types. This logic could be consolidated.const getBindProps = (schema, scope, context) => { const { componentName } = schema if (componentName === 'CanvasPlaceholder') { return {} } const bindProps = { ...parseData(schema.props, scope, context) } - if (Mapper[componentName]) { - bindProps.schema = schema - } - - // 如果是区块组件，传递完整的 schema - const blockSchema = getBlock(componentName) - if (blockSchema) { + // Mapper components and block components both need the full schema + if (Mapper[componentName] || getBlock(componentName)) { bindProps.schema = schema } // 绑定组件属性时需要将 className 重命名为 class，防止覆盖组件内置 class bindProps.class = bindProps.className delete bindProps.className return bindProps }packages/runtime-renderer/src/renderer/page-function/css-handler.ts (1)
13-17: Remove unused interface property.The
enableModernCSSoption inCSSHandlerOptionsis declared but never referenced in the implementation.Apply this diff to remove the unused property:
interface CSSHandlerOptions { pageId?: string enableScoped?: boolean - enableModernCSS?: boolean }packages/runtime-renderer/src/types/schema.ts (5)
2-22: Consider refining the AppSchema type.The interface mixes API response metadata (
code,message,error,errMsg,success) with actual schema data. Consider:
- Redundant error fields: Both
erroranderrMsgare typed asanyand serve similar purposes.- Type specificity:
anytypes reduce type safety.Consider separating concerns:
+// API response wrapper +export interface ApiResponse<T> { + data: T + code: string + message: string + error?: unknown + success: boolean +} + +// Application data schema +export interface AppData { + bridge: any[] + componentsMap: ComponentMap[] + componentsTree: PageSchema[] + css: string + dataSource: DataSourceConfig + utils: Util[] + packages: PackageConfig[] + meta: AppMeta + config: AppConfig + constants: string + i18n: I18nConfig + version: string +} + -export interface AppSchema { - data: { - bridge: any[] - componentsMap: ComponentMap[] - componentsTree: PageSchema[] - css: string - dataSource: DataSourceConfig - utils: Util[] - packages: PackageConfig[] - meta: AppMeta - config: AppConfig - constants: string - i18n: I18nConfig - version: string - } - code: string - message: string - error: any - errMsg: any - success: boolean -} +export type AppSchema = ApiResponse<AppData>
25-40: Consider typing the generic fields.Several fields use
anywhich reduces type safety:
npmrc?: any(line 31)styles: any[](line 38)If these have known shapes, consider adding specific types:
export interface ComponentMap { package?: string | null destructuring: boolean exportName?: string | null componentName: string version: string npmrc?: Record<string, unknown> | string // or the actual shape path?: string dependencies?: { scripts: Array<{ package?: string components: Record<string, string> }> styles: string[] // or Array<{ url: string; type?: string }> } }
43-64: Eliminate duplication between PageSchema and PageContent.
PageContentduplicates all fields fromPageSchemaexceptmeta. Use TypeScript utility types to maintain DRY principle.Apply this diff:
-// 页面Schema -export interface PageSchema { +export interface PageContent { children: ComponentNode[] css: string componentName: string fileName: string lifeCycles?: LifeCycles | null - meta: PageMeta methods: Record<string, JSFunction> props: Record<string, any> state: Record<string, any> } -export interface PageContent { - children: ComponentNode[] - css: string - componentName: string - fileName: string - lifeCycles?: LifeCycles | null - methods: Record<string, JSFunction> - props: Record<string, any> - state: Record<string, any> -} +// 页面Schema +export interface PageSchema extends PageContent { + meta: PageMeta +}
77-98: Inconsistent naming convention in PageMeta.The interface mixes camelCase (
isHome,lastUpdatedBy,rootElement) with snake_case (gmt_create,gmt_modified,page_content). While this likely mirrors a backend API, consider normalizing to camelCase for JavaScript consistency or documenting that this is a direct API mapping.If you prefer consistency, consider a transformer:
// API response type (as-is from backend) interface PageMetaAPI { app: number gmt_create: string // ... snake_case fields } // Normalized application type export interface PageMeta { app: number gmtCreate: string lastUpdatedBy: string // ... camelCase fields }Or add a JSDoc comment indicating this mirrors the API shape:
/** * Page metadata - mirrors backend API response structure. * Field names use snake_case to match API contract. */ export interface PageMeta { // ... }
135-139: Document the purpose of themodelfield.The
modelproperty inJSExpressionlacks documentation. Is this for v-model binding detection or two-way data binding?Add a JSDoc comment:
// JS表达式 export interface JSExpression { type: 'JSExpression' value: string /** Indicates if this expression is used in a v-model or two-way binding context */ model?: boolean }packages/runtime-renderer/src/renderer/LifecycleWrapper.ts (1)
50-58: Type the props more specifically.Both
schemaandparentuseObjectorObject as PropType<any>, reducing type safety. Consider importing the actual schema types.+import type { PageContent, PageSchema } from '../types/schema' + export const PageLifecycleWrapper = defineComponent({ name: 'PageLifecycleWrapper', props: { schema: { - type: Object as PropType<any>, + type: Object as PropType<Record<string, any>>, required: true }, parent: { - type: Object, - default: () => ({}) + type: Object as PropType<Partial<PageSchema>>, + default: () => ({} as Partial<PageSchema>) } },packages/runtime-renderer/src/renderer/RenderMain.ts (2)
41-44: Add null-safety for missing pages.Line 43 uses
JSON.parse(JSON.stringify(page))which will returnundefinedifpageisundefined, but this could throw or return unexpected values. Add explicit handling.const currentSchema = computed(() => { - const page = getPageById(props.pageId)?.meta?.page_content // 通过 pageId 获取最新的页面对象 - return JSON.parse(JSON.stringify(page)) + const page = getPageById(props.pageId)?.meta?.page_content + if (!page) return null + return JSON.parse(JSON.stringify(page)) })This makes the null-case explicit and avoids
JSON.parse(JSON.stringify(undefined)).
52-52: Add type safety for injected stores.Line 52 injects
'stores'without a default or type, which could result inundefined. Consider adding a default or type assertion.-const stores = inject('stores') +const stores = inject<Record<string, any>>('stores', {})Or if stores must be provided by parent:
const stores = inject<Record<string, any>>('stores') if (!stores) { console.error('stores must be provided by parent component') }packages/runtime-renderer/src/app-function/dataSource.ts (5)
11-24: Consider adding validation for item structure.The
normalizeItemfunction does not validate thatitem.dataexists or that expected properties are present. If the schema is malformed, this could silently produce objects with undefined properties, leading to runtime errors downstream.Apply this diff to add a basic guard:
const normalizeItem = (item: any) => { + if (!item || !item.data) { + console.warn('normalizeItem received invalid item:', item) + return null + } return { id: item.id, name: item.name,Then filter out null results when mapping:
- list: (rawConfig.list || []).map(normalizeItem) + list: (rawConfig.list || []).map(normalizeItem).filter(Boolean)
31-31: Consider typing dataSourceMap more strictly.The
Record<string, any>type provides no type safety for consumers. If the dataSource shape is well-defined, consider defining an interface or type to improve maintainability and IDE support.Example:
interface DataSource { config: any // or a more specific config type status: 'init' | 'loading' | 'loaded' | 'error' data: any error?: any load: (params?: any, customUrl?: string) => Promise<any> } export const dataSourceMap: Record<string, DataSource> = {}
53-55: Clarify the intent of returningundefinedwhen shouldFetch is false.When
shouldFetch()returnsfalse, the function resolves withundefined. This behavior should be documented, as callers must explicitly handle this case. Alternatively, consider returning a resolved promise with a sentinel value or a specific status to make the intent clearer.Example:
if (!shouldFetch()) { - return Promise.resolve(undefined) + // Do not fetch; return a no-op response + return Promise.resolve({ code: 'NO_FETCH', msg: 'Fetch skipped by shouldFetch', data: null }) }
36-71: Inconsistent error handling between local and remote data paths.The local data path (lines 38-51) catches exceptions, updates
dataSource.statusanddataSource.error, and rejects the promise. However, the remote data path (lines 57-70) delegates error handling to the HTTP interceptors (configured later at lines 103-104). This asymmetry means that errors in the local path are handled inline, while remote errors are handled elsewhere.For consistency and easier debugging, consider explicitly handling remote errors in the load function or clearly documenting that error handling is centralized in the interceptors.
Apply this diff to add explicit error handling for the remote path:
- return http.request(config) + return http.request(config).catch((error) => { + dataSource.status = 'error' + dataSource.error = error + throw error + })Alternatively, document that error handling is delegated to interceptors.
121-121: Both default and named exports are provided.Exporting
dataSourceMapas both a named export (line 31) and the default export (line 121) is acceptable, but may cause confusion for consumers. Consider standardizing on one pattern across the codebase for consistency.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (56)
designer-demo/package.json(1 hunks)designer-demo/runtime.html(1 hunks)designer-demo/src/runtime.js(1 hunks)packages/build/vite-config/src/vite-plugins/devAliasPlugin.js(1 hunks)packages/common/js/runtime-deploy.js(1 hunks)packages/design-core/package.json(1 hunks)packages/design-core/re-export.js(1 hunks)packages/design-core/registry.js(2 hunks)packages/layout/src/defaultLayout.js(1 hunks)packages/register/src/constants.ts(1 hunks)packages/runtime-renderer/index.ts(1 hunks)packages/runtime-renderer/package.json(1 hunks)packages/runtime-renderer/src/App.vue(1 hunks)packages/runtime-renderer/src/app-function/dataSource.ts(1 hunks)packages/runtime-renderer/src/app-function/http/axios.js(1 hunks)packages/runtime-renderer/src/app-function/http/config.js(1 hunks)packages/runtime-renderer/src/app-function/http/index.js(1 hunks)packages/runtime-renderer/src/app-function/utils.ts(1 hunks)packages/runtime-renderer/src/components/Loading.vue(1 hunks)packages/runtime-renderer/src/components/NotFound.vue(1 hunks)packages/runtime-renderer/src/components/PageRenderer.vue(1 hunks)packages/runtime-renderer/src/composables/useAppSchema.ts(1 hunks)packages/runtime-renderer/src/mock/blocks.json(1 hunks)packages/runtime-renderer/src/renderer/LifecycleWrapper.ts(1 hunks)packages/runtime-renderer/src/renderer/RenderMain.ts(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasBox.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasCollection.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasIcon.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasImg.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasPlaceholder.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasRouterLink.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasRouterView.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasSlot.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasText.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/index.ts(1 hunks)packages/runtime-renderer/src/renderer/page-function/accessor.ts(1 hunks)packages/runtime-renderer/src/renderer/page-function/blockContext.ts(1 hunks)packages/runtime-renderer/src/renderer/page-function/css-handler.ts(1 hunks)packages/runtime-renderer/src/renderer/page-function/index.ts(1 hunks)packages/runtime-renderer/src/renderer/page-function/state.ts(1 hunks)packages/runtime-renderer/src/renderer/parser/index.ts(1 hunks)packages/runtime-renderer/src/renderer/parser/parser.ts(1 hunks)packages/runtime-renderer/src/renderer/render.ts(1 hunks)packages/runtime-renderer/src/renderer/useContext.ts(1 hunks)packages/runtime-renderer/src/router/index.ts(1 hunks)packages/runtime-renderer/src/stores/index.ts(1 hunks)packages/runtime-renderer/src/types/config.ts(1 hunks)packages/runtime-renderer/src/types/schema.ts(1 hunks)packages/runtime-renderer/src/utils/data-utils.ts(1 hunks)packages/runtime-renderer/vite.config.ts(1 hunks)packages/toolbars/runtime-deploy/index.ts(1 hunks)packages/toolbars/runtime-deploy/meta.js(1 hunks)packages/toolbars/runtime-deploy/package.json(1 hunks)packages/toolbars/runtime-deploy/src/Main.vue(1 hunks)packages/toolbars/runtime-deploy/vite.config.ts(1 hunks)tsconfig.app.json(2 hunks)
🧰 Additional context used
🧠 Learnings (4)
📚 Learning: 2025-01-14T06:49:00.797Z
Learnt from: gene9831
PR: opentiny/tiny-engine#1011
File: packages/configurator/src/router-select-configurator/RouterSelectConfigurator.vue:63-73
Timestamp: 2025-01-14T06:49:00.797Z
Learning: In the tiny-engine project, the SvgIcon component is globally registered and available throughout Vue components without requiring explicit imports.
Applied to files:
packages/runtime-renderer/src/renderer/builtin/index.tspackages/runtime-renderer/src/renderer/builtin/CanvasIcon.vue
📚 Learning: 2024-09-30T07:51:10.036Z
Learnt from: chilingling
PR: opentiny/tiny-engine#837
File: packages/vue-generator/src/plugins/genDependenciesPlugin.js:66-66
Timestamp: 2024-09-30T07:51:10.036Z
Learning: In the `tiny-engine` project, `opentiny/tiny-engine-dsl-vue` refers to the current package itself, and importing types from it may cause circular dependencies.
Applied to files:
designer-demo/package.jsontsconfig.app.jsonpackages/runtime-renderer/package.json
📚 Learning: 2025-01-14T06:54:37.955Z
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/page-block-function/css.ts:3-7
Timestamp: 2025-01-14T06:54:37.955Z
Learning: The setPageCss function in packages/canvas/render/src/page-block-function/css.ts is intentionally kept simple without additional type safety or error handling as per the maintainer's preference.
Applied to files:
packages/runtime-renderer/src/renderer/page-function/css-handler.ts
📚 Learning: 2025-01-14T06:49:00.797Z
Learnt from: gene9831
PR: opentiny/tiny-engine#1011
File: packages/configurator/src/router-select-configurator/RouterSelectConfigurator.vue:63-73
Timestamp: 2025-01-14T06:49:00.797Z
Learning: In the tiny-engine project, the SvgIcon component is globally registered using `app.component('SvgIcon', SvgIcon)` in `packages/svgs/index.js`, making it available throughout Vue components without requiring explicit imports.
Applied to files:
packages/runtime-renderer/src/renderer/builtin/CanvasIcon.vue
🧬 Code graph analysis (18)
packages/common/js/runtime-deploy.js (1)
packages/register/src/common.ts (1)
getMergeMeta(53-55)
packages/runtime-renderer/src/renderer/LifecycleWrapper.ts (1)
packages/runtime-renderer/src/types/schema.ts (1)
JSFunction(129-132)
packages/runtime-renderer/src/app-function/http/index.js (2)
packages/runtime-renderer/src/app-function/http/axios.js (1)
config(16-143)packages/runtime-renderer/src/app-function/dataSource.ts (1)
response(110-112)
packages/runtime-renderer/src/app-function/utils.ts (2)
packages/runtime-renderer/src/types/schema.ts (1)
Util(184-188)packages/runtime-renderer/src/utils/data-utils.ts (1)
parseJSFunction(15-23)
packages/runtime-renderer/index.ts (3)
packages/runtime-renderer/src/composables/useAppSchema.ts (1)
useAppSchema(9-164)packages/runtime-renderer/src/router/index.ts (1)
createAppRouter(78-91)packages/runtime-renderer/src/stores/index.ts (2)
generateStoresConfig(7-26)createStores(28-45)
packages/runtime-renderer/src/renderer/RenderMain.ts (8)
packages/runtime-renderer/src/composables/useAppSchema.ts (1)
useAppSchema(9-164)packages/canvas/render/src/page-block-function/context.ts (1)
useContext(15-31)packages/plugins/materials/src/composable/types.ts (1)
Schema(305-317)packages/runtime-renderer/src/renderer/page-function/state.ts (1)
useState(6-46)packages/runtime-renderer/src/renderer/parser/parser.ts (1)
parseData(228-231)packages/runtime-renderer/src/app-function/utils.ts (1)
getUtilsAll(68-70)packages/runtime-renderer/src/renderer/page-function/css-handler.ts (1)
setPageCss(72-82)packages/runtime-renderer/src/renderer/LifecycleWrapper.ts (1)
PageLifecycleWrapper(48-208)
packages/runtime-renderer/src/composables/useAppSchema.ts (2)
packages/runtime-renderer/src/types/schema.ts (4)
AppSchema(2-22)Util(184-188)I18nConfig(232-235)BlockItem(248-287)packages/runtime-renderer/src/app-function/utils.ts (1)
initUtils(45-66)
packages/runtime-renderer/src/renderer/page-function/state.ts (2)
packages/runtime-renderer/src/renderer/page-function/accessor.ts (1)
useAccessorMap(10-50)packages/runtime-renderer/src/renderer/parser/parser.ts (2)
parseData(228-231)isStateAccessor(66-67)
packages/layout/src/defaultLayout.js (1)
packages/register/src/constants.ts (1)
META_APP(24-81)
packages/runtime-renderer/src/renderer/render.ts (3)
packages/runtime-renderer/src/renderer/page-function/blockContext.ts (1)
getBlockContext(72-76)packages/canvas/render/src/render.ts (1)
renderDefault(27-35)packages/runtime-renderer/src/renderer/parser/parser.ts (3)
parseData(228-231)parseLoopArgs(238-245)parseCondition(233-236)
packages/runtime-renderer/src/app-function/dataSource.ts (4)
packages/runtime-renderer/src/composables/useAppSchema.ts (1)
useAppSchema(9-164)packages/runtime-renderer/src/utils/data-utils.ts (1)
parseJSFunction(15-23)packages/runtime-renderer/src/app-function/http/axios.js (1)
config(16-143)packages/register/src/hooks.ts (1)
useHttp(89-89)
packages/runtime-renderer/src/stores/index.ts (3)
packages/runtime-renderer/src/composables/useAppSchema.ts (1)
useAppSchema(9-164)packages/runtime-renderer/src/utils/data-utils.ts (1)
parseJSFunction(15-23)packages/runtime-renderer/src/types/config.ts (1)
StoreConfig(25-30)
packages/runtime-renderer/src/router/index.ts (2)
packages/runtime-renderer/src/composables/useAppSchema.ts (1)
useAppSchema(9-164)packages/runtime-renderer/src/types/config.ts (1)
RouteConfig(7-23)
packages/runtime-renderer/src/app-function/http/axios.js (1)
packages/runtime-renderer/src/app-function/dataSource.ts (1)
response(110-112)
packages/runtime-renderer/src/renderer/page-function/css-handler.ts (1)
packages/common/js/preview.js (1)
pageId(236-236)
packages/runtime-renderer/src/renderer/page-function/blockContext.ts (5)
packages/canvas/render/src/page-block-function/context.ts (1)
useContext(15-31)packages/runtime-renderer/src/renderer/page-function/state.ts (1)
useState(6-46)packages/runtime-renderer/src/renderer/parser/parser.ts (1)
parseData(228-231)packages/runtime-renderer/src/app-function/utils.ts (1)
getUtilsAll(68-70)packages/runtime-renderer/src/renderer/page-function/css-handler.ts (1)
getCSSHandler(90-104)
designer-demo/src/runtime.js (1)
packages/runtime-renderer/index.ts (1)
initRuntimeRenderer(22-39)
packages/runtime-renderer/src/renderer/parser/parser.ts (4)
packages/vue-generator/src/utils/index.js (1)
fnRegexp(20-20)packages/runtime-renderer/src/utils/data-utils.ts (1)
parseJSFunction(15-23)packages/canvas/render/src/material-function/material-getter.ts (1)
getIcon(106-106)packages/vue-generator/src/generator/vue/sfc/generateAttribute.js (3)
propsEntries(641-641)modelValue(642-642)hasUpdateModelValue(643-643)
littla2liyou
changed the title
littla2liyou
changed the title
littla2liyou
changed the title
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (25)
packages/runtime-renderer/src/components/NotFound.vue (2)
143-178: Remove unused CSS for unimplemented feature.The CSS rules for
.available-pages,.page-list, and.page-linkhave no corresponding elements in the template. This is dead code for an unimplemented "available pages" feature.
200-208: Remove unused responsive rules.These media query rules target the same unused
.page-listand.page-linkclasses.packages/runtime-renderer/src/renderer/useContext.ts (1)
28-28: Remove or translate the orphaned comment.The Chinese comment "从大纲树控制隐藏" (control hide from outline tree) appears unrelated to the surrounding
setContextlogic and breaks language consistency.packages/runtime-renderer/src/renderer/builtin/CanvasRouterLink.vue (1)
51-75: Reduce code duplication and translate comments to English.The
activeandexactActivecomputed properties contain duplicated route resolution logic. The Chinese comments should be translated for consistency.packages/runtime-renderer/src/components/Loading.vue (1)
1-84: Add ARIA status for the loader.Screen-reader users get no loading feedback. Add
role="status"/aria-liveplus hidden text so the state is announced.- <div class="loading-warp"> + <div class="loading-warp" role="status" aria-live="polite" aria-busy="true"> <div class="loading"> <div><span></span></div> <div><span></span></div> <div><span></span></div> </div> + <span class="sr-only">Loading...</span> </div> </template> <style> +.sr-only { + position: absolute; + width: 1px; + height: 1px; + padding: 0; + margin: -1px; + overflow: hidden; + clip: rect(0, 0, 0, 0); + white-space: nowrap; + border-width: 0; +}packages/runtime-renderer/src/renderer/builtin/CanvasIcon.vue (1)
12-16: Avoid rendering with an undefined vnode type.When
nameis missing/invalid,SvgIcons[name]?.()yieldsundefined, soh(undefined, …)throws before the fallback ever runs. Guard the lookup and return a safe fallback vnode instead.- return () => - h('span', { class: 'iconwarp' }, [ - h(SvgIcons[props.name]?.(), { style: { width: '100%', height: '100%' } }) || 'error.' - ]) + return () => { + const iconFactory = props.name && SvgIcons[props.name] + if (!iconFactory) { + return h('span', { class: 'iconwarp iconwarp--missing' }, 'error.') + } + + const IconComponent = typeof iconFactory === 'function' ? iconFactory() : iconFactory + return h('span', { class: 'iconwarp' }, [ + h(IconComponent, { style: { width: '100%', height: '100%' } }) + ]) + }packages/runtime-renderer/src/mock/blocks.json (1)
161-171: Fix props access in state setter.
this.firstname/this.lastnameare undefined here. The schema exposes them as props, so the setter never emits the correct values. Usethis.propswhen emitting to keep the state in sync.- this.emit('update:firstname', this.firstname) - this.emit('update:lastname', this.lastname) + this.emit('update:firstname', this.props.firstname) + this.emit('update:lastname', this.props.lastname)packages/runtime-renderer/src/composables/useAppSchema.ts (1)
55-61: Handle non-OK schema responses and validate payload before use.HTTP failures still flow into
await response.json()andinitializeAppConfig, so a 404/500 (or malformed body) leavesappSchemapolluted or throws mid-initialization. Guardresponse.ok, validate the parsed shape, and bail with a clear error before touching state.const response = await fetch(`/app-center/v1/api/apps/schema/${appId}`) - appSchema.value = await response.json() - - // 解析并初始化应用级配置 - await initializeAppConfig(appSchema.value) + if (!response.ok) { + throw new Error(`加载应用Schema失败: ${response.status} ${response.statusText}`) + } + + const data = await response.json() + if (!data || typeof data !== 'object' || !data.data) { + throw new Error('加载应用Schema失败: 返回结构不符合预期') + } + + appSchema.value = data + + // 解析并初始化应用级配置 + await initializeAppConfig(appSchema.value)packages/common/js/runtime-deploy.js (3)
17-24: Don't emit null/undefined query parameters.String interpolation is producing
id=null/platform=undefined, which leaks bogus values into the runtime URL. Build the search params defensively and skip empty entries.- let query = `id=${appId}&tenant=${tenant}&platform=${platform}` - return query + const params = new URLSearchParams() + if (appId) params.set('id', appId) + if (tenant) params.set('tenant', tenant) + if (platform) params.set('platform', String(platform)) + + return params.toString()
34-41: Await async deployUrl overrides and fall back safely.If a toolbar override returns a Promise,
openUrlbecomes[object Promise]andwindow.openfails. Await function overrides, validate the result, and fall back to the default URL when the override is falsey.- let openUrl = '' - openUrl = customDeployUrl - ? typeof customDeployUrl === 'function' - ? customDeployUrl(defaultDeployUrl, query) - : `${customDeployUrl}?${query}` - : `${defaultDeployUrl}?${query}` - - return { openUrl } + let openUrl = '' + + if (customDeployUrl) { + if (typeof customDeployUrl === 'function') { + const override = await customDeployUrl(defaultDeployUrl, query) + if (typeof override === 'string' && override.trim()) { + openUrl = override + } + } else { + openUrl = `${customDeployUrl}?${query}` + } + } + + if (!openUrl) { + openUrl = `${defaultDeployUrl}?${query}` + } + + return { openUrl }
44-58: Handle popup-blockedwindow.openfailures.
window.openreturnsnullwhen blocked, so we silently lose the deploy action. Detect the failure, log it, and avoid keepingruntimeWindowstale so users can retry.- runtimeWindow = window.open(openUrl, 'tiny-engine-runtime') + const opened = window.open(openUrl, 'tiny-engine-runtime') + if (!opened) { + // eslint-disable-next-line no-console + console.error('[runtime-deploy] 打开运行时窗口失败，可能被浏览器拦截') + return + } + runtimeWindow = openedpackages/runtime-renderer/src/utils/data-utils.ts (1)
15-22: Block unsafe dynamic code execution inparseJSFunction.
new Functionon untrusteddata.valueis a straight code-injection vector; any compromised schema executes arbitrary JS inside the renderer process. Add strict validation/sanitization (or a sandboxed interpreter) before evaluation and surface failures via the project logger, falling back to a safe no-op rather than executing raw input.packages/runtime-renderer/src/app-function/http/axios.js (2)
21-55: Avoid mutatingMockAdapter.prototypeglobally.Attaching
proxyon the prototype pollutes everyMockAdapterconsumer, risking collisions with other modules or future library releases. Extend per-instance instead so the behavior is scoped.- if (typeof MockAdapter.prototype.proxy === 'undefined') { - MockAdapter.prototype.proxy = function ({ url, config = {}, proxy, response, handleData } = {}) { - // eslint-disable-next-line @typescript-eslint/no-this-alias - let stream = this + const attachProxy = (mockInstance) => { + if (typeof mockInstance.proxy === 'function') { + return mockInstance + } + mockInstance.proxy = function ({ url, config = {}, proxy, response, handleData } = {}) { + // eslint-disable-next-line @typescript-eslint/no-this-alias + let stream = this const request = (proxy, any) => { return (setting) => { return new Promise((resolve) => { config.responseType = 'json' axios .get(any ? proxy + setting.url + '.json' : proxy, config) .then(({ data }) => { if (typeof handleData === 'function') { data = handleData.call(null, data, setting) } resolve([200, data]) }) - .catch((error) => { - resolve([error.response.status, error.response.data]) - }) + .catch((error) => { + const status = error.response?.status ?? 0 + const payload = error.response?.data ?? { message: error.message || 'Request failed' } + resolve([status, payload]) + }) }) } } if (url === '*' && proxy && typeof proxy === 'string') { stream = proxy === '*' ? this.onAny().passThrough() : this.onAny().reply(request(proxy, true)) } else { if (proxy && typeof proxy === 'string') { stream = this.onAny(url).reply(request(proxy)) } else if (typeof response === 'function') { stream = this.onAny(url).reply(response) } } return stream } - } + return mockInstance + }And when creating the mock:
- mock = new MockAdapter(instance) + mock = attachProxy(new MockAdapter(instance))
123-129: Ensure proxy fetch errors always resolve with safe defaults.Network errors leave
error.responseundefined, so the currentresolve([error.response.status, ...])will throw, breaking the mock flow. Guard against missing response and provide a default payload.- mock.proxy(item) + mock.proxy(item)(Use the guarded implementation shown above inside
attachProxy.)packages/runtime-renderer/src/app-function/http/index.js (1)
19-24: Propagate HTTP errors and guardresponseaccess.The interceptor swallows every rejection and assumes
responseexists; callers never see failures and a network error throws before we hit the TODO. ReturnPromise.reject(error)and null-checkresponsefirst.http.interceptors.response.use(dataHandler, (error) => { const response = error.response - if (response.status === 403 && response.headers && response.headers['x-login-url']) { + if (response && response.status === 403 && response.headers && response.headers['x-login-url']) { // TODO 处理无权限时，重新登录再发送请求 } + return Promise.reject(error) })packages/runtime-renderer/src/app-function/utils.ts (2)
31-31: Critical: Undefined variable reference (duplicate).Line 31 references an undefined variable
pkgwhich should bec.package. This issue was flagged in a previous review and remains unresolved.
59-61: Inconsistent error handling (duplicate).Lines 59-61 call
parseJSFunctionwithout error handling, while npm utilities are protected with try/catch (lines 53-58). This inconsistency was flagged in a previous review and remains unresolved. IfparseJSFunctionthrows, the module will be left in an inconsistent state (loading = true,initialized = false).designer-demo/src/runtime.js (1)
14-16: Missing await on async initialization call.The
initRuntimeRenderer()function returns a Promise, but line 15 doesn't await it, leading to unhandled promise rejections and loss of the app instance.packages/runtime-renderer/package.json (1)
22-22: Axios version successfully pinned.The
axiosdependency has been correctly updated from"latest"to"^1.10.0", addressing the previous review concern about unpinned versions.packages/runtime-renderer/src/renderer/builtin/CanvasImg.vue (1)
1-3: Add alt attribute for accessibility.The
<img>element lacks analtattribute, which is required for screen readers and WCAG 2.1 compliance.Apply this diff:
<template> - <img :src="src" :style="style" /> + <img :src="src" :style="style" :alt="alt" /> </template>Add the prop:
props: { src: { type: String, default: '' }, style: { type: String, default: '' + }, + alt: { + type: String, + default: '' } }packages/runtime-renderer/src/renderer/builtin/CanvasBox.vue (1)
1-5: SetinheritAttrs: falseto prevent duplicate attribute binding.When using
v-bind="$attrs"to forward attributes to a child component, Vue also automatically binds$attrsto the root element unlessinheritAttrs: falseis set, which can result in duplicate attributes.Apply this diff:
<script> export default { + inheritAttrs: false, props: { tag: { type: String, default: 'div' } } } </script>packages/runtime-renderer/index.ts (2)
23-27: Handle missingidquery parameter before fetching schemaIf the URL lacks
?id=..., we send/app-center/v1/api/apps/schema/and keep bootstrapping with an empty schema, which leaves the runtime in a broken state. Bail out early (throw or show a user-facing error) instead of invokingfetchAppSchemawith an empty string.const searchParams = new URLSearchParams(location.search) const appId = searchParams.get('id') - const { fetchAppSchema, fetchBlocks } = useAppSchema() - await fetchAppSchema(appId || '') + if (!appId) { + throw new Error('Missing required "id" query parameter') + } + const { fetchAppSchema, fetchBlocks } = useAppSchema() + await fetchAppSchema(appId)
34-38: Callapp.providebefore mounting
app.provide('stores', ...)has to run beforeapp.mount('#app'); otherwiseinject('stores')in root components resolves toundefinedduring their initial setup. Move the provide call ahead of mount so dependencies are available from the first render.const app = createApp(App) - app.use(pinia).use(router).use(i18n).mount('#app') - - app.provide('stores', stores) + app.provide('stores', stores) + app.use(pinia).use(router).use(i18n).mount('#app')packages/runtime-renderer/src/stores/index.ts (1)
12-24: Guard against failed action/getter parsing before wiring Pinia stores.If
parseJSFunctioncannot compile a user-defined action/getter it returnsundefined, which we then register directly with Pinia. The first invocation will throwTypeError: fn is not a function, breaking the runtime. Please add a safe fallback (e.g., warn + noop/identity) before inserting entries, and avoidJSON.parse(JSON.stringify(...))so we don’t silently drop non-JSON state (functions, Dates, undefined). One concrete fix is:-import { defineStore, type Pinia } from 'pinia' -import { shallowReactive } from 'vue' +import { defineStore, type Pinia } from 'pinia' +import { shallowReactive } from 'vue' + +const cloneState = (value: Record<string, any>) => { + if (typeof structuredClone === 'function') return structuredClone(value) + return JSON.parse(JSON.stringify(value)) +} + +const parseOrFallback = ( + descriptor: any, + storeId: string, + key: string, + fallback: (...args: any[]) => any +) => { + const fn = parseJSFunction(descriptor, {}, {}) + if (typeof fn === 'function') return fn + console.error(`Failed to parse ${key} for store ${storeId}`) + return fallback +} @@ - return globalStates.value.map((store) => ({ - id: store.id, - state: JSON.parse(JSON.stringify(store.state)), - actions: Object.fromEntries( - Object.keys(store.actions || {}).map((key) => { - // 使用 parseJSFunction ，但是上下文由pinia内部绑定 - return [key, parseJSFunction(store.actions[key], {}, {})] - }) - ), - getters: Object.fromEntries( - Object.keys(store.getters || {}).map((key) => { - // 同样处理 getters - return [key, parseJSFunction(store.getters[key], {}, {})] - }) - ) - })) + return globalStates.value.map((store) => ({ + id: store.id, + state: cloneState(store.state || {}), + actions: Object.fromEntries( + Object.keys(store.actions || {}).map((key) => [ + key, + parseOrFallback(store.actions[key], store.id, key, () => {}) + ]) + ), + getters: Object.fromEntries( + Object.keys(store.getters || {}).map((key) => [ + key, + parseOrFallback(store.getters[key], store.id, key, () => undefined) + ]) + ) + }))This keeps the renderer alive even when authors paste invalid code.
packages/runtime-renderer/src/router/index.ts (1)
38-51: Don't drop child routes when parent appears later.If a child page is iterated before its parent,
parentRouteisundefined, wereturn, and the child route vanishes. In real schemas the ordering is not guaranteed, so nested pages disappear. Please buffer unmatched children (or build a route map) and attach them once the parent is available. While fixing this, switch toNumber.parseInt(..., 10)to avoid radix bugs. Otherwise runtime navigation breaks for perfectly valid trees.
Example patch:- pages.value.forEach((page) => { + const routesById = new Map<number, RouteConfig>() + const pendingChildren: Array<{ parentId: number; route: RouteConfig }> = [] + + pages.value.forEach((page) => { const isChildRoute = page.meta.parentId !== '0' - - const routeConfigCurrent = { + const routeConfigCurrent = { path: isChildRoute ? page.meta.router : `/${page.meta.router}`, name: `${page.meta.id}`, component: () => import('../components/PageRenderer.vue'), props: { pageId: page.meta.id }, children: [], meta: { pageId: page.meta.id, pageName: page.meta.name, isHome: page.meta.isHome, hasChildren: (page.children && page.children.length > 0) || false, depth: page.meta.depth, isDefault: page.meta.isDefault, hasDefault: false, defaultPath: '', parentPath: '/' } } + routesById.set(page.meta.id, routeConfigCurrent) + if (isChildRoute) { - const parentId = parseInt(page.meta.parentId) - const parentRoute = routesConfig.find((r) => r.meta?.pageId === parentId) + const parentId = Number.parseInt(page.meta.parentId, 10) + const parentRoute = routesById.get(parentId) if (parentRoute) { parentRoute.children = parentRoute.children || [] parentRoute.children.push(routeConfigCurrent) parentRoute.meta.hasChildren = true if (routeConfigCurrent.meta.isDefault) { parentRoute.meta.hasDefault = true parentRoute.meta.defaultPath = `${parentRoute.path}/${routeConfigCurrent.path}` parentRoute.redirect = parentRoute.meta.defaultPath } return } + pendingChildren.push({ parentId, route: routeConfigCurrent }) } else { routesConfig.push(routeConfigCurrent) } }) + + pendingChildren.forEach(({ parentId, route }) => { + const parentRoute = routesById.get(parentId) + if (!parentRoute) { + console.error(`父路由未找到: 页面 "${route.meta.pageName}" (ID: ${route.meta.pageId}) 引用的父路由 ID ${parentId} 不存在`) + routesConfig.push(route) + return + } + parentRoute.children = parentRoute.children || [] + parentRoute.children.push(route) + parentRoute.meta.hasChildren = true + if (route.meta.isDefault) { + parentRoute.meta.hasDefault = true + parentRoute.meta.defaultPath = `${parentRoute.path}/${route.path}` + parentRoute.redirect = parentRoute.meta.defaultPath + } + })
🧹 Nitpick comments (5)
packages/runtime-renderer/src/renderer/page-function/css-handler.ts (1)
22-22: Translate comment to English.The Chinese comment breaks language consistency in the codebase.
Apply this diff:
- // 简化版的作用域处理,将CSS选择器添加页面作用域 + // Simplified scoping: adds page scope to CSS selectors function processScopedCSS(key: string, css: string): string {packages/runtime-renderer/src/renderer/builtin/CanvasIcon.vue (1)
1-16: Do not import the entire icon library eagerly.
import * as SvgIconspulls every icon into this bundle, bloating the runtime renderer. Use the globally registered<SvgIcon>component or a narrow registry so only requested icons ship. Based on learnings.-<script> -import * as SvgIcons from '@opentiny/vue-icon' -import { h } from 'vue' +<script> +import { h, resolveComponent } from 'vue' @@ - return () => { - const iconFactory = props.name && SvgIcons[props.name] - if (!iconFactory) { - return h('span', { class: 'iconwarp iconwarp--missing' }, 'error.') - } - - const IconComponent = typeof iconFactory === 'function' ? iconFactory() : iconFactory - return h('span', { class: 'iconwarp' }, [ - h(IconComponent, { style: { width: '100%', height: '100%' } }) - ]) - } + const SvgIcon = resolveComponent('SvgIcon') + return () => + h('span', { class: 'iconwarp' }, [ + props.name && SvgIcon + ? h(SvgIcon, { name: props.name, style: { width: '100%', height: '100%' } }) + : 'error.' + ])packages/runtime-renderer/src/renderer/builtin/CanvasRouterView.vue (1)
7-11: Document the magic number.The
min-height: 48pxvalue appears to be a magic number without explanation. Consider adding a comment to document why this specific height was chosen (e.g., minimum clickable area, specific design requirement, etc.).packages/runtime-renderer/src/renderer/builtin/CanvasText.vue (1)
5-18: Prefer Composition API for new Vue 3 components.This component uses the Options API. For new Vue 3 code, the Composition API is recommended for better TypeScript support, code reusability, and consistency with the Vue 3 ecosystem.
Based on learnings
Consider refactoring to use Composition API:
-<script> -export default { - props: { - text: { - type: String, - default: '' - }, - className: { - type: String, - default: '' - } - } -} +<script setup lang="ts"> +defineProps<{ + text?: string + className?: string +}>() </script>packages/runtime-renderer/package.json (1)
23-23: Consider upgradingaxios-mock-adapterto 1.20.0.The current version (1.19.0, released Oct 2023) is functional, but version 1.20.0 (released Aug 2025) includes improvements such as Blob response support and better adapter type exposure. Based on learnings.
Apply this diff:
- "axios-mock-adapter": "^1.19.0", + "axios-mock-adapter": "^1.20.0",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (56)
designer-demo/package.json(1 hunks)designer-demo/runtime.html(1 hunks)designer-demo/src/runtime.js(1 hunks)packages/build/vite-config/src/vite-plugins/devAliasPlugin.js(1 hunks)packages/common/js/runtime-deploy.js(1 hunks)packages/design-core/package.json(1 hunks)packages/design-core/re-export.js(1 hunks)packages/design-core/registry.js(2 hunks)packages/layout/src/defaultLayout.js(1 hunks)packages/register/src/constants.ts(1 hunks)packages/runtime-renderer/index.ts(1 hunks)packages/runtime-renderer/package.json(1 hunks)packages/runtime-renderer/src/App.vue(1 hunks)packages/runtime-renderer/src/app-function/dataSource.ts(1 hunks)packages/runtime-renderer/src/app-function/http/axios.js(1 hunks)packages/runtime-renderer/src/app-function/http/config.js(1 hunks)packages/runtime-renderer/src/app-function/http/index.js(1 hunks)packages/runtime-renderer/src/app-function/utils.ts(1 hunks)packages/runtime-renderer/src/components/Loading.vue(1 hunks)packages/runtime-renderer/src/components/NotFound.vue(1 hunks)packages/runtime-renderer/src/components/PageRenderer.vue(1 hunks)packages/runtime-renderer/src/composables/useAppSchema.ts(1 hunks)packages/runtime-renderer/src/mock/blocks.json(1 hunks)packages/runtime-renderer/src/renderer/LifecycleWrapper.ts(1 hunks)packages/runtime-renderer/src/renderer/RenderMain.ts(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasBox.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasCollection.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasIcon.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasImg.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasPlaceholder.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasRouterLink.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasRouterView.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasSlot.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/CanvasText.vue(1 hunks)packages/runtime-renderer/src/renderer/builtin/index.ts(1 hunks)packages/runtime-renderer/src/renderer/page-function/accessor.ts(1 hunks)packages/runtime-renderer/src/renderer/page-function/blockContext.ts(1 hunks)packages/runtime-renderer/src/renderer/page-function/css-handler.ts(1 hunks)packages/runtime-renderer/src/renderer/page-function/index.ts(1 hunks)packages/runtime-renderer/src/renderer/page-function/state.ts(1 hunks)packages/runtime-renderer/src/renderer/parser/index.ts(1 hunks)packages/runtime-renderer/src/renderer/parser/parser.ts(1 hunks)packages/runtime-renderer/src/renderer/render.ts(1 hunks)packages/runtime-renderer/src/renderer/useContext.ts(1 hunks)packages/runtime-renderer/src/router/index.ts(1 hunks)packages/runtime-renderer/src/stores/index.ts(1 hunks)packages/runtime-renderer/src/types/config.ts(1 hunks)packages/runtime-renderer/src/types/schema.ts(1 hunks)packages/runtime-renderer/src/utils/data-utils.ts(1 hunks)packages/runtime-renderer/vite.config.ts(1 hunks)packages/toolbars/runtime-deploy/index.ts(1 hunks)packages/toolbars/runtime-deploy/meta.js(1 hunks)packages/toolbars/runtime-deploy/package.json(1 hunks)packages/toolbars/runtime-deploy/src/Main.vue(1 hunks)packages/toolbars/runtime-deploy/vite.config.ts(1 hunks)tsconfig.app.json(2 hunks)
🧰 Additional context used
🧠 Learnings (5)
📚 Learning: 2024-09-30T07:51:10.036Z
Learnt from: chilingling
PR: opentiny/tiny-engine#837
File: packages/vue-generator/src/plugins/genDependenciesPlugin.js:66-66
Timestamp: 2024-09-30T07:51:10.036Z
Learning: In the `tiny-engine` project, `opentiny/tiny-engine-dsl-vue` refers to the current package itself, and importing types from it may cause circular dependencies.
Applied to files:
designer-demo/package.jsontsconfig.app.jsonpackages/runtime-renderer/package.json
📚 Learning: 2024-11-06T09:38:05.573Z
Learnt from: yy-wow
PR: opentiny/tiny-engine#886
File: packages/common/composable/defaultGlobalService.js:53-56
Timestamp: 2024-11-06T09:38:05.573Z
Learning: 在 `packages/common/composable/defaultGlobalService.js` 文件中，对于 `fetchAppInfo` 和 `fetchAppList` 等函数，错误处理应由调用者负责，不应在函数内部添加错误处理。
Applied to files:
packages/runtime-renderer/src/composables/useAppSchema.ts
📚 Learning: 2025-01-14T06:50:21.158Z
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/data-function/parser.ts:191-195
Timestamp: 2025-01-14T06:50:21.158Z
Learning: The `newFn` function in `packages/canvas/render/src/data-function/parser.ts` has a known security issue with evaluating untrusted code through `data.value`. This was identified during code review but intentionally deferred as the original implementation was kept during code refactoring.
Applied to files:
packages/runtime-renderer/src/utils/data-utils.tspackages/runtime-renderer/src/renderer/parser/parser.ts
📚 Learning: 2025-01-14T06:49:00.797Z
Learnt from: gene9831
PR: opentiny/tiny-engine#1011
File: packages/configurator/src/router-select-configurator/RouterSelectConfigurator.vue:63-73
Timestamp: 2025-01-14T06:49:00.797Z
Learning: In the tiny-engine project, the SvgIcon component is globally registered and available throughout Vue components without requiring explicit imports.
Applied to files:
packages/runtime-renderer/src/renderer/builtin/index.tspackages/runtime-renderer/src/renderer/builtin/CanvasIcon.vue
📚 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/runtime-renderer/index.ts
🧬 Code graph analysis (18)
packages/runtime-renderer/src/renderer/page-function/css-handler.ts (1)
packages/common/js/preview.js (1)
pageId(236-236)
packages/runtime-renderer/src/renderer/page-function/blockContext.ts (5)
packages/canvas/render/src/page-block-function/context.ts (1)
useContext(15-31)packages/runtime-renderer/src/renderer/page-function/state.ts (1)
useState(6-46)packages/runtime-renderer/src/renderer/parser/parser.ts (1)
parseData(228-231)packages/runtime-renderer/src/app-function/utils.ts (1)
getUtilsAll(68-70)packages/runtime-renderer/src/renderer/page-function/css-handler.ts (1)
getCSSHandler(90-104)
packages/runtime-renderer/src/stores/index.ts (3)
packages/runtime-renderer/src/composables/useAppSchema.ts (1)
useAppSchema(9-164)packages/runtime-renderer/src/utils/data-utils.ts (1)
parseJSFunction(15-23)packages/runtime-renderer/src/types/config.ts (1)
StoreConfig(25-30)
packages/runtime-renderer/src/renderer/page-function/state.ts (2)
packages/runtime-renderer/src/renderer/page-function/accessor.ts (1)
useAccessorMap(10-50)packages/runtime-renderer/src/renderer/parser/parser.ts (2)
parseData(228-231)isStateAccessor(66-67)
packages/runtime-renderer/src/app-function/http/axios.js (1)
packages/runtime-renderer/src/app-function/dataSource.ts (1)
response(110-112)
packages/runtime-renderer/src/app-function/dataSource.ts (4)
packages/runtime-renderer/src/composables/useAppSchema.ts (1)
useAppSchema(9-164)packages/runtime-renderer/src/utils/data-utils.ts (1)
parseJSFunction(15-23)packages/runtime-renderer/src/app-function/http/axios.js (1)
config(16-143)packages/register/src/hooks.ts (1)
useHttp(89-89)
packages/layout/src/defaultLayout.js (1)
packages/register/src/constants.ts (1)
META_APP(24-81)
packages/runtime-renderer/src/app-function/http/index.js (2)
packages/runtime-renderer/src/app-function/http/axios.js (1)
config(16-143)packages/runtime-renderer/src/app-function/dataSource.ts (1)
response(110-112)
packages/runtime-renderer/src/renderer/RenderMain.ts (7)
packages/runtime-renderer/src/composables/useAppSchema.ts (1)
useAppSchema(9-164)packages/canvas/render/src/page-block-function/context.ts (1)
useContext(15-31)packages/plugins/materials/src/composable/types.ts (1)
Schema(305-317)packages/runtime-renderer/src/renderer/page-function/state.ts (1)
useState(6-46)packages/runtime-renderer/src/renderer/parser/parser.ts (1)
parseData(228-231)packages/runtime-renderer/src/app-function/utils.ts (1)
getUtilsAll(68-70)packages/runtime-renderer/src/renderer/page-function/css-handler.ts (1)
setPageCss(72-82)
packages/runtime-renderer/src/renderer/render.ts (4)
packages/runtime-renderer/src/renderer/page-function/blockContext.ts (1)
getBlockContext(72-76)packages/canvas/render/src/material-function/configure.ts (1)
configure(1-1)packages/canvas/render/src/render.ts (1)
renderDefault(27-35)packages/runtime-renderer/src/renderer/parser/parser.ts (3)
parseData(228-231)parseLoopArgs(238-245)parseCondition(233-236)
packages/runtime-renderer/src/composables/useAppSchema.ts (3)
scripts/buildMaterials.mjs (1)
appSchema(18-18)packages/runtime-renderer/src/types/schema.ts (4)
AppSchema(2-22)Util(184-188)I18nConfig(232-235)BlockItem(248-287)packages/runtime-renderer/src/app-function/utils.ts (1)
initUtils(45-66)
packages/runtime-renderer/src/app-function/utils.ts (2)
packages/runtime-renderer/src/types/schema.ts (1)
Util(184-188)packages/runtime-renderer/src/utils/data-utils.ts (1)
parseJSFunction(15-23)
designer-demo/src/runtime.js (1)
packages/runtime-renderer/index.ts (1)
initRuntimeRenderer(22-39)
packages/common/js/runtime-deploy.js (1)
packages/register/src/common.ts (1)
getMergeMeta(53-55)
packages/runtime-renderer/src/renderer/LifecycleWrapper.ts (1)
packages/runtime-renderer/src/types/schema.ts (1)
JSFunction(129-132)
packages/runtime-renderer/src/router/index.ts (2)
packages/runtime-renderer/src/composables/useAppSchema.ts (1)
useAppSchema(9-164)packages/runtime-renderer/src/types/config.ts (1)
RouteConfig(7-23)
packages/runtime-renderer/index.ts (3)
packages/runtime-renderer/src/composables/useAppSchema.ts (1)
useAppSchema(9-164)packages/runtime-renderer/src/router/index.ts (1)
createAppRouter(78-91)packages/runtime-renderer/src/stores/index.ts (2)
generateStoresConfig(7-26)createStores(28-45)
packages/runtime-renderer/src/renderer/parser/parser.ts (4)
packages/vue-generator/src/utils/index.js (1)
fnRegexp(20-20)packages/runtime-renderer/src/utils/data-utils.ts (1)
parseJSFunction(15-23)packages/canvas/render/src/material-function/material-getter.ts (1)
getIcon(106-106)packages/vue-generator/src/generator/vue/sfc/generateAttribute.js (3)
propsEntries(641-641)modelValue(642-642)hasUpdateModelValue(643-643)
🔇 Additional comments (18)
packages/build/vite-config/src/vite-plugins/devAliasPlugin.js (1)
44-44: LGTM!The new alias mappings for
@opentiny/tiny-engine-runtime-rendererand@opentiny/tiny-engine-toolbar-runtime-deployfollow the existing pattern and are correctly placed.Also applies to: 50-50
packages/runtime-renderer/src/types/schema.ts (1)
1-352: LGTM! Comprehensive and well-structured type definitions.The type definitions provide a strong-typed foundation for the runtime renderer, covering all essential aspects: application schema, page composition, component nodes, data sources, lifecycle hooks, blocks, and internationalization. The interfaces are consistent, well-organized, and support the broader runtime rendering features introduced in this PR.
designer-demo/package.json (1)
16-16: LGTM! Standard workspace dependency.The addition of the runtime-renderer workspace dependency correctly integrates the new package into the designer-demo application.
packages/runtime-renderer/src/renderer/parser/index.ts (1)
1-1: LGTM! Standard barrel export.The barrel export consolidates the parser module's public API, following established patterns in the runtime-renderer package.
packages/layout/src/defaultLayout.js (1)
29-29: LGTM! Toolbar integration is well-positioned.The RuntimeDeploy toolbar is logically placed after Preview and before GenerateCode/Save, aligning with the runtime rendering workflow introduced in this PR.
packages/runtime-renderer/src/App.vue (1)
1-3: LGTM! Standard Vue Router root component.The router-view with a dynamic key bound to
$route.fullPathensures proper component remounting on route changes, which is essential for the runtime renderer's page navigation.packages/design-core/package.json (1)
81-81: LGTM! Workspace dependencies correctly added.The addition of runtime-deploy toolbar and runtime-renderer workspace dependencies properly integrates the new packages into the design-core ecosystem.
Also applies to: 88-88
designer-demo/runtime.html (1)
1-13: LGTM! Standard HTML entry point.The HTML skeleton provides a clean entry point for the runtime renderer with proper meta tags and module script reference. The structure aligns with the new runtime bootstrap flow introduced in this PR.
packages/design-core/re-export.js (1)
14-14: LGTM!The RuntimeDeploy re-export follows the established pattern and integrates cleanly with the existing toolbar exports.
packages/runtime-renderer/src/renderer/page-function/index.ts (1)
13-13: LGTM!The barrel export follows the standard pattern for exposing module functionality.
packages/register/src/constants.ts (1)
40-40: LGTM!The RuntimeDeploy constant addition is consistent with the existing META_APP naming convention and structure.
packages/design-core/registry.js (1)
28-28: LGTM!The RuntimeDeploy toolbar integration follows the established pattern with proper registry guard and logical placement in the toolbar sequence (after Preview, before Refresh).
Also applies to: 144-144
tsconfig.app.json (1)
21-22: LGTM!The path alias additions follow the established pattern and are necessary for proper TypeScript module resolution in the workspace.
Also applies to: 49-49
packages/toolbars/runtime-deploy/index.ts (1)
13-19: LGTM!The toolbar entry point correctly merges metadata with the Vue component, following the standard TinyEngine toolbar plugin pattern.
packages/runtime-renderer/src/renderer/builtin/CanvasCollection.vue (1)
20-24: Verify if unused props are intentional.The
schemaanddataSourceprops are declared but not used in the template. If these are reserved for future functionality or consumed by parent components via prop drilling, consider adding a brief comment explaining their purpose.packages/runtime-renderer/src/components/PageRenderer.vue (1)
1-9: LGTM!The component correctly uses
pageIdas both a prop and a key to ensure proper remounting when the page changes, avoiding nested component schema issues as noted in the comment.packages/runtime-renderer/src/renderer/builtin/CanvasPlaceholder.vue (1)
14-17: Verify internationalization for default placeholder text.The default placeholder text is in Chinese ('请将元素拖放到这里'). Ensure this aligns with the project's i18n strategy, or consider using an i18n key if multilingual support is required.
packages/runtime-renderer/src/renderer/builtin/index.ts (1)
13-33: LGTM!The barrel export consolidates all built-in Canvas components into a single public API surface, following a clean and standard pattern.
packages/runtime-renderer/src/renderer/page-function/css-handler.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (4)
packages/common/js/runtime-deploy.js (3)
17-25: Address the query parameter handling issue.This concern was previously raised: the function concatenates parameters without validating that
appIdandplatformexist, potentially producing malformed URLs likeid=null&platform=undefined.The recommended fix from the previous review remains valid:
const getQueryParams = () => { const paramsMap = new URLSearchParams(location.search) const tenant = paramsMap.get('tenant') || '' const platform = getMergeMeta('engine.config')?.platformId const appId = paramsMap.get('id') - let query = `id=${appId}&tenant=${tenant}&platform=${platform}` + const params = new URLSearchParams() + if (appId) params.set('id', appId) + if (tenant) params.set('tenant', tenant) + if (platform) params.set('platform', platform) + + return params.toString() - return query }
27-42: Custom deploy URL validation remains unaddressed.A previous review flagged the need to validate
customDeployUrlfunction return values to prevent unexpected behavior and potential open redirect vulnerabilities. The current implementation:
- Does not validate that the custom function returns a safe URL
- Cannot support async custom deploy functions (the
asynckeyword was removed)If async custom functions are not needed, the current signature is acceptable. Otherwise, restore the
asynckeyword andawaitthe custom function call.Apply this diff to add validation:
export const deployPage = () => { const href = window.location.href.split('?')[0] || './' const query = getQueryParams() const customDeployUrl = getMergeMeta('engine.toolbars.runtimeDeploy')?.options?.deployUrl const defaultDeployUrl = isDevelopEnv ? `./runtime.html` : `${href.endsWith('/') ? href : `${href}/`}runtime` let openUrl = '' openUrl = customDeployUrl ? typeof customDeployUrl === 'function' - ? customDeployUrl(defaultDeployUrl, query) + ? String(customDeployUrl(defaultDeployUrl, query) || defaultDeployUrl) : `${customDeployUrl}?${query}` : `${defaultDeployUrl}?${query}` return { openUrl } }For async support, restore the
asynckeyword and useawait:-export const deployPage = () => { +export const deployPage = async () => { // ... openUrl = customDeployUrl ? typeof customDeployUrl === 'function' - ? customDeployUrl(defaultDeployUrl, query) + ? String((await customDeployUrl(defaultDeployUrl, query)) || defaultDeployUrl) : `${customDeployUrl}?${query}` : `${defaultDeployUrl}?${query}` // ... }
57-57: Handle popup blocker scenario.A previous review flagged that
window.opencan returnnullif blocked by the browser's popup blocker. The current code does not handle this case, which will cause subsequent calls to always attempt to open a new window.Apply this diff:
runtimeWindow = window.open(openUrl, 'tiny-engine-runtime') + + if (!runtimeWindow) { + // eslint-disable-next-line no-console + console.error('[runtime-deploy] Failed to open runtime window. Popup may be blocked.') + // Optionally notify the user via UI + } }packages/runtime-renderer/index.ts (1)
39-42: Provide stores before mounting.
app.provide('stores', stores)runs aftermount, so inject consumers created during the initial render seeundefined, breaking runtime state injection. Move theprovidecall beforemount.- const app = createApp(App) - app.use(pinia).use(router).use(i18n).mount('#app') - - app.provide('stores', stores) + const app = createApp(App) + app.provide('stores', stores) + app.use(pinia).use(router).use(i18n).mount('#app')
🧹 Nitpick comments (1)
packages/common/js/runtime-deploy.js (1)
44-45: Remove unnecessaryawaiton synchronous function.The
awaiton line 45 is unnecessary becausedeployPage()is not anasyncfunction (line 27). While this doesn't break functionality, it adds unnecessary overhead.Apply this diff:
export const runtimeDeploy = async () => { - const { openUrl } = await deployPage() + const { openUrl } = deployPage() if (runtimeWindow && !runtimeWindow.closed) {If you restore
asynctodeployPage(to support async custom deploy functions), then theawaitshould remain.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (6)
designer-demo/src/runtime.js(1 hunks)packages/common/js/runtime-deploy.js(1 hunks)packages/runtime-renderer/index.ts(1 hunks)packages/runtime-renderer/src/app-function/utils.ts(1 hunks)packages/runtime-renderer/src/composables/useAppSchema.ts(1 hunks)packages/runtime-renderer/src/renderer/page-function/blockContext.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- packages/runtime-renderer/src/composables/useAppSchema.ts
- packages/runtime-renderer/src/renderer/page-function/blockContext.ts
- designer-demo/src/runtime.js
🧰 Additional context used
🧠 Learnings (1)
📚 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/runtime-renderer/index.ts
🧬 Code graph analysis (3)
packages/runtime-renderer/src/app-function/utils.ts (2)
packages/runtime-renderer/src/types/schema.ts (1)
Util(184-188)packages/runtime-renderer/src/utils/data-utils.ts (1)
parseJSFunction(15-23)
packages/runtime-renderer/index.ts (3)
packages/runtime-renderer/src/composables/useAppSchema.ts (1)
useAppSchema(9-189)packages/runtime-renderer/src/router/index.ts (1)
createAppRouter(78-91)packages/runtime-renderer/src/stores/index.ts (2)
generateStoresConfig(7-26)createStores(28-45)
packages/common/js/runtime-deploy.js (1)
packages/register/src/common.ts (1)
getMergeMeta(53-55)
🔇 Additional comments (1)
packages/common/js/runtime-deploy.js (1)
13-14: LGTM: Clean imports.The imports are appropriate for the runtime deployment functionality.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (1)
packages/runtime-renderer/src/router/index.ts (1)
7-17: Validate that page.route exists; consider using page.meta.router as fallback.The
PageSchemainterface assumes aroutefield, but looking at the actualPageMetatype fromschema.ts(line 82), the field is namedrouter, notroute. If the schema providespage.meta.routerinstead ofpage.route, line 32 would create routes withpath: 'undefined'.Add validation and fallback logic:
const generateRoutesByPages = (pages: Array<PageSchema>): Array<IRouteConfig> => { // 建立路由-页面id映射 const pageRouteMap = new Map<string, IRouteConfig>() pages.forEach((page: PageSchema) => { + const routeSegment = page.route || page.meta?.router + if (!routeSegment) { + console.error(`页面缺少路由字段: ${page.name} (ID: ${page.id})`) + return // Skip this page + } const pageIdStr = String(page.id) // JAVA后端中page.id字段为number类型，前端mockServer中为string类型 // 为了同时兼容JAVA后端和mockserver,显示转换作为键的pageId为字符串 pageRouteMap.set(pageIdStr, { - path: `${page.route}`, + path: routeSegment,
🧹 Nitpick comments (2)
packages/runtime-renderer/src/router/index.ts (2)
31-48: Ensure props.pageId uses the stringified ID for consistency.Line 35 passes
pageId: page.id, which could be a number or string depending on the backend. Line 28 explicitly convertspage.idto a string for consistency. Theprops.pageIdshould also use the stringified version to maintain type consistency throughout the router.pageRouteMap.set(pageIdStr, { path: routeSegment, name: pageIdStr, component: () => import('../components/PageRenderer.ts'), - props: { pageId: page.id }, + props: { pageId: pageIdStr }, children: [], meta: {
66-70: Normalize root route paths to prevent double slashes.Line 68 adds a leading slash to root routes with
config.path = `/${config.path}`. Ifconfig.pathalready contains a leading slash (e.g., from malformed schema data), this would create//path.Apply path normalization:
} else { // 无父级路由均为根路由 - config.path = `/${config.path}` + config.path = `/${config.path}`.replace(/^\/+/, '/') routeConfs.push(config) }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/common/js/runtime-deploy.js(1 hunks)packages/runtime-renderer/src/router/index.ts(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/common/js/runtime-deploy.js
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-14T09:24:11.999Z
Learnt from: lichunn
PR: opentiny/tiny-engine#1011
File: packages/vue-generator/src/plugins/genRouterPlugin.js:19-19
Timestamp: 2025-01-14T09:24:11.999Z
Learning: In Vue router path construction, when recursively flattening nested routes, starting with empty parentPath for child routes is correct as the parent's path is already captured in the parent route object. This avoids duplicate path segments.
Applied to files:
packages/runtime-renderer/src/router/index.ts
🧬 Code graph analysis (1)
packages/runtime-renderer/src/router/index.ts (3)
packages/runtime-renderer/src/types/schema.ts (1)
PageMeta(71-92)packages/runtime-renderer/src/composables/useAppSchema.ts (1)
useAppSchema(27-276)packages/runtime-renderer/src/types/config.ts (1)
IRouteConfig(6-6)
🔇 Additional comments (2)
packages/runtime-renderer/src/router/index.ts (2)
86-92: LGTM on 404 catch-all route.The catch-all route for unmatched paths is correctly implemented using the Vue Router 4 syntax
/:pathMatch(.*)*. This ensures that any undefined routes navigate to the NotFound component.
94-101: Router initialization looks correct.The
createAppRouterfunction properly awaits the route configuration, creates a router with hash history, and returns the configured router instance. The base path/runtime.htmlaligns with the runtime rendering requirements mentioned in the PR objectives.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (2)
packages/runtime-renderer/src/components/PageRenderer.ts (1)
4-21: Consider typing WrappedComponent more strictly.The HOC pattern is implemented correctly and works as expected. For improved type safety, you could type
WrappedComponentmore precisely instead ofany.For example:
+import type { Component } from 'vue' + -export function withPageRenderer(WrappedComponent: any) { +export function withPageRenderer(WrappedComponent: Component) { return defineComponent({packages/runtime-renderer/src/renderer/RenderMain.ts (1)
80-80: Unnecessary String() conversion.
props.pageIdis already typed asstring, so theString()wrapper is redundant.Apply this diff:
- const cssScopeId = `data-te-page-${String(props.pageId) || 'render-main'}` + const cssScopeId = `data-te-page-${props.pageId || 'render-main'}`
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/runtime-renderer/src/components/PageRenderer.ts(1 hunks)packages/runtime-renderer/src/renderer/RenderMain.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/runtime-renderer/src/renderer/RenderMain.ts (8)
packages/runtime-renderer/src/composables/useAppSchema.ts (1)
useAppSchema(27-276)packages/canvas/render/src/page-block-function/context.ts (1)
useContext(15-31)packages/runtime-renderer/src/renderer/page-function/state.ts (1)
useState(6-46)packages/runtime-renderer/src/renderer/parser/parser.ts (1)
parseData(228-231)packages/runtime-renderer/src/app-function/dataSource.ts (1)
getDataSource(109-109)packages/runtime-renderer/src/app-function/utils.ts (1)
getUtilsAll(83-85)packages/runtime-renderer/src/renderer/page-function/css.ts (1)
setPageCss(117-127)packages/runtime-renderer/src/renderer/render.ts (1)
renderer(417-510)
🔇 Additional comments (1)
packages/runtime-renderer/src/renderer/RenderMain.ts (1)
36-113: Well-structured component with proper guards.The implementation correctly addresses the critical issues flagged in previous reviews:
- Schema null guards prevent crashes before data loads (lines 41-43)
- Watch properly validates schema before processing (lines 107-109)
- Props type consistency is maintained (pageId as string)
The context management, state initialization, and schema watching flow are all correctly implemented.
…of the runtime-renderer package on @opentiny/tiny-engine-utils
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/runtime-renderer/src/utils/data-utils.ts (1)
22-31: Duplicate: Security issue previously flagged.This function has the same code injection vulnerability that was already flagged in the previous review comment. The concerns about using the Function constructor with untrusted
data.valueinput, lack of sanitization, and console-only error logging remain valid.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/runtime-renderer/src/utils/data-utils.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (2)
📚 Learning: 2025-01-14T06:58:38.661Z
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/data-utils.ts:8-12
Timestamp: 2025-01-14T06:58:38.661Z
Learning: The use of Function constructor in `packages/canvas/render/src/data-utils.ts` is part of migrated code and was intentionally kept as-is during migration, despite potential security concerns.
Applied to files:
packages/runtime-renderer/src/utils/data-utils.ts
📚 Learning: 2025-01-14T06:50:21.158Z
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/data-function/parser.ts:191-195
Timestamp: 2025-01-14T06:50:21.158Z
Learning: The `newFn` function in `packages/canvas/render/src/data-function/parser.ts` has a known security issue with evaluating untrusted code through `data.value`. This was identified during code review but intentionally deferred as the original implementation was kept during code refactoring.
Applied to files:
packages/runtime-renderer/src/utils/data-utils.ts
🔇 Additional comments (3)
packages/runtime-renderer/src/utils/data-utils.ts (3)
12-14: LGTM!Simple utility for clearing object properties in-place. Implementation is correct.
16-20: Note: Helper for dynamic function creation.This wrapper around the Function constructor is used by
parseJSFunctionand other dynamic code execution utilities. While it bypasses eslint warnings, it doesn't address the underlying security concerns of executing untrusted code.
33-38: LGTM!Efficient implementation using Set for O(1) lookup when comparing object keys. Logic correctly identifies keys present in
objAbut not inobjB.
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
packages/runtime-renderer/src/stores/index.ts (1)
7-38: Good: Error handling added; however, JSON.parse(JSON.stringify()) remains problematic.The error handling improvements for
parseJSFunction(lines 16-22, 28-34) are excellent—they properly catch failures and provide safe fallbacks.However, line 12 still uses
JSON.parse(JSON.stringify(store.state))for deep cloning, which will:
- Throw on circular references
- Drop functions and
undefinedvalues- Convert
Dateobjects to strings- Lose
Map,Set,RegExp, and other special objectsSince
store.statecomes from the application schema and may contain complex objects, this can cause data corruption or runtime errors.Consider using a proper deep-clone utility:
+import { cloneDeep } from 'lodash-es' // or use an existing project utility + export const generateStoresConfig = () => { const { globalStates } = useAppSchema() if (globalStates.value.length === 0) return [] return globalStates.value.map((store) => ({ id: store.id, - state: JSON.parse(JSON.stringify(store.state)), + state: cloneDeep(store.state),Optional: Translate Chinese comments to English (lines 15, 27) for international maintainability.
🧹 Nitpick comments (1)
packages/runtime-renderer/src/stores/index.ts (1)
40-57: LGTM! Optional: Translate Chinese comments.The implementation correctly creates Pinia stores from the config array and binds them to the provided Pinia instance. Using
shallowReactivefor the stores map is appropriate here.Optional: Translate Chinese comments (lines 44, 52) to English for consistency and maintainability.
- // 使用 defineStore 创建 Pinia store + // Create Pinia store using defineStore const useStore = defineStore(config.id, {- // 使用useStore创建 store 实例并绑定到 pinia + // Create store instance and bind to pinia stores[config.id] = useStore(pinia)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/runtime-renderer/src/stores/index.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/runtime-renderer/src/stores/index.ts (3)
packages/runtime-renderer/src/composables/useAppSchema.ts (1)
useAppSchema(27-276)packages/runtime-renderer/src/utils/data-utils.ts (1)
parseJSFunction(23-31)packages/runtime-renderer/src/types/config.ts (1)
StoreConfig(8-13)
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
packages/runtime-renderer/src/app-function/dataSource.ts (1)
61-110: Consider tracking HTTP instances for explicit cleanup.Each call to
initDataSourcecreates new HTTP instances with interceptors (Line 73), but previous instances are not explicitly cleaned up. While JavaScript garbage collection will eventually reclaim them, explicitly ejecting interceptors when rebuilding the data source map would be cleaner and prevent potential memory accumulation in long-running applications with frequent re-initializations.Consider this approach:
+// Track interceptor IDs for cleanup +const httpCleanupHandlers: Array<{ request: number; response: number }> = [] + export const initDataSource = (config: any) => { + // Clean up old interceptors + httpCleanupHandlers.forEach(({ http, request, response }) => { + http.interceptors.request.eject(request) + http.interceptors.response.eject(response) + }) + httpCleanupHandlers.length = 0 + Object.keys(dataSourceMap).forEach((key) => delete dataSourceMap[key]) // ... existing normalization ... normalized.forEach((item) => { const http = useHttp(globalDataHandle) // ... existing setup ... - http.interceptors.request.use(willFetch, errorHandler) - http.interceptors.response.use(dataHandler, errorHandler) + const reqId = http.interceptors.request.use(willFetch, errorHandler) + const resId = http.interceptors.response.use(dataHandler, errorHandler) + httpCleanupHandlers.push({ http, request: reqId, response: resId })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/runtime-renderer/src/app-function/dataSource.ts(1 hunks)
🧰 Additional context used
🧬 Code graph analysis (1)
packages/runtime-renderer/src/app-function/dataSource.ts (4)
packages/runtime-renderer/src/app-function/http/index.js (2)
http(17-17)dataHandler(16-27)packages/runtime-renderer/src/app-function/http/axios.js (1)
config(16-143)packages/runtime-renderer/src/utils/data-utils.ts (1)
parseJSFunction(23-31)packages/register/src/hooks.ts (1)
useHttp(89-89)
🔇 Additional comments (2)
packages/runtime-renderer/src/app-function/dataSource.ts (2)
4-17: LGTM!The normalization logic correctly flattens the nested data source configuration for easier consumption.
112-114: LGTM!The exported API provides both a getter function and direct access to the data source map, offering flexibility for consumers.
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/runtime-renderer/src/app-function/dataSource.ts(1 hunks)packages/runtime-renderer/src/app-function/http/index.js(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/runtime-renderer/src/app-function/http/index.js
🧰 Additional context used
🧬 Code graph analysis (1)
packages/runtime-renderer/src/app-function/dataSource.ts (3)
packages/runtime-renderer/src/app-function/http/axios.js (1)
config(16-143)packages/runtime-renderer/src/utils/data-utils.ts (1)
parseJSFunction(23-31)packages/register/src/hooks.ts (1)
useHttp(89-89)
🔇 Additional comments (1)
packages/runtime-renderer/src/app-function/dataSource.ts (1)
64-132: Well-structured initialization with proper function parsing.The
initDataSourceimplementation correctly addresses the previous review feedback about parsing handlers once (Lines 84-87) rather than on every invocation. The interceptor setup, data source construction, and mapping logic are all sound.However, ensure the static data path inconsistency flagged in the
loadfunction is addressed to maintain consistent behavior across static and remote data sources.
There was a problem hiding this comment.
Actionable comments posted: 2
♻️ Duplicate comments (2)
packages/runtime-renderer/src/renderer/render.ts (1)
1-514: All critical issues already identified in previous reviews.The previous review comments comprehensively cover the main issues in this file:
- Module-level
pageScopeIdstate (lines 82-90, 132, 432): Race conditions with concurrent page rendering- Module-level
collectionMethodsMap(line 72, 188): Cross-page data leakage- Loop rendering logic (lines 335, 396): Incorrect handling of empty arrays
No additional critical issues were identified beyond those already flagged.
packages/runtime-renderer/src/renderer/parser/parser.ts (1)
1-360: Most critical issues already identified in previous reviews.The previous review comments cover most of the major issues in this file:
newFnduplication (lines 70-74): Duplicated from data-utils.ts- Critical security with
withstatement (lines 95-116): Unsafe code execution- Duplicate regex pattern (lines 130-145): Duplicated from vue-generator
- Security with unvalidated function body (lines 148-167): Executes untrusted code
- Hardcoded default return value (lines 169-210): Table-specific fallback
- Incomplete JSSlot with circular dependency (lines 221-223): Also has a new undefined variable bug flagged separately
One new critical bug was identified and flagged separately (undefined variables in parseJSSlot).
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/runtime-renderer/src/app-function/dataSource.ts(1 hunks)packages/runtime-renderer/src/renderer/parser/parser.ts(1 hunks)packages/runtime-renderer/src/renderer/render.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-14T06:50:21.158Z
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/data-function/parser.ts:191-195
Timestamp: 2025-01-14T06:50:21.158Z
Learning: The `newFn` function in `packages/canvas/render/src/data-function/parser.ts` has a known security issue with evaluating untrusted code through `data.value`. This was identified during code review but intentionally deferred as the original implementation was kept during code refactoring.
Applied to files:
packages/runtime-renderer/src/renderer/parser/parser.ts
🧬 Code graph analysis (3)
packages/runtime-renderer/src/app-function/dataSource.ts (4)
packages/runtime-renderer/src/app-function/http/index.js (2)
http(17-17)dataHandler(16-28)packages/runtime-renderer/src/app-function/http/axios.js (1)
config(16-143)packages/runtime-renderer/src/utils/data-utils.ts (1)
parseJSFunction(23-31)packages/register/src/hooks.ts (1)
useHttp(89-89)
packages/runtime-renderer/src/renderer/render.ts (5)
packages/runtime-renderer/src/renderer/page-function/blockContext.ts (2)
getBlockContext(88-92)getBlockCssScopeId(25-32)packages/canvas/render/src/material-function/configure.ts (1)
configure(1-1)packages/canvas/types.ts (1)
Node(1-12)packages/runtime-renderer/src/renderer/parser/parser.ts (3)
parseData(225-228)parseLoopArgs(235-242)parseCondition(230-233)packages/runtime-renderer/src/types/schema.ts (1)
JSFunction(123-126)
packages/runtime-renderer/src/renderer/parser/parser.ts (5)
packages/vue-generator/src/utils/index.js (1)
fnRegexp(20-20)packages/runtime-renderer/src/utils/data-utils.ts (1)
parseJSFunction(23-31)packages/runtime-renderer/src/renderer/render.ts (1)
renderDefault(194-202)packages/canvas/render/src/material-function/material-getter.ts (1)
getIcon(106-106)packages/vue-generator/src/generator/vue/sfc/generateAttribute.js (3)
propsEntries(641-641)modelValue(642-642)hasUpdateModelValue(643-643)
🔇 Additional comments (4)
packages/runtime-renderer/src/app-function/dataSource.ts (4)
1-17: LGTM! Clean imports and normalization logic.The
normalizeItemfunction correctly flattens the nested configuration structure, making it easier to work with data source items throughout the module.
19-21: LGTM! Module-level state is appropriate.The module-level
dataSourceMapandglobalDataHandleprovide the necessary shared state for managing data sources across the application.
24-62: LGTM! Load function handles both static and remote data correctly.The static data path (lines 26-42) properly applies transformations in order:
globalDataHandle→parsedDataHandler→ wrap to standard format. The remote data path (lines 44-62) correctly builds the request config and delegates to HTTP interceptors.One minor consideration: Line 45 returns
Promise.resolve(undefined)whenshouldFetch()returns false. Verify this is the intended behavior rather than returning the existingdataSource.data.
134-136: LGTM! Exports provide appropriate access patterns.Exporting both
getDataSource()and the defaultdataSourceMapallows consumers to choose between functional or direct access to the data source registry.
| const dataHandler = (res) => { | ||
| // 先应用用户定义的dataHandler | ||
| const handled = parsedDataHandler ? parsedDataHandler(res) : res | ||
|
|
||
| // 统一数据结构，确保与远程数据源与静态数据源结构一致 | ||
| let unifiedData | ||
| if (handled && typeof handled === 'object' && !handled.data?.items) { | ||
| // 如果不是静态数据源的格式，则转换为统一格式 | ||
| const items = Array.isArray(handled.data) ? handled.data : handled.data ? [handled.data] : [] | ||
| unifiedData = { | ||
| code: handled.code || '', | ||
| msg: handled.message || handled.msg || 'success', | ||
| data: { | ||
| items, | ||
| total: items.length | ||
| } | ||
| } | ||
| } else { | ||
| unifiedData = handled | ||
| } | ||
|
|
||
| dataSource.status = 'loaded' | ||
| dataSource.data = unifiedData | ||
| return unifiedData | ||
| } |
There was a problem hiding this comment.
Critical: Fix wrapping logic to handle raw data correctly.
The conditional wrapping logic at lines 95-108 fails when parsedDataHandler returns raw data (array or plain object) instead of a wrapped response.
Issue: When handled is a raw array like [1, 2, 3]:
- Line 95 condition is true (handled is an object and
handled.data?.itemsis undefined) - Line 97 evaluates to
items = []becausehandled.datais undefined - This loses all the data!
The logic needs to distinguish three cases:
- Already fully wrapped:
{code, msg, data: {items, total}} - Partially wrapped:
{code, msg, data}wheredatais array/object - Raw data: array or plain object without wrapper
Apply this diff to fix the wrapping logic:
const dataHandler = (res) => {
// 先应用用户定义的dataHandler
const handled = parsedDataHandler ? parsedDataHandler(res) : res
// 统一数据结构,确保与远程数据源与静态数据源结构一致
let unifiedData
- if (handled && typeof handled === 'object' && !handled.data?.items) {
- // 如果不是静态数据源的格式,则转换为统一格式
+
+ // Case 1: Already in standard format with data.items
+ if (handled && typeof handled === 'object' && handled.data?.items !== undefined) {
+ unifiedData = handled
+ }
+ // Case 2: Wrapped response with code/msg/data but data is not in {items, total} format
+ else if (handled && typeof handled === 'object' && 'code' in handled && 'data' in handled) {
const items = Array.isArray(handled.data) ? handled.data : handled.data ? [handled.data] : []
unifiedData = {
code: handled.code || '',
msg: handled.message || handled.msg || 'success',
data: {
items,
total: items.length
}
}
- } else {
- unifiedData = handled
+ }
+ // Case 3: Raw data (array or plain object)
+ else {
+ const items = Array.isArray(handled) ? handled : handled ? [handled] : []
+ unifiedData = {
+ code: '',
+ msg: 'success',
+ data: {
+ items,
+ total: items.length
+ }
+ }
}
dataSource.status = 'loaded'
dataSource.data = unifiedData
return unifiedData
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| const dataHandler = (res) => { | |
| // 先应用用户定义的dataHandler | |
| const handled = parsedDataHandler ? parsedDataHandler(res) : res | |
| // 统一数据结构，确保与远程数据源与静态数据源结构一致 | |
| let unifiedData | |
| if (handled && typeof handled === 'object' && !handled.data?.items) { | |
| // 如果不是静态数据源的格式，则转换为统一格式 | |
| const items = Array.isArray(handled.data) ? handled.data : handled.data ? [handled.data] : [] | |
| unifiedData = { | |
| code: handled.code || '', | |
| msg: handled.message || handled.msg || 'success', | |
| data: { | |
| items, | |
| total: items.length | |
| } | |
| } | |
| } else { | |
| unifiedData = handled | |
| } | |
| dataSource.status = 'loaded' | |
| dataSource.data = unifiedData | |
| return unifiedData | |
| } | |
| const dataHandler = (res) => { | |
| // 先应用用户定义的dataHandler | |
| const handled = parsedDataHandler ? parsedDataHandler(res) : res | |
| // 统一数据结构,确保与远程数据源与静态数据源结构一致 | |
| let unifiedData | |
| // Case 1: Already in standard format with data.items | |
| if (handled && typeof handled === 'object' && handled.data?.items !== undefined) { | |
| unifiedData = handled | |
| } | |
| // Case 2: Wrapped response with code/msg/data but data is not in {items, total} format | |
| else if (handled && typeof handled === 'object' && 'code' in handled && 'data' in handled) { | |
| const items = Array.isArray(handled.data) ? handled.data : handled.data ? [handled.data] : [] | |
| unifiedData = { | |
| code: handled.code || '', | |
| msg: handled.message || handled.msg || 'success', | |
| data: { | |
| items, | |
| total: items.length | |
| } | |
| } | |
| } | |
| // Case 3: Raw data (array or plain object) | |
| else { | |
| const items = Array.isArray(handled) ? handled : handled ? [handled] : [] | |
| unifiedData = { | |
| code: '', | |
| msg: 'success', | |
| data: { | |
| items, | |
| total: items.length | |
| } | |
| } | |
| } | |
| dataSource.status = 'loaded' | |
| dataSource.data = unifiedData | |
| return unifiedData | |
| } |
🤖 Prompt for AI Agents
In packages/runtime-renderer/src/app-function/dataSource.ts around lines 89 to
113, the wrapping logic incorrectly drops raw arrays or plain objects returned
by parsedDataHandler; change the branch so it distinguishes three cases: (1)
already fully wrapped (has data?.items) — leave as-is; (2) partially wrapped
(has data but data is an array or an object that should become items) — if
handled.data is an array use that as items and set total; if handled.data is a
plain object wrap it into items = [handled.data]; (3) raw data (handled itself
is an array or plain object) — if Array.isArray(handled) set items = handled, if
plain object set items = [handled]; then build unifiedData using handled.code ||
'' and handled.message || handled.msg || 'success' with data: { items, total:
items.length }; keep setting dataSource.status = 'loaded' and dataSource.data =
unifiedData and return unifiedData.
…oid undefined variable references in parseJSSlot
There was a problem hiding this comment.
Actionable comments posted: 4
♻️ Duplicate comments (9)
packages/runtime-renderer/src/renderer/render.ts (4)
72-72: Module-levelcollectionMethodsMapstill causes cross-instance data leakage.As flagged in previous reviews, this exported object is mutated at line 188 during schema processing. Since all RenderMain instances share this module-level state, collection data from one page can leak into another page's rendering context.
The recommended fix remains: move
collectionMethodsMapto per-instance context (stored in the context object created viauseContext()in RenderMain) rather than module-level state.
82-90: Module-levelpageScopeIdstill causes race conditions with concurrent rendering.As detailed in previous reviews, each RenderMain instance calls
setPageScopeId()at line 429 during setup, but all instances share this module-level variable. When multiple pages render concurrently (nested routes, page transitions), one instance's write can overwrite another's before the value is read at line 132, causing the wrong CSS scope ID to be applied.The recommended fix remains: pass
pageScopeIdthrough the scope/context object instead of module-level state. The context is already available at line 127-130 whererenderGroupis called, so the scope ID should be included in that scope object and propagated through the render chain.
302-334: Loop rendering still mishandles empty arrays.As flagged in previous reviews, line 332's condition
loopList?.length ? loopList.map(renderElement) : renderElement(undefined, 0)treats empty arrays[]as falsy (sincelengthis0), incorrectly rendering one element with undefined data instead of rendering nothing.The recommended fix remains: use
Array.isArray(loopList)to distinguish empty arrays from null/undefined, ensuring empty arrays render nothing while null/undefined renders a single element without loop data.Apply this fix:
- return loopList?.length ? loopList.map(renderElement) : renderElement(undefined, 0) + if (Array.isArray(loopList)) { + return loopList.length > 0 ? loopList.map(renderElement) : null + } + return renderElement(undefined, 0)
356-394: Same empty array handling issue inrenderComponent.Line 393 has the identical loop rendering bug as line 332 in
renderGroup(flagged above). The same fix should be applied here to properly distinguish empty arrays from null/undefined.Apply this fix:
- return loopList?.length ? loopList.map(renderElement) : renderElement(undefined, 0) + if (Array.isArray(loopList)) { + return loopList.length > 0 ? loopList.map(renderElement) : null + } + return renderElement(undefined, 0)packages/runtime-renderer/src/renderer/parser/parser.ts (5)
70-74: De-dup newFn: import the shared util.This local newFn duplicates the one in src/utils/data-utils.ts. Remove it and import instead.
@@ -import { renderDefault } from '../render' +import { renderDefault } from '../render' +import { newFn } from '../../utils/data-utils' @@ -// 规避创建function eslint报错 -export const newFn = (...argv) => { - const Fn = Function - return new Fn(...argv) -}
129-145: Duplicate function-regex; extract to shared constant.Same pattern as vue-generator/utils/index.js. Centralize to avoid drift.
Proposed shared util:
export const FUNCTION_REGEXP = /(async)?.*?(\w+) *\(([\s\S]*?)\) *\{([\s\S]*)\}/ export const parseFunctionString = (fnStr: string) => { const m = FUNCTION_REGEXP.exec(fnStr) return m ? { type: m[1] || '', name: m[2], params: m[3].split(',').map(s=>s.trim()).filter(Boolean), body: m[4] } : null }Then import here and in vue-generator.
169-210: Default async fallback is table-shaped; make it configurable.Generalize generateFn to accept options.defaultReturn.
-export const generateFn = (innerFn, context) => { +export const generateFn = (innerFn, context, options: { defaultReturn?: any } = {}) => { + const { defaultReturn = { result: [{}], page: { total: 1 } } } = options return (...args) => { @@ - if (result?.then) { + if (result?.then) { result = new Promise((resolve) => { result.then(resolve).catch((error) => { Notify({ type: 'warning', title: '异步函数执行报错', message: error?.message || '异步函数执行报错，请检查语法' }) - // 这里需要至少返回一条空数据，方便用户使用表格默认插槽 - resolve({ - result: [{}], - page: { total: 1 } - }) + resolve(defaultReturn) }) }) }Call sites keep working; pass a different defaultReturn when needed.
95-116: Critical: Function + with + ctx mutation → RCE risk and deopts. Replace with restricted evaluator.Avoid with($scope) and stop mutating ctx; evaluate with explicit scope args.
const parseExpression = (data, scope, ctx, isJsx = false) => { try { - if (data.value.indexOf('this.i18n') > -1) { - ctx.i18n = i18nHost.global.t - } else if (data.value.indexOf('t(') > -1) { - ctx.t = i18nHost.global.t - } - const expression = isJsx ? transformJSX(data.value) : data.value - return newFn('$scope', `with($scope || {}) { return ${expression} }`).call(ctx, { - ...ctx, - ...scope, - slotScope: scope - }) + const evalScope = { + ...scope, + ...ctx, + slotScope: scope, + ...(data.value.includes('this.i18n') ? { i18n: i18nHost.global.t } : {}), + ...(data.value.includes('t(') ? { t: i18nHost.global.t } : {}) + } + const keys = Object.keys(evalScope) + const vals = keys.map((k) => evalScope[k]) + return newFn(...keys, `return (${expression})`).apply(ctx, vals) } catch (err) { if (!isJsx) { return parseExpression(data, scope, ctx, true) } return undefined } }If you prefer, move this into a dedicated safe-eval util with identifier allowlists; I can supply it. Based on learnings.
147-167: Validate JSX-derived function bodies before execution.Currently creates executable functions without checks; block unsafe constructs first.
const parseJSXFunction = (data, _scope, ctx) => { try { const newValue = transformJSX(data.value) const fnInfo = parseFunctionString(newValue) if (!fnInfo) throw Error('函数解析失败，请检查格式。示例：function fnName() { }') + // naive denylist; replace with robust validator/allowlist + const unsafe = /\b(Function|eval|import\s*\(|XMLHttpRequest|fetch\s*\(|localStorage|document|window|process|require|globalThis)\b/ + if (unsafe.test(fnInfo.body)) { + throw Error('函数体包含不安全代码') + } + return newFn(...fnInfo.params, fnInfo.body).bind({ ...ctx, getComponent: ctx.getComponent }) } catch (error) { @@ return newFn() } }
🧹 Nitpick comments (1)
packages/runtime-renderer/src/renderer/parser/parser.ts (1)
263-267: Unreachable Icon branch inside parseObjectData.Icon objects are already handled by isIcon → parseIcon before isObject. Remove to reduce confusion.
- // 解析通过属性传递icon图标组件 - if (data.componentName === 'Icon') { - return getIcon(data.props.name) - }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/runtime-renderer/src/renderer/parser/parser.ts(1 hunks)packages/runtime-renderer/src/renderer/render.ts(1 hunks)
🧰 Additional context used
🧠 Learnings (1)
📚 Learning: 2025-01-14T06:50:21.158Z
Learnt from: rhlin
PR: opentiny/tiny-engine#1011
File: packages/canvas/render/src/data-function/parser.ts:191-195
Timestamp: 2025-01-14T06:50:21.158Z
Learning: The `newFn` function in `packages/canvas/render/src/data-function/parser.ts` has a known security issue with evaluating untrusted code through `data.value`. This was identified during code review but intentionally deferred as the original implementation was kept during code refactoring.
Applied to files:
packages/runtime-renderer/src/renderer/parser/parser.ts
🧬 Code graph analysis (2)
packages/runtime-renderer/src/renderer/render.ts (3)
packages/runtime-renderer/src/renderer/page-function/blockContext.ts (2)
getBlockContext(88-92)getBlockCssScopeId(25-32)packages/runtime-renderer/src/renderer/parser/parser.ts (3)
parseData(224-227)parseLoopArgs(234-241)parseCondition(229-232)packages/runtime-renderer/src/types/schema.ts (1)
JSFunction(123-126)
packages/runtime-renderer/src/renderer/parser/parser.ts (5)
packages/vue-generator/src/utils/index.js (1)
fnRegexp(20-20)packages/runtime-renderer/src/utils/data-utils.ts (1)
parseJSFunction(23-31)packages/runtime-renderer/src/renderer/render.ts (1)
renderDefault(194-199)packages/canvas/render/src/material-function/material-getter.ts (1)
getIcon(106-106)packages/vue-generator/src/generator/vue/sfc/generateAttribute.js (3)
propsEntries(641-641)modelValue(642-642)hasUpdateModelValue(643-643)
🔇 Additional comments (4)
packages/runtime-renderer/src/renderer/parser/parser.ts (4)
312-359: Parser registry ordering looks correct.Good coverage and sensible precedence (arrays before objects, icons before objects, etc.).
13-18: Minor: import order and types are fine.No action needed.
278-295: Validate the v-model setter generation safeguards and data source.The concern about non-assignable LHS expressions in the generated setter is technically plausible. The code concatenates
modelValue[1].value(a string from JSExpression) directly into an assignment without validation. However, the risk cannot be definitively confirmed without understanding:
- Where the
model: trueflag originates (likely external design tool/config, not visible in this codebase)- Whether upstream validation or data sanitization exists before this parser is invoked
- Whether test coverage demonstrates this scenario is already guarded against
The codebase shows no assignability checks (Identifier/MemberExpression validation) in the parser. The suggested defensive check would prevent invalid expressions like
"123","foo()", etc., from generating runtime errors.
212-219: Gate this code path behind explicit trust verification or adopt the restricted evaluator pattern fromparseExpression.The
parseJSFunctionevaluates untrusted code fromdata.valuewithout scope restriction or syntax validation, unlikeparseExpression(which useswithscoping) andparseJSXFunction(which validates structure). Since internal code can generate JSFunction types (line 288–291 in the same file), user-influenced data may reach this evaluator without explicit trust checks.Recommend either:
- Add a trust flag requirement before this evaluation
- Wrap
data.valuein the samewith($scope || {})scoping pattern used inparseExpressionto restrict context accessPlease confirm schema.function sources are trusted-only at runtime, or apply one of these mitigations.
⛔ Skipped due to learnings
Learnt from: rhlin PR: opentiny/tiny-engine#1011 File: packages/canvas/render/src/data-function/parser.ts:191-195 Timestamp: 2025-01-14T06:50:21.158Z Learning: The `newFn` function in `packages/canvas/render/src/data-function/parser.ts` has a known security issue with evaluating untrusted code through `data.value`. This was identified during code review but intentionally deferred as the original implementation was kept during code refactoring.Learnt from: littla2liyou PR: opentiny/tiny-engine#1661 File: packages/runtime-renderer/src/utils/data-utils.ts:22-31 Timestamp: 2025-10-29T05:47:19.824Z Learning: In packages/runtime-renderer/src/utils/data-utils.ts, the parseJSFunction function uses Function constructor to parse user-defined functions from low-code schemas. This is an intentional architectural decision for the low-code platform to enable users to define custom logic in their schemas.
| const transformJSX = (code) => { | ||
| const res = transformSync(code, { | ||
| plugins: [ | ||
| [ | ||
| babelPluginJSX, | ||
| { | ||
| pragma: 'h' | ||
| } | ||
| ] | ||
| ] | ||
| }) | ||
| return (res.code || '') | ||
| .replace(/import \{.+\} from "vue";/, '') | ||
| .replace(/h\(_?resolveComponent\((.*?)\)/g, `h(this.getComponent($1)`) | ||
| .replace(/_?resolveComponent/g, 'h') | ||
| .replace(/_?createTextVNode\((.*?)\)/g, '$1') | ||
| .trim() | ||
| } |
There was a problem hiding this comment.
Guard Babel result: res may be null.
transformSync can return null; res.code access will throw.
const transformJSX = (code) => {
const res = transformSync(code, {
@@
- return (res.code || '')
+ const out = res?.code || ''
- .replace(/import \{.+\} from "vue";/, '')
+ return out
+ .replace(/import \{.+\} from "vue";/, '')
.replace(/h\(_?resolveComponent\((.*?)\)/g, `h(this.getComponent($1)`)
.replace(/_?resolveComponent/g, 'h')
.replace(/_?createTextVNode\((.*?)\)/g, '$1')
.trim()
}🤖 Prompt for AI Agents
In packages/runtime-renderer/src/renderer/parser/parser.ts around lines 76 to
93, the code assumes transformSync returns a non-null object and directly
accesses res.code; update the function to safely handle a null/undefined result
by checking res and res.code before using them (e.g., use optional chaining or
an explicit null check), and return an empty string or a clear error/log if res
is null so the subsequent replace calls never operate on undefined.
| const parseJSSlot = (data, _scope, _ctx) => { | ||
| return ($scope) => renderDefault(data.value, { ..._scope, ...$scope }, data) | ||
| } |
There was a problem hiding this comment.
Bug: renderDefault called with 3 args; it requires renderComponent.
This will throw when renderDefault tries to call renderComponent.
-const parseJSSlot = (data, _scope, _ctx) => {
- return ($scope) => renderDefault(data.value, { ..._scope, ...$scope }, data)
+const parseJSSlot = (data, _scope, ctx) => {
+ return ($scope, renderComponent?) =>
+ renderDefault(data.value, { ..._scope, ...$scope }, data, renderComponent || ctx?.renderComponent)
}🤖 Prompt for AI Agents
In packages/runtime-renderer/src/renderer/parser/parser.ts around lines 221 to
223, parseJSSlot currently returns a function that calls renderDefault with
three arguments which causes a runtime error because renderDefault expects
different usage and the code should be invoking renderComponent; update the
returned function to call renderComponent(data.value, { ..._scope, ...$scope },
data) instead of renderDefault, and add/import renderComponent if it isn’t
already imported so the call matches renderComponent’s expected signature.
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
当前社区内对低代码应用的运行时预览有较强呼声，为了实现运行时渲染，新实现了一个runtime-renderer包来执行整个应用schema的运行时渲染
What is the current behavior?
当前仅能在设计器或者预览中查看页面效果，完整的应用效果需要出码
Issue Number: N/A
What is the new behavior?
可以点击预览按钮旁的runtime-deploy这个toolbar按钮进入运行时渲染窗口，直接查看应用运行时渲染效果
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Data & State
Styling & UX
UI