feat: schemaDataAsync #885
Conversation
WalkthroughThe pull request includes extensive modifications across multiple files, focusing on enhancing functionality and improving the structure of code related to schema management and component interactions. Key changes involve the formatting of JSON objects in Changes
Possibly related PRs
Suggested labels
Suggested reviewers
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
github-actions
bot
added
enhancement
chilingling
force-pushed
the
feat/schemaDataAsync
branch
from
ba58c1b to
5ab6050
Compare
chilingling
force-pushed
the
feat/schemaDataAsync
branch
from
82c922f to
1f023f4
Compare
chilingling
force-pushed
the
feat/schemaDataAsync
branch
from
9e39e65 to
e335ce7
Compare
chilingling
force-pushed
the
feat/schemaDataAsync
branch
from
4ae4714 to
411f916
Compare
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 16
🧹 Outside diff range and nitpick comments (48)
packages/canvas/container/src/components/CanvasAction.vue (2)
238-238: Consider caching the useCanvas hook resultThe
useCanvas()hook is called on every computation ofshowToParent. Consider extracting the hook call to the setup function to avoid potential performance impacts.- const showToParent = computed(() => getCurrent().parent !== useCanvas().getSchema()) + const canvas = useCanvas() + const showToParent = computed(() => getCurrent().parent !== canvas.getSchema())
Line range hint
347-452: Consider adding documentation for the positioning strategyThe
Alignclass and positioning logic are complex but crucial for the component's functionality. Consider adding JSDoc comments to explain:
- The purpose and strategy of the positioning system
- The significance of alignment calculations
- Edge cases and their handling
packages/plugins/state/src/DataSourceList.vue (2)
Line range hint
71-91: Consider enhancing error handling and user feedbackWhile the code correctly prevents deletion of referenced stores, consider these improvements:
- Add error handling for cases where
pageTreemight be undefined- Enhance the message UI to show the exact locations where the store is referenced
Here's a suggested improvement:
const appPages = useResource().appSchemaState.pageTree.filter( (page) => page.componentName === COMPONENT_NAME.Page && page?.meta?.group !== 'publicPages' ) const expression = `stores.${key}` - const expresstionPages = findExpressionInAppSchema(appPages, expression) + try { + const expressionPages = findExpressionInAppSchema(appPages, expression) + + if (expressionPages.length > 0) { + useModal().message({ + title: '提示', + message: ( + <div> + <div>不允许删除，因为检查到 {expression} 在以下页面中被引用：</div> + <div style="max-height: 200px; overflow-y: auto; margin-top: 8px;"> + {expressionPages.map((pageName) => ( + <div key={pageName} style="padding: 4px 0;">• {pageName}</div> + ))} + </div> + </div> + ) + }) + } else { + useModal().confirm({ + title: '提示', + message: `您确定要删除 ${key} 吗？`, + exec: () => emit('removeStore', key) + }) + } + } catch (error) { + useModal().message({ + title: '错误', + message: '检查store引用时发生错误，请稍后重试' + }) + }
Line range hint
71-91: Consider implementing store reference cachingFor better performance, consider caching the store references instead of searching through the entire schema on every deletion attempt. This would be especially beneficial for large applications with many pages.
Would you like me to provide an example implementation of a caching mechanism for store references?
packages/plugins/datasource/src/js/datasource.js (2)
65-66: Consider request lifecycle implicationsThe migration of
willFetchhandler toappSchemaStatecompletes the handler migration pattern. This change affects the request lifecycle, particularly pre-request validation and transformation.Consider documenting the request lifecycle flow in the component's documentation, especially highlighting:
- The order of handler execution
- The interaction between global and local handlers
- The state management flow through
appSchemaState
57-66: Consider implementing handler state management patternThe migration of all handlers to
appSchemaStatesuggests an opportunity for a more structured state management pattern.Consider implementing a dedicated handler management system that could:
- Provide type safety for handlers
- Implement handler composition patterns
- Add handler lifecycle hooks
Example structure:
interface HandlerState { dataHandler: Handler<Data>; errorHandler: Handler<Error>; willFetch: Handler<Request>; } class HandlerManager { constructor(private state: HandlerState) {} composeHandlers<T>(handler: Handler<T>, defaultHandler: Handler<T>): Handler<T> { return this.state[handler] || defaultHandler; } }packages/common/component/LifeCycles.vue (1)
158-161: Consider adding error handling for schema access.While the simplified schema access is good, consider adding error handling to gracefully handle cases where neither props nor schema contains the lifecycle data.
Consider this enhancement:
- const bindLifeCycleSource = props.bindLifeCycles?.[item] || useCanvas().getSchema()?.lifeCycles?.[item] + const bindLifeCycleSource = props.bindLifeCycles?.[item] || useCanvas().getSchema()?.lifeCycles?.[item] + if (!bindLifeCycleSource) { + console.warn(`No lifecycle source found for ${item}`) + } state.editorValue = bindLifeCycleSource?.value || `function ${item} (${item === 'setup' ? '{ props, state, watch, onMounted }' : ''}) {} `packages/settings/events/src/components/AdvanceConfig.vue (3)
138-138: Improve number checking logic for clarityThe current method for checking if
valueis a number can be simplified and made more readable. UsingisNaNprovides a clearer intention.Apply this diff to improve the number checking logic:
- const isNumber = Number(value).toString() !== 'NaN' + const isNumber = !isNaN(Number(value))
202-207: Avoid mutatingloopArgsdirectly insetLoopIndexDirectly modifying
loopArgscan lead to unintended side effects since it mutates the original array in the schema. It's better to create a new array to ensure immutability.Apply this diff to clone
loopArgsbefore modifying:- loopArgs[1] = value || DEFAULT_LOOP_NAME.INDEX + loopArgs = loopArgs ? [...loopArgs] : [] + loopArgs[1] = value || DEFAULT_LOOP_NAME.INDEX
237-242: Avoid mutatingloopArgsdirectly insetLoopItemSimilarly, in
setLoopItem, cloningloopArgsbefore modification prevents potential side effects from mutating the original array.Apply this diff to clone
loopArgsbefore modifying:- loopArgs[0] = value || DEFAULT_LOOP_NAME.ITEM + loopArgs = loopArgs ? [...loopArgs] : [] + loopArgs[0] = value || DEFAULT_LOOP_NAME.ITEMpackages/configurator/src/container-configurator/ContainerConfigurator.vue (1)
84-86: Confirm insertion position when adding new tabsIn line 86, the new child node is inserted with
position: 'after'. If the intention is to add the new tab at the end of the existing tabs, consider usingposition: 'append'for clarity and to avoid unexpected ordering.Apply this diff if necessary:
-operateNode({ type: 'insert', parentId: schema.id, newNodeData, position: 'after' }) +operateNode({ type: 'insert', parentId: schema.id, newNodeData, position: 'append' })packages/canvas/render/src/builtin/CanvasCollection.vue (1)
51-53: Add Null Checks When Accessing Global ObjectsWhen accessing
window.host.schemaUtils, consider adding checks to ensure thatwindow,window.host, andwindow.host.schemaUtilsare defined. This will prevent potential runtime errors if any of these objects are undefined.packages/plugins/schema/src/Main.vue (2)
105-107: Implement History Stack Management for Undo/Redo FunctionalityThe history stack management (
useHistory().addHistory()) is commented out with a TODO note. Implementing this is crucial for supporting undo and redo actions within the application.Would you like assistance in implementing the history stack management? I can help draft the necessary code changes.
130-136: Ensure Proper Subscription Management in Lifecycle HooksIn the
onActivatedhook, you subscribe to theschemaChangetopic, and inonDeactivated, you unsubscribe. Confirm that these hooks are appropriate for your component's lifecycle, especially if it's wrapped with<keep-alive>. Ensure that subscriptions are managed correctly to prevent memory leaks or missed messages.packages/configurator/src/html-attributes-configurator/HtmlAttributesConfigurator.vue (2)
98-117: Optimize Attribute Processing LogicThe current logic iterates over
propsand updatesattrsby checking if keys are excluded. Consider refactoring this block to improve readability and efficiency. For example, filter the entries first and then map them toattrs, reducing the complexity of the loop.Apply this diff to refactor the attribute processing logic:
+const filteredProps = Object.entries(useProperties().getSchema()?.props || {}).filter( + ([key]) => !properties.includes(key) +) +attrs.value = filteredProps.map(([key, value]) => ({ + id: utils.guid(), + text: `${key} = '${value}'`, + data: { key, value } +}))
Line range hint
127-143: Improve Schema Update Method by Avoiding Direct Schema ModificationCurrently,
updateSchemaretrieves the schema multiple times and directly modifies it. Consider minimizing direct schema access and utilizeoperateNodeconsistently for updates.Apply this diff to streamline the schema update process:
-const schema = useProperties().getSchema() -operateNode({ type: 'updateAttributes', id: schema.id, value: { props: mergeProps } }) +operateNode({ type: 'updateAttributes', id: useProperties().getSchema().id, value: { props: mergeProps } })packages/configurator/src/js-slot-configurator/JsSlotConfigurator.vue (2)
97-110: ClarifyupdateSlotFunction LogicThe
updateSlotfunction has been updated to handle slot binding and unbinding with additional parameters. Ensure that the logic for toggling slots and updating the slot data is clear and accounts for all edge cases.
157-167: Separate Parameter Validation from Slot Update LogicIn
setParams, parameters are set only if the slot is bound. Ensure that parameter validation and setting are performed consistently, and consider separating concerns by validating parameters before updating the slot.packages/canvas/container/src/CanvasContainer.vue (1)
Line range hint
167-177: Ensure proper cleanup of event listeners added towindowIn the
runfunction, event listeners are added towindowfor'mousedown'and'dragenter'events. However, these listeners are not removed in theonUnmountedlifecycle hook, which can lead to memory leaks and unintended side effects. It's important to remove these listeners when the component is unmounted.To fix this issue, add the corresponding
window.removeEventListenercalls in theonUnmountedhook:onUnmounted(() => { if (iframe.value?.contentDocument) { removeHostkeyEvent(iframe.value.contentDocument) } window.removeEventListener('message', updateI18n, false) + window.removeEventListener('mousedown', windowMouseDownHandler) + window.removeEventListener('dragenter', windowDragEnterHandler) })You will need to define the event handler functions separately to enable removing them properly:
- window.addEventListener('mousedown', (e) => { - insertPosition.value = insertPanel.value?.contains(e.target) - target.value = e.target - }) + const windowMouseDownHandler = (e) => { + insertPosition.value = insertPanel.value?.contains(e.target) + target.value = e.target + } + window.addEventListener('mousedown', windowMouseDownHandler) - window.addEventListener('dragenter', () => { - // 如果拖拽范围超出了iframe范围，则清空拖拽位置数据 - clearLineState() - }) + const windowDragEnterHandler = () => { + // 如果拖拽范围超出了iframe范围，则清空拖拽位置数据 + clearLineState() + } + window.addEventListener('dragenter', windowDragEnterHandler)packages/configurator/src/variable-configurator/VariableConfigurator.vue (1)
Line range hint
402-418: Address potential issue with schema state updateThe
updateSchemafunction is called with a spread ofpageSchema.state:-updateSchema({ ...pageSchema.state, [stateName]: staticData }) +updateSchema({ state: { ...pageSchema.state, [stateName]: staticData } })Passing
{ ...pageSchema.state, [stateName]: staticData }toupdateSchemamay unintentionally overwrite the entire schema with the state properties. To correctly update thestateproperty without affecting other parts of the schema, wrap the state updates within thestatekey.packages/configurator/src/table-columns-configurator/TableColumnsConfigurator.vue (1)
26-26: Architectural improvement with potential error handling enhancementThe change to use
operateNodealigns with the PR's objective of providing a unified entry point for schema modifications. However, consider adding error handling for the operation.- useCanvas().operateNode({ type: 'updateAttributes', id: useProperties().getSchema().id, value: { children } }) + try { + useCanvas().operateNode({ + type: 'updateAttributes', + id: useProperties().getSchema().id, + value: { children } + }) + } catch (error) { + console.error('Failed to update table columns:', error) + // Consider adding user notification here + }packages/settings/styles/src/js/useEditor.js (1)
18-18: Track TODO commentThe TODO comment about checking references should be tracked and addressed.
Would you like me to create a GitHub issue to track this TODO item? I can help search for references to determine if this code can be safely removed.
packages/toolbars/preview/src/Main.vue (1)
69-69: Consider strengthening error handling for getSchemaThe optional chaining (
?.) suggestsgetSchemamight be undefined. Consider adding explicit error handling to provide better feedback when schema access fails.- schema: getSchema?.() + schema: getSchema?.() || {}packages/configurator/src/html-text-configurator/HtmlTextConfigurator.vue (1)
56-61: Consider batching multiple attribute updatesThe implementation correctly uses
operateNodefor schema updates. However, if there are cases where multiple attributes need to be updated simultaneously, consider batching them in a singleoperateNodecall to reduce the number of update events.const change = (value) => { const { operateNode } = useCanvas() const schema = useProperties().getSchema() if (schema) { - operateNode({ type: 'updateAttributes', id: schema.id, value: { children: value } }) + // If multiple attributes need updating, batch them: + operateNode({ + type: 'updateAttributes', + id: schema.id, + value: { + children: value, + // other attributes... + } + }) } }packages/configurator/src/related-editor-configurator/RelatedEditorConfigurator.vue (1)
Line range hint
93-97: Consider caching schema lookupsWhile the transition to
useCanvas().getSchema()is correct, consider caching the schema lookup result if it's used multiple times within the same execution context to improve performance.const getStateValue = (value) => { let newValue = value if (value?.type === CONSTANT.JSEXPRESSION) { - const pageSchema = useCanvas().getSchema() + // Cache schema lookup if used frequently + if (!this.cachedSchema) { + this.cachedSchema = useCanvas().getSchema() + } + const pageSchema = this.cachedSchema const stateName = value?.value?.replace(CONSTANT.STATE, '') newValue = pageSchema?.state?.[stateName] } return newValue }packages/canvas/container/src/keyboard.js (1)
65-65: Consider error handling for schema retrievalWhile the change to use useCanvas().getSchema() aligns with the new pattern, consider adding error handling for cases where schema retrieval fails.
- schema = useCanvas().getSchema() + try { + schema = useCanvas().getSchema() + } catch (error) { + console.error('Failed to retrieve schema:', error) + return + }packages/canvas/DesignCanvas/README.md (4)
17-17: Consider adding error handling guidanceThe warning about node manipulation limitations is crucial. Consider adding examples of error cases and how to handle them to prevent runtime issues.
48-48: Fix markdown heading formatRemove trailing colons from headings to comply with markdown standards:
-### 获取当前页面/区块 schema: +### 获取当前页面/区块 schema -### 获取当前选中节点 schema: +### 获取当前选中节点 schemaAlso applies to: 56-56
🧰 Tools
🪛 Markdownlint (0.35.0)
48-48: Punctuation: ':'
Trailing punctuation in heading(MD026, no-trailing-punctuation)
179-179: Add type information for value property exampleThe example for updateAttributes operation should include more specific type information in the spread operator to better guide developers.
- value: { props: { ... }, loop: { ... } }, + value: { + props: { /* INodeProps */ }, + loop: { /* ILoopConfig */ } + },
20-20: Consider versioning experimental APIsAdd version numbers or feature flags for experimental APIs to help track adoption and future stability changes.
packages/settings/props/src/composable/useProperties.js (2)
124-125: Consider optimizing props spread for large objectsFor components with many props, spreading the entire props object could impact performance. Consider using
Object.assignor selective property copying for better performance with large prop objects.- const newProps = { ...(properties.schema.props || {}) } + const newProps = Object.assign({}, properties.schema.props || {})
150-159: Consider consolidating duplicate code with setPropThe
delPropfunction shares similar logic withsetProp. Consider refactoring to reuse the logic.-const delProp = (name) => { - const newProps = { ...(properties.schema.props || {}) } - - delete newProps[name] - - useCanvas().operateNode({ - type: 'changeProps', - id: properties.schema.id, - value: { props: newProps }, - option: { overwrite: true } - }) - propsUpdateKey.value++ -} +const delProp = (name) => setProp(name, null)🧰 Tools
🪛 Biome (1.9.4)
[error] 157-158: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/toolbars/save/src/js/index.js (1)
Line range hint
156-186: Remove or implement commented codeThe commented code block contains important functionality for retrieving original schema data. Consider either implementing this feature or removing the comments if they're no longer relevant.
packages/configurator/src/slot-configurator/SlotConfigurator.vue (1)
101-106: LGTM: Improved template removal logicThe change from splice to filter improves code clarity. The consistent use of
operateNodemaintains the unified schema modification approach.Consider extracting the filter condition to a named function for better readability:
-const newChildren = schema.children.filter( - ({ componentName, props }) => - componentName !== 'Template' || (props?.slot !== name && props?.slot?.name !== name) -) +const isNotMatchingTemplate = ({ componentName, props }) => + componentName !== 'Template' || (props?.slot !== name && props?.slot?.name !== name) +const newChildren = schema.children.filter(isNotMatchingTemplate)packages/plugins/materials/src/composable/useResource.js (1)
Line range hint
124-153: LGTM: Well-structured app state fetchingThe
fetchAppStatefunction provides a clean implementation for fetching and updating the application schema state, with proper fallbacks for interceptors.Consider implementing a caching mechanism for the app schema to improve performance for frequently accessed data. This could be particularly beneficial for the
dataSourceandutilsproperties which might not change frequently.packages/common/component/BlockLinkField.vue (1)
Line range hint
82-83: Consider refactoring getCurrent access patternThe functions still use
canvasApi.value?.getCurrent?.()while the rest of the code has moved to using direct methods fromuseCanvas. Consider refactoring for consistency with the new pattern.- const { - schema: { id, componentName } - } = canvasApi.value?.getCurrent?.() || {} + const { + schema: { id, componentName } + } = useCanvas().getCurrent?.() || {}Also applies to: 108-109
packages/common/component/BlockLinkEvent.vue (1)
Line range hint
89-90: Consider consistent schema access patternSimilar to BlockLinkField.vue, these functions use the old
canvasApi.value?.getCurrent?.()pattern. Consider refactoring for consistency.- const { - schema: { id, componentName } - } = useCanvas().canvasApi.value?.getCurrent?.() || {} + const { + schema: { id, componentName } + } = useCanvas().getCurrent?.() || {}Also applies to: 125-126
packages/canvas/DesignCanvas/src/DesignCanvas.vue (1)
135-149: LGTM! Improved node selection logic.The changes enhance node selection by:
- Using dedicated methods like
getNodeById- Clearly separating canvas-level and node-level schema handling
- Maintaining a clean flow of data
Consider implementing a publish-subscribe pattern for node selection events to allow other components to react to selection changes without tight coupling.
packages/common/component/BlockDeployDialog.vue (1)
Line range hint
213-219: Consider improving the error message.The change to use
importSchemais good, but the error message could be more specific about the JSON parsing error.Consider this improvement:
- message: '代码静态检查有错误，请先修改后再保存' + message: `JSON 解析错误: ${err.message}，请检查格式后重试`packages/settings/styles/src/Main.vue (1)
138-152: Good implementation of centralized style updates.The
updateStyleToSchemamethod properly handles both regular schema updates and canvas page schema updates. However, there's a TODO comment that needs to be addressed.The TODO comment on line 150 indicates a need to support style key removal. Would you like me to help implement this feature or create a GitHub issue to track it?
packages/plugins/block/src/composable/useBlock.js (1)
279-285: Good improvement in schema management!The changes improve schema handling by:
- Using explicit getSchema() instead of accessing through canvasApi
- Ensuring proper initialization with await
- Maintaining a single source of truth for block schema
Consider adding error handling for the async operation.
Add error handling:
- await resetBlockCanvasState({ pageSchema: getBlockPageSchema(block) }) - block.content = getSchema() + try { + await resetBlockCanvasState({ pageSchema: getBlockPageSchema(block) }) + block.content = getSchema() + } catch (error) { + console.error('Failed to initialize block schema:', error) + throw error + }packages/canvas/render/src/RenderMain.js (2)
357-364: Consider making the throttle interval configurable inthrottleUpdateSchemaThe throttle interval for
throttleUpdateSchemais hardcoded to 100 milliseconds. Making this interval configurable would provide flexibility to adjust the frequency of schema updates based on performance needs.Proposed change:
+const THROTTLE_INTERVAL = 100; // You can adjust this value as needed const throttleUpdateSchema = useThrottleFn( () => { window.host.patchLatestSchema(schema) }, - 100, + THROTTLE_INTERVAL, true )
390-415: Assess performance implications of deep watchers on large objectsUsing deep watchers on
schema.methods,schema.css, andschema.statecan have performance overhead, especially if these objects are large or change frequently. Consider optimizing the watchers or restructuring data to minimize performance impact.Possible approaches:
- Use shallow watchers if deep observation isn't necessary.
- Debounce or throttle the callbacks to reduce frequent executions.
- Split large objects into smaller, more manageable parts and watch them individually.
packages/settings/styles/src/js/useStyle.js (1)
133-173: Good refactoring of style panel watch initialization.The centralization of watch setup logic improves code organization and maintainability. The deep watch on schema changes ensures proper reactivity.
Consider adding error boundaries around the schema retrieval:
- let schema = getCurrentSchema() + let schema + try { + schema = getCurrentSchema() + } catch (error) { + console.error('Failed to get current schema:', error) + return + }packages/canvas/render/src/builtin/CanvasCollection.js (4)
Line range hint
104-112: Consider improving robustness and immutabilityWhile the function works, consider these improvements:
- Add input validation
- Return a new array instead of mutating the input
- Use deep cloning for nested objects if needed
-const generateAssignColumns = (newColumns, oldColumns) => { +const generateAssignColumns = (newColumns, oldColumns) => { + if (!Array.isArray(newColumns) || !Array.isArray(oldColumns)) { + return newColumns; + } + return newColumns.map((item) => { + const targetColumn = oldColumns.find((value) => value.field === item.field); + return targetColumn ? { ...item, ...targetColumn } : item; + }); - newColumns.forEach((item) => { - const targetColumn = oldColumns.find((value) => value.field === item.field) - if (targetColumn) { - Object.assign(item, targetColumn) - } - }) - return newColumns }
Line range hint
137-183: Extract pagination configuration to constantsThe pagination configuration is hardcoded. Consider extracting it to a configurable constant or making it customizable through props.
+const DEFAULT_PAGER_CONFIG = { + attrs: { + currentPage: 1, + pageSize: 50, + pageSizes: [10, 20, 50], + total: 0, + layout: 'sizes,total, prev, pager, next, jumper' + } +}; - node.props.pager = pageConfig + node.props.pager = { ...DEFAULT_PAGER_CONFIG }🧰 Tools
🪛 Biome (1.9.4)
[error] 146-148: Avoid the delete operator which can impact performance.
(lint/performance/noDelete)
Line range hint
185-205: Enhance cleanup in clearBindVarThe cleanup function only removes the method but should also clean up any related props or state.
const clearBindVar = () => { // 当数据源组件children字段为空时，及时清空创建的methods delete pageSchema.methods[methodName] + // Clean up related props + if (node.props) { + delete node.props.fetchData; + delete node.props.pager; + } }
281-283: Improve readability of handler selectionConsider restructuring the ternary operation for better readability.
-export const getHandler = ({ node, sourceRef, schemaId, pageSchema, updateKey }) => - extraHandlerMap[node.componentName] - ? extraHandlerMap[node.componentName]({ node, sourceRef, schemaId, pageSchema, updateKey }) - : defaultHandlerTemplate({ node, sourceRef, schemaId, pageSchema }) +export const getHandler = ({ node, sourceRef, schemaId, pageSchema, updateKey }) => { + const handler = extraHandlerMap[node.componentName]; + const params = { node, sourceRef, schemaId, pageSchema, updateKey }; + + return handler ? handler(params) : defaultHandlerTemplate(params); +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (60)
designer-demo/public/mock/bundle.json(52 hunks)packages/canvas/DesignCanvas/README.md(1 hunks)packages/canvas/DesignCanvas/src/DesignCanvas.vue(4 hunks)packages/canvas/DesignCanvas/src/api/useCanvas.js(6 hunks)packages/canvas/container/src/CanvasContainer.vue(2 hunks)packages/canvas/container/src/components/CanvasAction.vue(2 hunks)packages/canvas/container/src/container.js(12 hunks)packages/canvas/container/src/keyboard.js(2 hunks)packages/canvas/package.json(1 hunks)packages/canvas/render/src/RenderMain.js(9 hunks)packages/canvas/render/src/builtin/CanvasCollection.js(5 hunks)packages/canvas/render/src/builtin/CanvasCollection.vue(5 hunks)packages/canvas/render/src/context.js(0 hunks)packages/canvas/render/src/render.js(4 hunks)packages/common/component/BlockDeployDialog.vue(2 hunks)packages/common/component/BlockLinkEvent.vue(1 hunks)packages/common/component/BlockLinkField.vue(1 hunks)packages/common/component/ConfigItem.vue(2 hunks)packages/common/component/LifeCycles.vue(2 hunks)packages/common/component/MetaListItems.vue(2 hunks)packages/common/js/canvas.js(1 hunks)packages/common/js/completion.js(1 hunks)packages/configurator/src/collection-configurator/CollectionConfigurator.vue(2 hunks)packages/configurator/src/container-configurator/ContainerConfigurator.vue(3 hunks)packages/configurator/src/html-attributes-configurator/HtmlAttributesConfigurator.vue(4 hunks)packages/configurator/src/html-text-configurator/HtmlTextConfigurator.vue(2 hunks)packages/configurator/src/js-slot-configurator/JsSlotConfigurator.vue(4 hunks)packages/configurator/src/layout-grid-configurator/LayoutGridConfigurator.vue(3 hunks)packages/configurator/src/related-editor-configurator/RelatedEditorConfigurator.vue(1 hunks)packages/configurator/src/slot-configurator/SlotConfigurator.vue(3 hunks)packages/configurator/src/table-columns-configurator/TableColumnsConfigurator.vue(1 hunks)packages/configurator/src/variable-configurator/VariableConfigurator.vue(7 hunks)packages/plugins/block/src/composable/useBlock.js(1 hunks)packages/plugins/bridge/src/js/resource.js(4 hunks)packages/plugins/datasource/src/DataSourceGlobalDataHandler.vue(2 hunks)packages/plugins/datasource/src/DataSourceList.vue(4 hunks)packages/plugins/datasource/src/DataSourceRecordList.vue(3 hunks)packages/plugins/datasource/src/js/datasource.js(1 hunks)packages/plugins/i18n/src/composable/useTranslate.js(2 hunks)packages/plugins/materials/src/composable/useMaterial.js(1 hunks)packages/plugins/materials/src/composable/useResource.js(6 hunks)packages/plugins/schema/package.json(1 hunks)packages/plugins/schema/src/Main.vue(4 hunks)packages/plugins/script/src/js/method.js(4 hunks)packages/plugins/state/src/DataSourceList.vue(1 hunks)packages/plugins/state/src/Main.vue(8 hunks)packages/plugins/tree/src/Main.vue(2 hunks)packages/settings/events/src/components/AdvanceConfig.vue(2 hunks)packages/settings/props/src/components/Empty.vue(2 hunks)packages/settings/props/src/components/modal/ModalContent.vue(2 hunks)packages/settings/props/src/composable/useProperties.js(2 hunks)packages/settings/styles/src/Main.vue(1 hunks)packages/settings/styles/src/components/classNamesContainer/index.vue(3 hunks)packages/settings/styles/src/js/useEditor.js(2 hunks)packages/settings/styles/src/js/useStyle.js(5 hunks)packages/toolbars/generate-code/src/Main.vue(1 hunks)packages/toolbars/preview/src/Main.vue(2 hunks)packages/toolbars/redoundo/src/composable/useHistory.js(2 hunks)packages/toolbars/save/src/js/index.js(2 hunks)packages/utils/src/constants/index.js(1 hunks)
💤 Files with no reviewable changes (1)
- packages/canvas/render/src/context.js
✅ Files skipped from review due to trivial changes (1)
- designer-demo/public/mock/bundle.json
👮 Files not reviewed due to content moderation or server errors (3)
- packages/plugins/datasource/src/DataSourceGlobalDataHandler.vue
- packages/plugins/datasource/src/DataSourceList.vue
- packages/plugins/datasource/src/DataSourceRecordList.vue
🧰 Additional context used
🪛 Biome (1.9.4)
packages/settings/props/src/composable/useProperties.js
[error] 157-158: Change to an optional chain.
Unsafe fix: Change to an optional chain.
(lint/complexity/useOptionalChain)
packages/plugins/script/src/js/method.js
[error] 121-122: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
🪛 Markdownlint (0.35.0)
packages/canvas/DesignCanvas/README.md
48-48: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
56-56: Punctuation: ':'
Trailing punctuation in heading
(MD026, no-trailing-punctuation)
🔇 Additional comments (77)
packages/canvas/container/src/components/CanvasAction.vue (2)
130-130: LGTM: Import changes align with schema management objectives
The addition of useCanvas hook aligns with the PR's goal of providing a unified entry point for schema operations.
Line range hint 516-704: LGTM: Well-structured styling with proper theming support
The styling implementation:
- Uses CSS variables for consistent theming
- Handles complex positioning and resize interactions
- Maintains clear separation of concerns
packages/plugins/state/src/DataSourceList.vue (1)
71-71: Verify the migration from resState to appSchemaState
The change aligns with the PR's objective of consolidating schema state management. However, we should ensure that appSchemaState.pageTree is always available when this code executes.
Let's verify the availability and initialization of appSchemaState.pageTree:
✅ Verification successful
The schema state access is properly initialized and safe to use
The verification shows that appSchemaState.pageTree is:
- Properly initialized in
useResourcecomposable during app schema fetch - Set from
appData.componentsTreewhich is retrieved via HTTP API - Used consistently across multiple components
- Accessed safely through the
useResource()hook which is properly imported
The migration from resState to appSchemaState is correctly implemented and the state is guaranteed to be available when the component loads.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for appSchemaState initialization and usage patterns
rg -A 3 "appSchemaState\s*=|appSchemaState:" packages/
# Search for pageTree initialization
ast-grep --pattern 'appSchemaState.pageTree = $_'
Length of output: 529
Script:
#!/bin/bash
# Let's verify the initialization flow and usage
rg -B 2 -A 5 "appSchemaState.pageTree = appData.componentsTree" packages/
# Check where useResource is imported and used
rg "useResource" packages/
# Check the structure and initialization of appData
rg "appData\s*=|appData:" packages/
Length of output: 7974
packages/plugins/datasource/src/js/datasource.js (2)
57-58: Verify dataHandler availability in appSchemaState
The change from resState to appSchemaState aligns with the PR's objective of unifying schema operations. However, we should verify that dataHandler is consistently available in appSchemaState.
✅ Verification successful
The search results provide sufficient context to generate a final response. The findings show that:
appSchemaState.dataHandleris properly initialized inuseResource.jswith a fallback toDEFAULT_INTERCEPTOR.dataHandler- It's used consistently in
datasource.jsfor request handling - It's properly updated in
DataSourceGlobalDataHandler.vuewith the correct structure{ type: 'JSFunction', value: ... }
The change to use appSchemaState.dataHandler is verified and safe
The dataHandler is consistently available in appSchemaState with proper initialization, fallback values, and correct structure. The change aligns with the unified schema management approach while maintaining the expected handler functionality.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other occurrences of dataHandler in appSchemaState
rg "appSchemaState.dataHandler" -A 2 -B 2
Length of output: 1965
61-62: Ensure consistent error handling across state transitions
The migration to appSchemaState for error handling is consistent with the overall changes. However, this is a critical path for error handling.
✅ Verification successful
Let me gather more information about the error handler usage across the codebase.
Based on the analysis of the shell script results, I can now provide the final response:
The error handler migration to appSchemaState is properly implemented and consistent
The error handling implementation follows the established pattern across the codebase:
- Global error handler is properly initialized from
appSchemaStatewith fallback toDEFAULT_INTERCEPTOR - Error handling is consistently applied in HTTP interceptors for both requests and responses
- The error handler state is properly managed in the Vue components and data source configurations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify error handler initialization and usage
rg "errorHandler.*appSchemaState" -A 2 -B 2
# Check for any error handling patterns that might need updating
ast-grep --pattern 'errorHandler = $$$'
Length of output: 633
Script:
#!/bin/bash
# Search for error handler usage patterns
rg "errorHandler" -A 3 -B 3
# Look for error handling function definitions
ast-grep --pattern 'function $_(error) {
$$$
}'
# Check Vue component error handling
ast-grep --pattern 'onError($$$) {
$$$
}'
Length of output: 36202
packages/common/component/LifeCycles.vue (2)
Line range hint 135-139: LGTM! Simplified schema access pattern.
The change aligns with the PR's objective of providing a unified schema modification entry point. The new getSchema() method provides a cleaner and more direct way to access the schema.
Let's verify that the new schema access pattern is consistently implemented:
✅ Verification successful
Based on the comprehensive search results, I can now generate the final response:
LGTM! The schema access pattern is consistently implemented across the codebase.
The change from useCanvas().canvasApi.value?.getSchema?.() to useCanvas().getSchema() aligns with the codebase's unified schema access pattern. The search results confirm that:
- The new
getSchema()method is properly implemented in the canvas API (packages/canvas/DesignCanvas/src/api/useCanvas.js) - All other components consistently use this pattern for schema access
- No instances of the old pattern (
canvasApi.value?.getSchema) remain in the codebase
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of the old pattern and verify the new pattern exists
rg "canvasApi\.value\?\.getSchema"
ast-grep --pattern 'useCanvas().getSchema()'
Length of output: 258
Script:
#!/bin/bash
# Let's verify if there are any other variations of schema access patterns
rg "getSchema" -A 2
Length of output: 31604
Line range hint 134-165: Verify the impact of breaking changes on existing applications.
Since this PR introduces breaking changes in schema access patterns, we should verify that existing applications using this component continue to work as expected, particularly:
- Custom lifecycle implementations
- External components that might depend on the old schema access pattern
- Applications that might have extended this component's functionality
Let's check for potential impacts:
✅ Verification successful
Breaking changes appear to have minimal impact on dependent components
Based on the codebase analysis, the changes to lifecycle management appear to be safe because:
- PageSetting.vue and BlockSetting.vue only interact with LifeCycles component through the
bindLifeCyclesprop and don't directly access schema patterns - The schema access pattern change is internal to the LifeCycles component and doesn't affect its public API
- No custom implementations or extensions of the LifeCycles component were found
- The existing components use their own lifecycle storage mechanisms (page_content.lifeCycles and content.lifeCycles) which remain unchanged
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for components that might be affected by the breaking changes
rg -l "lifeCycles.*getSchema"
# Look for potential custom implementations or extensions
ast-grep --pattern 'extends LifeCycles'
Length of output: 73
Script:
#!/bin/bash
# Let's try a broader search to find lifecycle-related patterns
rg -l "lifeCycles" --type vue --type js --type ts
# Search for schema access patterns
rg -l "getSchema()" --type vue --type js --type ts
# Look for potential imports of LifeCycles
rg -l "import.*LifeCycles"
Length of output: 273
Script:
#!/bin/bash
# Let's examine the content of files that import LifeCycles
rg "lifeCycles" packages/plugins/page/src/PageSetting.vue -A 5 -B 5
rg "lifeCycles" packages/plugins/block/src/BlockSetting.vue -A 5 -B 5
# Let's also check for any usage patterns without file type restrictions
rg "getSchema.*lifeCycles" -A 3
Length of output: 2259
packages/configurator/src/container-configurator/ContainerConfigurator.vue (1)
42-65: Initialize child nodes with default properties if missing
The updateChildrenToValid function ensures that each child node has props with default values. This prevents potential errors caused by missing properties and maintains schema integrity.
packages/canvas/render/src/builtin/CanvasCollection.vue (2)
2-2: Ensure Proper Management of updateKey for Component Re-rendering
The :key="updateKey" prop has been added to the <component> to trigger re-renders when updateKey changes. Verify that updateKey is incremented appropriately to prevent unnecessary re-renders or missed updates.
108-111: Confirm Message Publication and Update of updateKey
After publishing the schemaChange message, updateKey.value is incremented to trigger a re-render. Ensure that subscribers to the schemaChange topic are handling the message correctly and that incrementing updateKey synchronizes the component state as intended.
packages/plugins/schema/src/Main.vue (1)
116-123: Verify Throttle Function Parameters in throttleUpdateData
The useThrottleFn is used with parameters (callback, 100, true). Confirm that the third parameter true corresponds to the trailing option and that this behavior aligns with the expected update strategy. Adjust the options if you intend to control leading and trailing executions differently.
packages/configurator/src/js-slot-configurator/JsSlotConfigurator.vue (1)
170-173: Ensure Correct Validation Call in validParams Function
In the validParams function, after validation succeeds, setParams is called. Verify that this callback correctly updates the slot parameters and that validation errors are handled appropriately.
packages/plugins/tree/src/Main.vue (1)
121-141: LGTM!
The implementation of message subscription and unsubscription in the onActivated and onDeactivated hooks is correct. The component properly updates pageSchema upon schema changes, ensuring the outline tree stays synchronized.
packages/settings/styles/src/components/classNamesContainer/index.vue (5)
114-114: Addition of setProp enhances property management
The inclusion of setProp from useProperties allows for more streamlined updates to properties within the component. This change improves code clarity and functionality.
174-180: Proper use of setProp within conditional schema handling
The code correctly checks for the existence of schema and utilizes setProp to update properties when schema is present. Incrementing propsUpdateKey ensures reactivity.
183-186: Fallback to page schema when component schema is unavailable
When schema is not present, the code appropriately retrieves the pageSchema and updates it using updateSchema. The spread operator is used correctly to merge existing properties with the new value.
192-192: Consistent retrieval of getCanvasPageSchema
Extracting getCanvasPageSchema from useCanvas aligns with the pattern used elsewhere in the code, ensuring consistency in schema retrieval.
450-453: Ensure that canvasApi.value is defined before using updateRect
Accessing updateRect from useCanvas().canvasApi.value may lead to errors if canvasApi.value is undefined. Please verify that canvasApi.value is always initialized before calling updateRect.
packages/common/component/ConfigItem.vue (5)
268-268: Included getSchema improves access to current schema
Adding getSchema from useProperties within updateValue facilitates direct access to the current schema, enhancing the function’s capability to manipulate component properties.
272-272: Safely retrieve componentName from the current schema
Using getSchema().componentName to obtain the current component's name is appropriate. Ensure that getSchema() reliably returns a valid schema to prevent potential runtime errors.
285-286: Integration of operateNode and isSaved improves node operations
Importing operateNode and isSaved from useCanvas enhances node manipulation capabilities and aligns with the application’s canvas management strategy.
288-289: Correct usage of getSchema for handling 'children' property
Retrieving the schema using getSchema() when updating the 'children' property ensures that the operation targets the correct node, maintaining data integrity.
291-291: Appropriate condition check before updating properties
The condition ensures that updates occur only when the canvas is saved and the page status is appropriate, preventing unintended changes during restricted states.
packages/canvas/container/src/container.js (7)
22-23: Updated imports reflect necessary dependencies
Importing useCanvas, useLayout, useTranslate, and useMaterial brings in essential functions for canvas operations. Including utils from @opentiny/tiny-engine-utils is appropriate for utility functions like guid().
76-76: Refactored getSchema function aligns with new API
Changing getSchema to use useCanvas().getPageSchema() centralizes schema retrieval and adheres to the updated canvas API structure.
247-257: Refactored node insertion using operateNode
The insertAfter function now uses useCanvas().operateNode to insert nodes, which standardizes node operations and improves code maintainability. Generating a unique ID when data.id is not provided ensures each node is identifiable.
287-291: Simplified node deletion with operateNode
Using useCanvas().operateNode to delete nodes in removeNode function streamlines the deletion process and maintains consistency with other node operations.
425-431: Accurate parent retrieval in ancestor check
Fetching the parent node using useCanvas().getNodeWithParentById in the isAncestor function accurately determines the ancestral relationship between nodes, ensuring correct insertion logic.
649-651: Enhanced node selection using updated API
In selectNode, retrieving the node and parent with useCanvas().getNodeWithParentById improves reliability. Emitting the 'selected' event with relevant data keeps the application state synchronized.
Also applies to: 654-654, 664-664
711-712: Consistent node copying functionality
The copyNode function correctly uses useCanvas().getNodeWithParentById to obtain the node and parent before inserting a copy, ensuring the copied node is accurately placed in the hierarchy.
packages/configurator/src/variable-configurator/VariableConfigurator.vue (5)
154-162: Refactored node access with useCanvas improves robustness
Updating the retrieval of getCurrent and getNodeWithParentById through useCanvas enhances the consistency of accessing canvas state and node hierarchy, leading to more reliable functionality.
453-453: Safely accessing state variables from the schema
Retrieving state variables using useCanvas().getSchema()?.state || {} ensures that even if getSchema() returns undefined, state.variables will default to an empty object, preventing potential runtime errors.
Line range hint 461-477: Dynamic variable selection logic enhanced
Using useCanvas().getSchema() within selectItem method streamlines access to properties and state variables, improving the flexibility of variable binding functionality.
470-470: Accessing application schema state using useResource
Fetching resources from useResource().appSchemaState accurately retrieves the necessary application state, supporting variable binding and dynamic configurations.
Also applies to: 505-505
526-526: Correctly obtaining variables based on selected item
Assigning state.variables = getSchema()?.[item.id] appropriately updates the variable list based on the user's selection, ensuring the correct variables are presented for binding.
packages/settings/props/src/components/Empty.vue (2)
6-7: Architectural improvement: Migration to useCanvas
The change from useBroadcastChannel to useCanvas aligns well with the PR's objective of creating a unified entry point for schema operations.
23-28: Consider potential race condition in watch setup
The watch setup might miss initial schema updates if getSchema() is not immediately available. Consider using watchEffect or adding immediate: true option to ensure proper initialization.
watch(
() => getSchema()?.children?.length,
(len) => {
tipsDesc.value = len ? EMPTY_SELECTION : EMPTY_COMPONENT
- }
+ },
+ { immediate: true }
)packages/plugins/schema/package.json (1)
31-31: Verify @vueuse/core version compatibility and security
The addition of @vueuse/core is appropriate for the new functionality. However, let's verify the version compatibility and check for any known security issues.
packages/common/js/canvas.js (1)
19-19: Verify the state transition from resState to appSchemaState
The change from resState.isDemo to appSchemaState.isDemo aligns with the PR's objective of unifying schema management. However, we should ensure this transition doesn't affect existing demo mode functionality.
✅ Verification successful
State transition from resState to appSchemaState is properly implemented
The verification shows that:
- The
isDemostate is correctly migrated fromresStatetoappSchemaState - The source of truth for
isDemois properly set inuseResource.jsfromappData.meta?.is_demo - The demo mode functionality in
canvas.jsremains intact with proper state management and conditional logic
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other occurrences of resState.isDemo to ensure complete migration
rg "resState\.isDemo"
# Search for all usages of appSchemaState.isDemo to verify consistency
rg "appSchemaState\.isDemo"
Length of output: 238
Script:
#!/bin/bash
# Check the previous implementation to understand the migration
git show HEAD~1 packages/common/js/canvas.js
# Look for any other demo mode related state management
rg "is_demo|isDemo" -A 2
Length of output: 5469
packages/canvas/package.json (1)
47-49: Review dependency versions and security implications
The addition of diff-match-patch and jsondiffpatch supports the schema change tracking functionality. However:
- Consider pinning exact versions for better reproducibility
- Verify these versions for known vulnerabilities
✅ Verification successful
Dependencies are secure, but consider version pinning
No security vulnerabilities found in either diff-match-patch@^1.0.5 or jsondiffpatch@^0.6.0. While the caret ranges allow for minor updates, the current versions are safe to use. However, for better build reproducibility in a monorepo context, consider pinning exact versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for known vulnerabilities in the new dependencies
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: NPM, package: "diff-match-patch") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}
'
gh api graphql -f query='
{
securityVulnerabilities(first: 10, ecosystem: NPM, package: "jsondiffpatch") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
}
}
}
'
Length of output: 589
packages/toolbars/preview/src/Main.vue (1)
31-31: LGTM: Improved method access pattern
The change from canvasApi to destructured methods aligns with the PR's goal of providing a unified entry point for schema operations.
packages/toolbars/redoundo/src/composable/useHistory.js (2)
98-98: LGTM: Consistent schema access pattern
The change to use useCanvas().getSchema() aligns with the unified schema access pattern.
64-64: Verify breaking change: setSchema → importSchema
This change from setSchema to importSchema is a breaking change that could affect existing applications. Please ensure all consumers of this API are updated accordingly.
✅ Verification successful
Migration from setSchema to importSchema is correct and safe
The change from setSchema to importSchema in the history implementation is correct and aligned with the codebase:
setSchemais only used internally in the render package and is not exposed as a public APIimportSchemais the official public API method exposed throughuseCanvas()and is already being used across multiple components- The history functionality remains intact as both methods handle schema updates appropriately
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining uses of setSchema that might need updating
rg "setSchema" --type vue --type js
Length of output: 63
Script:
#!/bin/bash
# Search for any remaining uses of setSchema in all files
rg "setSchema"
# Also search for importSchema to understand the migration pattern
rg "importSchema"
Length of output: 861
Script:
#!/bin/bash
# Let's check the implementation of both methods to understand the migration
ast-grep --pattern 'const setSchema = $_' -l js
ast-grep --pattern 'const importSchema = $_' -l js
# Get the actual implementation
rg -A 10 "const setSchema = " packages/canvas/render/src/RenderMain.js
rg -A 10 "const importSchema = " packages/canvas/DesignCanvas/src/api/useCanvas.js
Length of output: 6777
packages/configurator/src/collection-configurator/CollectionConfigurator.vue (2)
14-16: LGTM: Improved import organization
The reorganization of imports improves code readability by grouping related imports together.
7-7: Verify icon functionality after rename
The icon component has been renamed from icon-conment-refresh to icon-common-refresh. While this appears to be a typo fix, please verify that:
- The icon still displays correctly
- The click handler still works as expected
Also applies to: 15-15, 22-22
✅ Verification successful
Let me analyze the component registration and usage patterns to verify the icon functionality. The previous command failed because the file type was not recognized, so I've modified the approach to search for both old and new naming patterns, and also look for component registrations.
Let me try a different approach by searching for icon-related content more broadly and examining the specific file content to understand the component usage and imports.
Icon component is correctly renamed and imported
The verification shows that the icon component is properly handled:
- The import statement correctly aliases
iconConmentRefreshtoiconCommonRefreshfrom@opentiny/vue-icon - The component is properly registered as
IconCommonRefreshin the components section - The template uses the kebab-case version
icon-common-refreshconsistently - The click handler
@click="refreshData"is properly bound and therefreshDatamethod is implemented
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining instances of the old icon name
rg "iconConment" --type vue --type js
Length of output: 65
Script:
#!/bin/bash
# Search for any instances of the old or new icon name patterns
rg -t js -t vue "icon-co[mn][mn]ent-refresh|iconCo[mn][mn]entRefresh"
# Search for icon component registration patterns
ast-grep --pattern 'components: {
$$$
iconCommonRefresh: $_,
$$$
}'
Length of output: 175
Script:
#!/bin/bash
# Search for any files containing icon-related content
rg "icon.*refresh" -l
# Let's also check the specific file content
cat "packages/configurator/src/collection-configurator/CollectionConfigurator.vue"
# Check imports in the file
rg "import.*icon" "packages/configurator/src/collection-configurator/CollectionConfigurator.vue"
Length of output: 2974
packages/configurator/src/html-text-configurator/HtmlTextConfigurator.vue (2)
18-18: LGTM: Import aligns with new schema management approach
The addition of useCanvas import aligns with the PR's objective of providing a unified entry point for schema modifications.
47-52: Verify schema update propagation
The transition to using operateNode for component name updates aligns with the new schema management approach. However, we should verify that schema change events are properly propagated to dependent components like the outline tree.
✅ Verification successful
Let's gather more information about how schema changes are handled in the codebase, particularly focusing on the outline tree component.
Let's try a different approach to check the event handling and schema updates:
Schema updates are properly propagated through the event system
The verification shows that:
- The outline tree (packages/plugins/tree/src/Main.vue) properly subscribes to 'schemaChange' events
- When schema changes occur, the tree component updates its view using
filterSchema(pageState.pageSchema) - The
updateAttributesoperation in useCanvas correctly handles attribute updates and triggers the necessary state changes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for schema change event subscriptions in the codebase
# This helps verify that components like the outline tree are properly listening for schema updates
rg -l "schemaChange|useCanvas\(\).subscribe"
Length of output: 359
Script:
#!/bin/bash
# Check the implementation of schema change subscriptions in the tree plugin
ast-grep --pattern 'useCanvas().subscribe($$$)' packages/plugins/tree/src/Main.vue
# Also check how the schema plugin handles these updates
ast-grep --pattern 'useCanvas().subscribe($$$)' packages/plugins/schema/src/Main.vue
# Look for updateAttributes handler in useCanvas
ast-grep --pattern 'updateAttributes($$$)' packages/canvas/DesignCanvas/src/api/useCanvas.js
Length of output: 266
Script:
#!/bin/bash
# Check for event handling patterns in tree plugin
rg -A 5 "useCanvas\(\)" packages/plugins/tree/src/Main.vue
# Check schema update handling in useCanvas
rg -A 10 "updateAttributes" packages/canvas/DesignCanvas/src/api/useCanvas.js
# Look for schema change subscriptions with different pattern
rg -A 5 "subscribe\(" packages/plugins/tree/src/Main.vue
Length of output: 1879
packages/settings/props/src/components/modal/ModalContent.vue (2)
48-48: LGTM: State management transition
The transition from resState to appSchemaState aligns with the centralized schema management approach.
68-70: Verify i18n fallback behavior
The i18n handling logic has been updated to use appSchemaState. Ensure proper fallback behavior when translations are missing.
✅ Verification successful
Let me gather more information about the i18n fallback behavior by checking the implementation and usage patterns.
Based on the gathered information, I can now provide a final response about the i18n fallback behavior:
The i18n fallback implementation is correctly handled
The code follows the established i18n pattern across the codebase where:
- Primary lookup is done via
appSchemaState.langs[i18nKey][appSchemaState.currentLang] - Fallback to
props.values.title[appSchemaState.currentLang]when the i18n key is not found - Further fallback to
props.values.titlefor non-i18n values
This matches the i18n handling patterns found in other components like MetaListItems.vue and is properly integrated with the centralized i18n state management.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for i18n fallback handling patterns across the codebase
# This helps verify consistent i18n handling
rg -A 3 "langs\[.*?\]\[.*?currentLang"
Length of output: 795
Script:
#!/bin/bash
# Check for appSchemaState type definitions and initialization
ast-grep --pattern 'interface $_State {
$$$
langs: $$$
currentLang: $$$
$$$
}'
# Check for other i18n related patterns
rg -A 3 "currentLang" --type vue
Length of output: 149
Script:
#!/bin/bash
# Check for i18n fallback patterns without file type restriction
rg -A 5 "currentLang"
# Check for appSchemaState initialization or definition
rg -A 5 "appSchemaState.*=|const.*appSchemaState"
# Look for any i18n related configuration files
fd -e json -e js -e ts "i18n|lang"
Length of output: 22953
packages/configurator/src/related-editor-configurator/RelatedEditorConfigurator.vue (1)
Line range hint 102-107: Verify event propagation chain
The saveValue function publishes messages for related components. Ensure the event propagation chain is complete and all subscribers receive updates.
✅ Verification successful
Let me gather more information about the message publishing and subscription system to verify the complete event propagation chain.
Based on the analysis of the message system implementation, I can now provide the final response:
Event propagation chain is properly implemented
The saveValue function in RelatedEditorConfigurator.vue correctly publishes messages using the established message system. The verification shows:
- The component uses the standard
useMessagehook from the meta-register package - Messages are published with the correct topic format
lowcode.${componentName}.dataChange - The RelatedColumnsConfigurator component is properly subscribed to these messages, handling them in its
onMountedlifecycle hook - The message system supports proper unsubscription on component unmount
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for message subscribers in the component chain
# This helps verify the complete event propagation path
# Look for subscribers to the lowcode.<component>.dataChange topic
rg "lowcode\..+\.dataChange" -A 3
Length of output: 953
Script:
#!/bin/bash
# Search for useMessage hook usage and message subscriptions
rg "useMessage|subscribe\(" -A 3
# Search for components that might be listening to these messages
ast-grep --pattern 'onMounted(() => {
$$$
subscribe($$$)
$$$
})'
Length of output: 18381
packages/utils/src/constants/index.js (1)
92-92: Verify removal of SchemaLength from BROADCAST_CHANNEL
The removal of SchemaLength from BROADCAST_CHANNEL could impact existing code that relies on this broadcast channel.
✅ Verification successful
Removal of SchemaLength from BROADCAST_CHANNEL is safe
Based on the codebase analysis, there are no references to the SchemaLength broadcast channel. The existing BROADCAST_CHANNEL usage is limited to:
Notify: For error and warning notificationsCanvasLang: For language switching in canvasPreviewLang: For language switching in preview mode
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any remaining references to SchemaLength in broadcast channel usage
rg -l "SchemaLength|BROADCAST_CHANNEL\.SchemaLength"
# Search for BroadcastChannel initialization or usage patterns
ast-grep --pattern 'new BroadcastChannel($$$)'
Length of output: 103
Script:
#!/bin/bash
# Search for any references to SchemaLength in a broader context
rg -i "schemalength" -A 3 -B 3
# Search for broadcast channel usage patterns more broadly
rg "BROADCAST_CHANNEL\." -A 3 -B 3
# Look for files that might use broadcast channels
rg "BroadcastChannel" -A 3 -B 3
Length of output: 10421
packages/canvas/container/src/keyboard.js (1)
24-24: LGTM: Import of useCanvas aligns with new schema management approach
The addition of useCanvas import supports the unified schema modification entry point objective.
packages/common/component/MetaListItems.vue (1)
106-106: LGTM: Consistent state management variable renaming
The change from resState to appSchemaState aligns with the broader refactoring effort to unify schema state management across the application.
Also applies to: 162-162
packages/settings/props/src/composable/useProperties.js (1)
124-140: LGTM: Improved schema modification handling
The changes implement a more robust approach to property updates by:
- Using immutable state updates
- Centralizing schema modifications through
operateNode - Adding proper handling of property removal
packages/toolbars/save/src/js/index.js (2)
67-75: LGTM: Improved canvas state management
The changes properly implement context-aware state resets using the unified canvas API.
97-97: 🛠️ Refactor suggestion
Consider enhancing error handling for schema operations
The destructured getSchema could be undefined if useCanvas() fails. Consider adding a null check.
- const { isSaved, getSchema } = useCanvas()
+ const canvas = useCanvas()
+ if (!canvas) {
+ useNotify({
+ type: 'error',
+ message: 'Failed to initialize canvas'
+ })
+ return
+ }
+ const { isSaved, getSchema } = canvaspackages/configurator/src/slot-configurator/SlotConfigurator.vue (1)
88-92: LGTM: Schema update now uses the unified entry point
The change to use operateNode aligns with the PR's objective of providing a unified entry point for schema modifications. This approach is more maintainable and consistent.
Let's verify that all schema modifications use the new pattern:
packages/plugins/script/src/js/method.js (2)
79-84: LGTM: Well-structured method item creation
The method item creation follows a consistent pattern using the SCHEMA_DATA_TYPE constant, which improves type safety and maintainability.
119-124: LGTM: Consistent method creation with validation
The code properly validates method names before creation and maintains a consistent structure with the individual saveMethod function.
🧰 Tools
🪛 Biome (1.9.4)
[error] 121-122: Avoid the delete operator which can impact performance.
Unsafe fix: Use an undefined assignment instead.
(lint/performance/noDelete)
packages/plugins/materials/src/composable/useResource.js (1)
155-162: LGTM: Clean resource fetching implementation
The refactored fetchResource function properly separates concerns by delegating app state fetching to fetchAppState and maintains a clear event-driven architecture.
packages/common/component/BlockLinkField.vue (1)
51-51: LGTM! Schema access refactoring looks good.
The change to use destructured getSchema from useCanvas aligns with the PR's objective of providing a unified entry point for schema operations.
Let's verify consistent usage of this pattern across the codebase:
Also applies to: 54-54
✅ Verification successful
Schema access refactoring is consistently implemented
The codebase shows consistent usage of the new pattern where getSchema is destructured from useCanvas(). There are no remaining instances of the old canvasApi.value.getSchema pattern, and all files are using the new unified schema access approach. The change in BlockLinkField.vue aligns perfectly with the established pattern across the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining direct schema access through canvasApi
# Expected: No instances of old pattern (canvasApi.value.getSchema)
rg "canvasApi\.value\.getSchema" --type vue
Length of output: 73
Script:
#!/bin/bash
# Search for any remaining direct schema access through canvasApi, considering .vue extension
rg "canvasApi\.value\.getSchema" -g "*.vue"
# Also search for the new pattern to verify consistent usage
rg "const.*getSchema.*=.*useCanvas" -g "*.vue"
# Let's also check for any other potential schema access patterns
rg "getSchema.*=.*canvasApi" -g "*.vue"
Length of output: 1889
packages/common/component/BlockLinkEvent.vue (1)
Line range hint 41-46: LGTM! Schema access and event list mapping look good.
The schema access pattern is consistent with the unified approach, and the event list mapping is clean and well-structured.
packages/canvas/DesignCanvas/src/DesignCanvas.vue (2)
33-34: LGTM! Enhanced messaging capabilities.
The addition of useMessage and META_SERVICE improves the component's messaging and service integration capabilities.
189-191: LGTM! Enhanced controller capabilities.
The addition of ast, useModal, and useMessage to the controller properly encapsulates these functionalities for use in canvas/render and internal components.
packages/plugins/i18n/src/composable/useTranslate.js (1)
Line range hint 119-137: LGTM! Verify schema state access across the application.
The change from resState to appSchemaState aligns with the PR's objective of unifying schema access. The implementation correctly uses optional chaining for safe property access.
Let's verify consistent usage across the codebase:
✅ Verification successful
Let me check the remaining resState usage to ensure it's not related to our current changes.
LGTM! The schema state access is consistent across the application.
The verification shows that:
- The remaining
resStateusage inpackages/vue-generator/src/generator/vue/sfc/generateScript.jsis unrelated to our changes as it's in a different context. - All instances of
appSchemaState.langsaccess follow the same pattern with proper optional chaining. - The
useResourcehook is consistently imported and used across the codebase for accessing the schema state.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining instances of resState that might need updating
# and verify consistent usage of appSchemaState
# Search for remaining resState usage
echo "Checking for remaining resState usage..."
rg "resState" -l
# Verify consistent appSchemaState.langs access pattern
echo "Verifying consistent appSchemaState.langs access..."
rg "appSchemaState\\.langs" -A 2
Length of output: 1512
Script:
#!/bin/bash
# Check the context of resState usage in the remaining file
echo "Checking resState usage context..."
rg "resState" -A 2 packages/vue-generator/src/generator/vue/sfc/generateScript.js
# Also verify the useResource import and usage pattern
echo "Checking useResource usage..."
rg "useResource" -A 2
Length of output: 13803
packages/common/js/completion.js (1)
74-74: LGTM! Consistent schema state access.
The change maintains consistency with the unified schema access pattern and properly handles defaults for destructured properties.
packages/common/component/BlockDeployDialog.vue (1)
Line range hint 180-188: LGTM! Improved schema access pattern.
The change simplifies schema access by using the unified getSchema method, with proper null checking.
packages/configurator/src/layout-grid-configurator/LayoutGridConfigurator.vue (2)
39-39: LGTM: Import statement updated to use unified schema management.
The addition of useCanvas import aligns with the PR's objective of providing a unified entry point for schema modifications.
198-209: Good addition of error handling for schema operations.
The code properly checks for schema existence before proceeding with operations, preventing potential runtime errors.
Let's verify the schema error handling across the codebase:
✅ Verification successful
Schema error handling is consistently implemented across the codebase
The error handling pattern for schema validation in LayoutGridConfigurator.vue aligns with the codebase's overall approach. The search results show consistent schema validation across multiple components:
- Similar null checks (
if (!schema)) are implemented in various components like:useHistory.jsuseStyle.jsuseProperties.jsAdvanceConfig.vueCanvasMenu.vue
The error handling includes appropriate fallbacks, early returns, and error messages where necessary, making this implementation a good fit with the existing patterns.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for similar error handling patterns in schema operations
ast-grep --pattern 'if (!$schema) {
$$$
return
}'
Length of output: 55
Script:
#!/bin/bash
# Let's search for schema-related error handling patterns with more variations
rg -U "if.*!schema.*\{[\s\S]*?return" -g "*.{vue,js,ts,jsx,tsx}"
# Also search for console.error messages related to schema
rg "console.error.*schema" -g "*.{vue,js,ts,jsx,tsx}"
# Search for schema validation or checks
rg "(!schema|schema\s*===\s*null|schema\s*==\s*null|schema\s*===\s*undefined|schema\s*==\s*undefined)" -g "*.{vue,js,ts,jsx,tsx}"
Length of output: 21075
packages/toolbars/generate-code/src/Main.vue (1)
54-54: LGTM: Simplified schema retrieval using useCanvas.
The change aligns with the PR's objective of providing a unified entry point for schema access.
packages/canvas/render/src/render.js (3)
662-665: Good fix for empty children handling!
Returning null instead of an empty array for empty children prevents components from misinterpreting the absence of children as custom slots. This is particularly important for components like TinyTree.
726-732: LGTM! Consistent empty children handling.
The additional check to convert empty arrays to null in the render function maintains consistency with the getChildren implementation, providing a robust solution for handling empty children throughout the rendering pipeline.
Line range hint 620-673: LGTM! Verify parent-child relationships.
The removal of the parent parameter streamlines the rendering logic. The function maintains its core responsibilities while reducing parameter coupling.
Let's verify that parent-child relationships are still properly maintained:
✅ Verification successful
Based on the search results, I can now generate the final response:
Parent-child relationships are properly maintained without the parent parameter
The removal of the parent parameter from renderGroup is safe because:
- Parent-child relationships are managed through the schema tree structure itself, not through an explicit parent parameter
- The component hierarchy is maintained through:
- The nested structure of
childrenarrays in the schema - Scope inheritance via
mergeScopefor proper data context - Slot rendering logic that correctly handles parent context when needed (e.g., for Block components)
- The nested structure of
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential parent-child relationship issues
# Look for instances where parent context might be needed
# Search for parent references in component rendering
ast-grep --pattern 'parent: $_'
# Search for parent-dependent logic
rg -A 3 'parent\??\.'
Length of output: 12785
packages/canvas/render/src/RenderMain.js (1)
471-484: Verify that removed methods are not used elsewhere in the codebase
Several methods have been removed from the api export, including:
updateUtilsdeleteUtilsgetBridgesetBridgegetMethodsgetSchemasetSchemagetStatedeleteStatesetStategetPropssetPropsgetContextgetNodegetRootsetNodegetGlobalState
Ensure that these methods are not referenced elsewhere in the codebase to prevent runtime errors.
Run the following script to check for usages of the removed methods:
This script uses rg (ripgrep) to recursively search for occurrences of the removed methods, excluding the .git and node_modules directories.
packages/settings/styles/src/js/useStyle.js (2)
334-341: Good addition of initialization guard.
The hasWatchInitialized flag effectively prevents multiple initializations of watchers, avoiding potential memory leaks and performance issues.
190-257: Verify selector list generation logic.
The watch implementation for selector list generation is complex and might have edge cases.
✅ Verification successful
Let me gather more information about the selector list generation logic by examining the related functions and dependencies.
Let me check one more aspect to understand the complete flow of selector handling, particularly the getSelectorArr function and any validation logic.
Let me check the implementation of the parser module since it contains the critical getSelectorArr function.
After analyzing the implementation, I can now provide a final response about the selector list generation logic.
The selector list generation implementation is robust and well-handled
The implementation correctly handles:
- Individual class and ID selectors with proper prefix mapping (
.and#) - Combination selectors (e.g.,
.class1.class2) through proper parsing ingetSelectorArr - Mouse states (hover, pressed, focused, disabled)
- Input validation and edge cases:
- Empty or invalid selectors
- Unsupported selector types (attribute selectors, combinators)
- Null/undefined checks
The watch implementation follows a clear logic:
- Builds options from current class and ID lists
- Validates combination selectors against existing classes/IDs
- Maintains proper state synchronization with default selections
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Search for potential issues with selector list generation
# Test: Look for similar patterns in other style-related files
rg -A 5 "selectorOptionLists.*="
Length of output: 1711
Script:
#!/bin/bash
# Check for getSelectorArr function implementation and usage
ast-grep --pattern 'function getSelectorArr($_) {
$$$
}'
# Look for other watchers or computed properties related to selectors
rg -A 5 "watch\(\(\) => \[.*selector"
# Check for any other usage of currentClassNameList and currentIdList
rg "currentClassNameList|currentIdList" -A 3
Length of output: 2443
Script:
#!/bin/bash
# Look for getSelectorArr implementation and selector validation
rg "getSelectorArr" -B 2 -A 5
# Check for any selector-related utility functions
ast-grep --pattern 'function $_(selector) {
$$$
}'
# Look for any selector validation or error handling
rg "selector.*valid|valid.*selector" -A 3
Length of output: 3954
Script:
#!/bin/bash
# Check the complete implementation of parser.js
cat packages/settings/styles/src/js/parser.js
# Look for any error handling or validation in the parser
rg "throw|error|warning" packages/settings/styles/src/js/parser.js -A 3
Length of output: 5684
packages/plugins/materials/src/composable/useMaterial.js (1)
369-369: Documentation update reflects state management changes.
The comment update from 'resState' to 'appSchemaState' indicates a shift in state management approach. This change should be part of a broader refactoring effort.
✅ Verification successful
Documentation update is consistent with state management implementation.
Based on the search results, the change from 'resState' to 'appSchemaState' is consistent with the actual implementation in the codebase. The codebase shows extensive usage of appSchemaState across multiple files, particularly in the materials plugin and other components. There are no remaining references to resState.thirdPartyDeps, confirming this was a proper documentation update to match the actual state management approach being used.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify consistent usage of appSchemaState across the codebase
# Test: Look for any remaining references to resState
rg "resState\.thirdPartyDeps"
Length of output: 31
Script:
#!/bin/bash
# Let's search for both old and new state references more broadly
rg "resState" -A 2
rg "appSchemaState" -A 2
# Also check for any state management related imports
rg "import.*State" -A 2
Length of output: 20181
packages/configurator/src/container-configurator/ContainerConfigurator.vue
Show resolved
Hide resolved
There was a problem hiding this comment.
Actionable comments posted: 7
🧹 Outside diff range and nitpick comments (10)
packages/plugins/bridge/src/js/resource.js (4)
186-189: Use English for error messages.For consistency and internationalization purposes, error messages should be in English.
- message: '修改失败' + message: 'Modification failed'
197-197: Remove or update the orphaned comment.This comment about canvas refresh seems outdated since the code no longer directly interacts with the canvas.
- // 更新画布工具函数环境，保证渲染最新工具类返回值, 并触发画布的强制刷新
238-239: Use English for error messages.For consistency and internationalization purposes, error messages should be in English.
- message: '删除失败' + message: 'Deletion failed'
186-192: Consider centralizing notification handling.The success/error notifications are duplicated across different functions. Consider extracting them into a dedicated notification helper to improve maintainability and consistency.
Example implementation:
const notifyOperation = (success, operation) => { useNotify({ type: success ? 'success' : 'error', message: `${operation} ${success ? 'successful' : 'failed'}` }); }; // Usage notifyOperation(false, 'Modification'); notifyOperation(true, 'Deletion');Also applies to: 236-243
packages/canvas/render/src/builtin/CanvasCollection.js (1)
Line range hint
192-201: Add cleanup for updateKey subscriptionThe
TinyGridhandler should clean up theupdateKeysubscription when the component is unmounted.TinyGrid: ({ node, sourceRef, schemaId, pageSchema, updateKey }) => { const sourceName = sourceRef.value?.name const methodName = `${NAME_PREFIX.table}${schemaId}` + let updateKeySubscription = null node.props.fetchData = { type: 'JSExpression', value: `{ api: this.${methodName} }` } - const updateNode = () => updateNodeHandler({ node, sourceRef, pageSchema, sourceName, methodName, updateKey }) + const updateNode = () => { + updateKeySubscription = updateNodeHandler({ node, sourceRef, pageSchema, sourceName, methodName, updateKey }) + } const clearBindVar = () => { delete pageSchema.methods[methodName] + if (updateKeySubscription) { + updateKeySubscription.unsubscribe() + updateKeySubscription = null + } }packages/canvas/render/src/RenderMain.js (3)
365-371: Optimize schema update throttlingThe throttle implementation is good but could be improved by making the delay configurable and adding error handling.
-const throttleUpdateSchema = useThrottleFn( - () => { - window.host.patchLatestSchema(schema) - }, - 100, - true -) +const SCHEMA_UPDATE_DELAY = 100 +const throttleUpdateSchema = useThrottleFn( + () => { + try { + window.host.patchLatestSchema(schema) + } catch (error) { + console.error('Failed to patch schema:', error) + } + }, + SCHEMA_UPDATE_DELAY, + true +)
380-392: Consider adding cleanup for failed subscriptionsThe subscription setup looks good, but should handle potential subscription failures.
+const subscribeToTopic = (topic, subscriber, callback) => { + try { window.host.subscribe({ topic, subscriber, callback }) + } catch (error) { + console.error(`Failed to subscribe to ${topic}:`, error) + } +} -window.host.subscribe({ - topic: 'schemaChange', - subscriber: 'canvasRenderer', - callback: throttleUpdateSchema -}) +subscribeToTopic('schemaChange', 'canvasRenderer', throttleUpdateSchema) -window.host.subscribe({ - topic: 'schemaImport', - subscriber: 'canvasRenderer', - callback: () => { - setSchema(window.host.getSchema()) - } -}) +subscribeToTopic('schemaImport', 'canvasRenderer', () => { + setSchema(window.host.getSchema()) +})
426-473: Enhance watch handler cleanupThe watch handlers are properly set up with cleanup on unmount, but could benefit from error boundaries.
+const createWatchHandler = (getter, callback, options = {}) => { + try { + return window.host.watch(getter, callback, { + immediate: true, + deep: true, + ...options + }) + } catch (error) { + console.error('Failed to setup watch:', error) + return () => {} + } +} -const utilsWatchCanceler = window.host.watch( +const utilsWatchCanceler = createWatchHandler( () => window.host.appSchema?.utils, (data) => { setUtils(data) - }, - { - immediate: true, - deep: true } )packages/plugins/state/src/Main.vue (2)
Line range hint
168-172: Add error handling for schema operationsThe
addfunction should handle cases wheregetSchema()returns undefined to prevent silent failures.Consider this improvement:
const add = (name, variable) => { const { getSchema } = useCanvas() + const schema = getSchema() - if (getSchema()) { + if (!schema) { + notifySaveError('Failed to get schema') + return + } + if (updateKey.value !== name && flag.value === OPTION_TYPE.UPDATE) { delete state.dataSource[updateKey.value] } state.dataSource[name] = variable }
Line range hint
271-286: Improve security and error handling in lifecycle updatesThere are two concerns in the lifecycle update logic:
- The regex pattern could be vulnerable to ReDoS attacks
- Missing validation for
setup.valueexistenceConsider these improvements:
const schema = getSchema() let { lifeCycles } = schema const { [key]: deletedKey, ...restState } = schema.state if (key.startsWith('datasource')) { const pageSchema = getSchema() const { start, end } = getCommentByKey(key) + // Limit the input length to prevent ReDoS attacks + if (start.length > 100 || end.length > 100) { + notifySaveError('Invalid comment key length') + return + } const pattern = new RegExp(`([\\s\\n]*\\/\\*\\* ${start} \\*\\/[\\s\\S]*\\/\\*\\* ${end} \\*\\/)`) + + if (!pageSchema?.lifeCycles?.setup?.value) { + notifySaveError('Invalid lifecycle setup configuration') + return + } lifeCycles.setup.value = pageSchema.lifeCycles.setup.value.replace(pattern, '') }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (8)
packages/canvas/DesignCanvas/src/DesignCanvas.vue(4 hunks)packages/canvas/DesignCanvas/src/api/useCanvas.js(6 hunks)packages/canvas/render/src/RenderMain.js(9 hunks)packages/canvas/render/src/builtin/CanvasCollection.js(5 hunks)packages/configurator/src/container-configurator/ContainerConfigurator.vue(3 hunks)packages/plugins/bridge/src/js/resource.js(4 hunks)packages/plugins/i18n/src/composable/useTranslate.js(2 hunks)packages/plugins/state/src/Main.vue(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/plugins/i18n/src/composable/useTranslate.js
🔇 Additional comments (9)
packages/plugins/bridge/src/js/resource.js (1)
14-14: LGTM! Import changes align with centralization goals.
The addition of meta-register imports and removal of useCanvas aligns with the PR's objective of centralizing schema modifications.
packages/canvas/render/src/builtin/CanvasCollection.js (1)
114-141: 🛠️ Refactor suggestion
Internationalize messages and improve error handling
The implementation has hard-coded messages and could benefit from better error handling.
Additionally, consider adding specific error types and handling:
+const ERRORS = {
+ COLUMN_IMPORT: 'COLUMN_IMPORT_ERROR',
+ INVALID_DATA: 'INVALID_DATA_ERROR'
+}
try {
const sourceColumns = sourceRef.value?.data?.columns?.map(({ title, field }) => ({ title, field })) || []
+ if (!Array.isArray(sourceColumns)) {
+ throw new Error(ERRORS.INVALID_DATA)
+ }
node.props.columns = generateAssignColumns(sourceColumns, node.props.columns)
} catch (error) {
+ console.error('Failed to import columns:', error)
getController().useNotify({
type: 'error',
- message: '引入配置数据失败'
+ message: error.message === ERRORS.INVALID_DATA
+ ? i18n.t('canvas.errors.invalid_data')
+ : i18n.t('canvas.errors.import_failed')
})
}packages/plugins/state/src/Main.vue (1)
336-343: LGTM! Proper initialization of data source
The initialization logic correctly handles undefined cases by providing default values.
packages/configurator/src/container-configurator/ContainerConfigurator.vue (4)
7-7: Correct the event handler parameters for @update:modelValue
The event handler is still not properly handling the new value from the tiny-input component.
23-24: LGTM: Component imports and registration
The imports and component registration are properly configured for the tiny-input implementation.
Also applies to: 33-34
108-126: Optimize node reordering in the dragEnd function
The current implementation using delete and insert operations can be optimized.
128-132: 🛠️ Refactor suggestion
Add validation and error handling to onTitleUpdate
The method should validate the input and handle potential errors from schema operations.
-const onTitleUpdate = (value) => {
+const onTitleUpdate = (value) => {
+ if (!value?.id || !value?.props) {
+ useNotify({
+ type: 'error',
+ title: '更新标题失败',
+ message: '无效的节点数据'
+ })
+ return
+ }
+
const { operateNode } = useCanvas()
const id = value.id
-
- operateNode({ type: 'changeProps', id, value: { props: value.props } })
+ try {
+ operateNode({ type: 'changeProps', id, value: { props: value.props } })
+ } catch (error) {
+ useNotify({
+ type: 'error',
+ title: '更新标题失败',
+ message: error.message
+ })
+ }
}Likely invalid or redundant comment.
packages/canvas/DesignCanvas/src/DesignCanvas.vue (2)
33-35: LGTM: Added notification utilities
The addition of messaging utilities aligns with the PR's goal of improving error handling and user feedback.
190-193: LGTM: Enhanced controller capabilities
The addition of AST utilities and notification methods to the controller improves the overall functionality.
| const getNodeById = (id) => { | ||
| return nodesMap.value.get(id)?.node | ||
| } | ||
|
|
||
| const getNodeWithParentById = (id) => { | ||
| return nodesMap.value.get(id) | ||
| } | ||
|
|
||
| const delNode = (id) => { | ||
| nodesMap.value.delete(id) | ||
| } | ||
|
|
||
| const clearNodes = () => { | ||
| nodesMap.value.clear() | ||
| } | ||
|
|
||
| const setNode = (schema, parent) => { | ||
| schema.id = schema.id || utils.guid() | ||
|
|
||
| nodesMap.value.set(schema.id, { node: schema, parent }) | ||
| } | ||
|
|
||
| const getNode = (id, parent) => { | ||
| return parent ? nodesMap.value.get(id) : nodesMap.value.get(id)?.node | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for node operations
The node management functions need input validation to prevent potential errors.
const getNodeById = (id) => {
+ if (!id) {
+ console.warn('getNodeById called with invalid id')
+ return null
+ }
return nodesMap.value.get(id)?.node
}
const getNodeWithParentById = (id) => {
+ if (!id) {
+ console.warn('getNodeWithParentById called with invalid id')
+ return null
+ }
return nodesMap.value.get(id)
}
const delNode = (id) => {
+ if (!id || !nodesMap.value.has(id)) {
+ console.warn('delNode called with invalid or non-existent id')
+ return
+ }
nodesMap.value.delete(id)
}📝 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 getNodeById = (id) => { | |
| return nodesMap.value.get(id)?.node | |
| } | |
| const getNodeWithParentById = (id) => { | |
| return nodesMap.value.get(id) | |
| } | |
| const delNode = (id) => { | |
| nodesMap.value.delete(id) | |
| } | |
| const clearNodes = () => { | |
| nodesMap.value.clear() | |
| } | |
| const setNode = (schema, parent) => { | |
| schema.id = schema.id || utils.guid() | |
| nodesMap.value.set(schema.id, { node: schema, parent }) | |
| } | |
| const getNode = (id, parent) => { | |
| return parent ? nodesMap.value.get(id) : nodesMap.value.get(id)?.node | |
| } | |
| const getNodeById = (id) => { | |
| if (!id) { | |
| console.warn('getNodeById called with invalid id') | |
| return null | |
| } | |
| return nodesMap.value.get(id)?.node | |
| } | |
| const getNodeWithParentById = (id) => { | |
| if (!id) { | |
| console.warn('getNodeWithParentById called with invalid id') | |
| return null | |
| } | |
| return nodesMap.value.get(id) | |
| } | |
| const delNode = (id) => { | |
| if (!id || !nodesMap.value.has(id)) { | |
| console.warn('delNode called with invalid or non-existent id') | |
| return | |
| } | |
| nodesMap.value.delete(id) | |
| } | |
| const clearNodes = () => { | |
| nodesMap.value.clear() | |
| } | |
| const setNode = (schema, parent) => { | |
| schema.id = schema.id || utils.guid() | |
| nodesMap.value.set(schema.id, { node: schema, parent }) | |
| } | |
| const getNode = (id, parent) => { | |
| return parent ? nodesMap.value.get(id) : nodesMap.value.get(id)?.node | |
| } |
| // 获取传入的 schema 与最新 schema 的 diff | ||
| const getSchemaDiff = (schema) => { | ||
| return jsonDiffPatchInstance.diff(schema, pageState.pageSchema) | ||
| } | ||
|
|
||
| const patchLatestSchema = (schema) => { | ||
| // 这里 pageSchema 需要 deepClone，不然 patch 的时候，会 patch 成同一个引用，造成画布无法更新 | ||
| const diff = jsonDiffPatchInstance.diff(schema, deepClone(pageState.pageSchema)) | ||
|
|
||
| if (diff) { | ||
| jsonDiffPatchInstance.patch(schema, diff) | ||
| } | ||
| } | ||
|
|
||
| const importSchema = (data) => { | ||
| let importData = data | ||
|
|
||
| if (typeof data === 'string') { | ||
| try { | ||
| importData = JSON.parse(data) | ||
| } catch (error) { | ||
| // eslint-disable-next-line no-console | ||
| console.error('[useCanvas.importSchema] Invalid data') | ||
| } | ||
| } | ||
|
|
||
| // JSON 格式校验 | ||
| resetCanvasState({ | ||
| pageSchema: importData | ||
| }) | ||
| } | ||
|
|
||
| const exportSchema = () => { | ||
| return JSON.stringify(pageState.pageSchema) | ||
| } | ||
|
|
||
| const getSchema = () => { | ||
| return pageState.pageSchema || {} | ||
| } | ||
|
|
||
| const getNodePath = (id, nodes = []) => { | ||
| const { parent, node } = getNodeWithParentById(id) || {} | ||
|
|
||
| node && nodes.unshift({ name: node.componentName, node: id }) | ||
|
|
||
| if (parent) { | ||
| parent && getNodePath(parent.id, nodes) | ||
| } else { | ||
| nodes.unshift({ name: 'BODY', node: id }) | ||
| } | ||
|
|
||
| return nodes | ||
| } | ||
|
|
||
| const updateSchema = (data) => { | ||
| Object.assign(pageState.pageSchema, data) | ||
|
|
||
| publish({ topic: 'schemaChange', data: {} }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve schema diff and update handling
The schema management functions need better error handling and type checking.
const getSchemaDiff = (schema) => {
+ if (!schema || typeof schema !== 'object') {
+ console.error('Invalid schema provided to getSchemaDiff')
+ return null
+ }
return jsonDiffPatchInstance.diff(schema, pageState.pageSchema)
}
const patchLatestSchema = (schema) => {
+ if (!schema || typeof schema !== 'object') {
+ console.error('Invalid schema provided to patchLatestSchema')
+ return
+ }
const diff = jsonDiffPatchInstance.diff(schema, deepClone(pageState.pageSchema))
if (diff) {
+ try {
jsonDiffPatchInstance.patch(schema, diff)
+ } catch (error) {
+ console.error('Failed to patch schema:', error)
+ }
}
}📝 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.
| // 获取传入的 schema 与最新 schema 的 diff | |
| const getSchemaDiff = (schema) => { | |
| return jsonDiffPatchInstance.diff(schema, pageState.pageSchema) | |
| } | |
| const patchLatestSchema = (schema) => { | |
| // 这里 pageSchema 需要 deepClone，不然 patch 的时候，会 patch 成同一个引用，造成画布无法更新 | |
| const diff = jsonDiffPatchInstance.diff(schema, deepClone(pageState.pageSchema)) | |
| if (diff) { | |
| jsonDiffPatchInstance.patch(schema, diff) | |
| } | |
| } | |
| const importSchema = (data) => { | |
| let importData = data | |
| if (typeof data === 'string') { | |
| try { | |
| importData = JSON.parse(data) | |
| } catch (error) { | |
| // eslint-disable-next-line no-console | |
| console.error('[useCanvas.importSchema] Invalid data') | |
| } | |
| } | |
| // JSON 格式校验 | |
| resetCanvasState({ | |
| pageSchema: importData | |
| }) | |
| } | |
| const exportSchema = () => { | |
| return JSON.stringify(pageState.pageSchema) | |
| } | |
| const getSchema = () => { | |
| return pageState.pageSchema || {} | |
| } | |
| const getNodePath = (id, nodes = []) => { | |
| const { parent, node } = getNodeWithParentById(id) || {} | |
| node && nodes.unshift({ name: node.componentName, node: id }) | |
| if (parent) { | |
| parent && getNodePath(parent.id, nodes) | |
| } else { | |
| nodes.unshift({ name: 'BODY', node: id }) | |
| } | |
| return nodes | |
| } | |
| const updateSchema = (data) => { | |
| Object.assign(pageState.pageSchema, data) | |
| publish({ topic: 'schemaChange', data: {} }) | |
| } | |
| // 获取传入的 schema 与最新 schema 的 diff | |
| const getSchemaDiff = (schema) => { | |
| if (!schema || typeof schema !== 'object') { | |
| console.error('Invalid schema provided to getSchemaDiff') | |
| return null | |
| } | |
| return jsonDiffPatchInstance.diff(schema, pageState.pageSchema) | |
| } | |
| const patchLatestSchema = (schema) => { | |
| if (!schema || typeof schema !== 'object') { | |
| console.error('Invalid schema provided to patchLatestSchema') | |
| return | |
| } | |
| // 这里 pageSchema 需要 deepClone，不然 patch 的时候，会 patch 成同一个引用，造成画布无法更新 | |
| const diff = jsonDiffPatchInstance.diff(schema, deepClone(pageState.pageSchema)) | |
| if (diff) { | |
| try { | |
| jsonDiffPatchInstance.patch(schema, diff) | |
| } catch (error) { | |
| console.error('Failed to patch schema:', error) | |
| } | |
| } | |
| } | |
| const importSchema = (data) => { | |
| let importData = data | |
| if (typeof data === 'string') { | |
| try { | |
| importData = JSON.parse(data) | |
| } catch (error) { | |
| // eslint-disable-next-line no-console | |
| console.error('[useCanvas.importSchema] Invalid data') | |
| } | |
| } | |
| // JSON 格式校验 | |
| resetCanvasState({ | |
| pageSchema: importData | |
| }) | |
| } | |
| const exportSchema = () => { | |
| return JSON.stringify(pageState.pageSchema) | |
| } | |
| const getSchema = () => { | |
| return pageState.pageSchema || {} | |
| } | |
| const getNodePath = (id, nodes = []) => { | |
| const { parent, node } = getNodeWithParentById(id) || {} | |
| node && nodes.unshift({ name: node.componentName, node: id }) | |
| if (parent) { | |
| parent && getNodePath(parent.id, nodes) | |
| } else { | |
| nodes.unshift({ name: 'BODY', node: id }) | |
| } | |
| return nodes | |
| } | |
| const updateSchema = (data) => { | |
| Object.assign(pageState.pageSchema, data) | |
| publish({ topic: 'schemaChange', data: {} }) | |
| } |
| if (!node) { | ||
| node = pageState.pageSchema | ||
| } | ||
|
|
||
| if (!node.props) { | ||
| node.props = {} | ||
| } | ||
|
|
||
| if (overwrite) { | ||
| node.props = value.props | ||
| } else { | ||
| Object.assign(node.props, value?.props || {}) | ||
| } | ||
|
|
||
| return { | ||
| current: node, | ||
| previous | ||
| } | ||
| }, | ||
| updateAttributes: (operation) => { | ||
| const { id, value, overwrite } = operation | ||
| const { id: _id, children, ...restAttr } = value | ||
| const node = getNode(id) | ||
|
|
||
| // 其他属性直接浅 merge | ||
| Object.assign(node, restAttr) | ||
|
|
||
| // 配置了 overwrite，需要将没有传入的属性进行删除（不包括 children） | ||
| if (overwrite) { | ||
| const { id: _unUsedId, children: _unUsedChildren, ...restOrigin } = node | ||
| const originKeys = Object.keys(restOrigin) | ||
| const newKeysSet = new Set(Object.keys(restAttr)) | ||
|
|
||
| originKeys.forEach((key) => { | ||
| if (!newKeysSet.has(key)) { | ||
| delete node[key] | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| if (!Array.isArray(children)) { | ||
| // 非数组类型的 children，比如是直接的字符串作为 children | ||
| if (children || typeof children === 'string') { | ||
| node.children = children | ||
| } | ||
|
|
||
| return | ||
| } | ||
|
|
||
| const newChildren = children.map((item) => { | ||
| if (!item.id) { | ||
| item.id = utils.guid() | ||
| } | ||
|
|
||
| return item | ||
| }) | ||
| // 传了 children 进来，需要找出来被删除的、新增的，剩下的是修改的。 | ||
| const originChildrenIds = (node.children || []).filter(({ id }) => id).map(({ id }) => id) | ||
| const originChildrenSet = new Set(originChildrenIds) | ||
|
|
||
| const newChildrenSet = new Set(newChildren.map(({ id }) => id)) | ||
| // 被删除的项 | ||
| const deletedIds = originChildrenIds.filter((id) => !newChildrenSet.has(id)) | ||
| const deletedIdsSet = new Set(deletedIds) | ||
|
|
||
| for (const id of deletedIds) { | ||
| operationTypeMap.delete({ id }) | ||
| } | ||
|
|
||
| // 筛选出来新增的和修改的 | ||
| const changedChildren = newChildren.filter(({ id }) => !deletedIdsSet.has(id)) | ||
|
|
||
| changedChildren.forEach((childItem, index) => { | ||
| // 新增 | ||
| if (!originChildrenSet.has(childItem.id)) { | ||
| operationTypeMap.insert({ | ||
| parentId: id, | ||
| newNodeData: childItem, | ||
| position: 'after', | ||
| referTargetNodeId: changedChildren?.[index]?.id | ||
| }) | ||
| return | ||
| } | ||
|
|
||
| // 直接改引用插入进来，但是没有构建对应的 Map，需要构建Map | ||
| if (!getNode(childItem.id)) { | ||
| setNode(childItem, node) | ||
|
|
||
| // 递归构建 nodeMap | ||
| if (Array.isArray(childItem?.children) && childItem.children.length) { | ||
| const newNode = getNode(childItem.id) | ||
| generateNodesMap(childItem.children, newNode) | ||
| } | ||
| } | ||
|
|
||
| // 递归修改 | ||
| operationTypeMap.updateAttributes({ id: childItem.id, value: childItem }) | ||
| }) | ||
| } | ||
| } |
There was a problem hiding this comment.
Enhance error handling in operation handlers
The operation handlers need better error handling and validation.
const operationTypeMap = {
insert: (operation) => {
+ if (!operation?.parentId || !operation?.newNodeData) {
+ console.error('Invalid insert operation:', operation)
+ return {}
+ }
const { parentId, newNodeData, position, referTargetNodeId } = operation
// ... rest of the code
delete: (operation) => {
+ if (!operation?.id) {
+ console.error('Invalid delete operation:', operation)
+ return
+ }
const { id } = operation
// ... rest of the code
changeProps: (operation) => {
+ if (!operation?.id || !operation?.value) {
+ console.error('Invalid changeProps operation:', operation)
+ return { current: null, previous: null }
+ }
const { id, value, option: changeOption } = operation
// ... rest of the codeCommittable suggestion skipped: line range outside the PR's diff.
| const storeList = [...useResource().appSchemaState.globalState] || [] | ||
| const index = storeList.findIndex((store) => store.id === key) | ||
|
|
||
| if (index !== -1) { | ||
| const { id } = getMetaApi(META_SERVICE.GlobalService).getBaseInfo() | ||
|
|
||
| storeListt.splice(index, 1) | ||
| updateGlobalState(id, { global_state: storeListt }).then((res) => { | ||
| setGlobalState(res.global_state) | ||
| storeList.splice(index, 1) | ||
| updateGlobalState(id, { global_state: storeList }).then((res) => { | ||
| useResource().appSchemaState.globalState = res.global_state || [] |
There was a problem hiding this comment.
Add error handling for global state updates
The global state update lacks error handling and could be susceptible to race conditions.
Consider these improvements:
-const storeList = [...useResource().appSchemaState.globalState] || []
+const resource = useResource()
+const storeList = [...(resource.appSchemaState.globalState || [])]
const index = storeList.findIndex((store) => store.id === key)
if (index !== -1) {
const { id } = getMetaApi(META_SERVICE.GlobalService).getBaseInfo()
storeList.splice(index, 1)
- updateGlobalState(id, { global_state: storeList }).then((res) => {
- useResource().appSchemaState.globalState = res.global_state || []
- setGlobalStateToDataSource()
- })
+ updateGlobalState(id, { global_state: storeList })
+ .then((res) => {
+ resource.appSchemaState.globalState = res.global_state || []
+ setGlobalStateToDataSource()
+ })
+ .catch((error) => {
+ notifySaveError('Failed to update global state')
+ console.error('Global state update failed:', error)
+ })📝 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 storeList = [...useResource().appSchemaState.globalState] || [] | |
| const index = storeList.findIndex((store) => store.id === key) | |
| if (index !== -1) { | |
| const { id } = getMetaApi(META_SERVICE.GlobalService).getBaseInfo() | |
| storeListt.splice(index, 1) | |
| updateGlobalState(id, { global_state: storeListt }).then((res) => { | |
| setGlobalState(res.global_state) | |
| storeList.splice(index, 1) | |
| updateGlobalState(id, { global_state: storeList }).then((res) => { | |
| useResource().appSchemaState.globalState = res.global_state || [] | |
| const resource = useResource() | |
| const storeList = [...(resource.appSchemaState.globalState || [])] | |
| const index = storeList.findIndex((store) => store.id === key) | |
| if (index !== -1) { | |
| const { id } = getMetaApi(META_SERVICE.GlobalService).getBaseInfo() | |
| storeList.splice(index, 1) | |
| updateGlobalState(id, { global_state: storeList }) | |
| .then((res) => { | |
| resource.appSchemaState.globalState = res.global_state || [] | |
| setGlobalStateToDataSource() | |
| }) | |
| .catch((error) => { | |
| notifySaveError('Failed to update global state') | |
| console.error('Global state update failed:', error) | |
| }) |
| const schema = getSchema() | ||
| updateSchema({ state: { ...(schema.state || {}), [name]: variable } }) | ||
|
|
There was a problem hiding this comment.
Preserve existing schema properties during update
The current implementation might accidentally remove other schema properties during the update.
Consider this safer approach:
const schema = getSchema()
-updateSchema({ state: { ...(schema.state || {}), [name]: variable } })
+updateSchema({
+ ...schema,
+ state: { ...(schema.state || {}), [name]: variable }
+})📝 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 schema = getSchema() | |
| updateSchema({ state: { ...(schema.state || {}), [name]: variable } }) | |
| const schema = getSchema() | |
| updateSchema({ | |
| ...schema, | |
| state: { ...(schema.state || {}), [name]: variable } | |
| }) |
| const updateChildrenToValid = () => { | ||
| const schema = useProperties().getSchema() | ||
| const schemaChildren = schema.children || [] | ||
| let hasUpdate = false | ||
|
|
||
| const newChildren = schemaChildren.map((item) => { | ||
| if (!item.props) { | ||
| hasUpdate = true | ||
|
|
||
| item.props = { | ||
| title: '选项卡', | ||
| name: '' | ||
| } | ||
| } | ||
|
|
||
| return item | ||
| }) | ||
|
|
||
| const { operateNode } = useCanvas() | ||
|
|
||
| if (hasUpdate) { | ||
| operateNode({ type: 'updateAttributes', id: schema.id, value: { children: newChildren } }) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for schema operations
While the validation logic is good, the schema update operation should include error handling to gracefully handle failures.
const updateChildrenToValid = () => {
const schema = useProperties().getSchema()
const schemaChildren = schema.children || []
let hasUpdate = false
const newChildren = schemaChildren.map((item) => {
if (!item.props) {
hasUpdate = true
item.props = {
title: '选项卡',
name: ''
}
}
return item
})
const { operateNode } = useCanvas()
if (hasUpdate) {
+ try {
operateNode({ type: 'updateAttributes', id: schema.id, value: { children: newChildren } })
+ } catch (error) {
+ useNotify({
+ type: 'error',
+ title: '更新架构失败',
+ message: error.message
+ })
+ }
}
}Committable suggestion skipped: line range outside the PR's diff.
| const nodeSelected = (node, parent, type, id) => { | ||
| const { toolbars } = useLayout().layoutState | ||
| if (type !== 'clickTree') { | ||
| useLayout().closePlugin() | ||
| } | ||
|
|
||
| const { getSchema, getNodePath } = useCanvas().canvasApi.value | ||
| const { getSchema, getNodePath } = useCanvas() | ||
| const schemaItem = useCanvas().getNodeById(id) | ||
|
|
||
| const pageSchema = getSchema() | ||
|
|
||
| const schema = getSchema() | ||
| // 如果选中的节点是画布，就设置成默认选中最外层schema | ||
| useProperties().getProps(node || schema, parent) | ||
| useCanvas().setCurrentSchema(node || schema) | ||
| footData.value = getNodePath(node?.id) | ||
| useProperties().getProps(schemaItem || pageSchema, parent) | ||
| useCanvas().setCurrentSchema(schemaItem || pageSchema) | ||
| footData.value = getNodePath(schemaItem?.id) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation and null checks to nodeSelected
The method should validate the id parameter and handle potential null references.
-const nodeSelected = (node, parent, type, id) => {
+const nodeSelected = (node, parent, type, id) => {
const { toolbars } = useLayout().layoutState
if (type !== 'clickTree') {
useLayout().closePlugin()
}
+ if (!id) {
+ useNotify({
+ type: 'error',
+ title: '节点选择失败',
+ message: '无效的节点ID'
+ })
+ return
+ }
const { getSchema, getNodePath } = useCanvas()
const schemaItem = useCanvas().getNodeById(id)
const pageSchema = getSchema()
+ if (!schemaItem && !pageSchema) {
+ useNotify({
+ type: 'error',
+ title: '节点选择失败',
+ message: '未找到有效的架构'
+ })
+ return
+ }
useProperties().getProps(schemaItem || pageSchema, parent)
useCanvas().setCurrentSchema(schemaItem || pageSchema)
footData.value = getNodePath(schemaItem?.id)
toolbars.visiblePopover = false
}Committable suggestion skipped: line range outside the PR's diff.
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
【背景&问题】What is the current behavior?
【解决方案】What is the new behavior?
统一 schema 修改入口 + schema 变更事件发布订阅 + jsondiff 同步差异
【PR主要改动点】
修改 schema 统一入口。
以往修改 schema 的方法：直接
schema.props = xxx，即拿到引用直接进行修改当前 schema 修改的入口改为：
useCanvas().opeateNode(operation)主要用于操作节点删除、节点新增、节点 props 属性修改。useCanvas().updateSchema(data)主要用于更新 schema 的state、css、method、lifecycles等非节点属性。useCanvas().importSchema(data)主要用于页面、区块之间的切换。事件通知 schema 改变事件。
schemaChange事件。调用opeateNode以及updateSchema会触发该事件的发布。schemaImport事件。调用importSchema会触发该事件的发布。修改
utils、bridge、globalState、dataSourceMap等入口从直接修改画布改为直接修改useResource中的appSchemaState画布
RenderMain减少相关 schema 的操作接口（修改移动到统一的页面修改入口以及 appSchema 修改）setUtils、updateUtils、deleteUtils、getBridge、setBridge、getMethods、setMethods、getSchema、setSchema、getState、deleteState、setState、getProps、setProps、getContext、getRoot、getNode、setNode、getCondition、getGlobalState、setGlobalState、getDataSourceMap、setDataSourceMap画布对 schema 变更的状态的感知，改为 schema 相关修改事件的监听以及状态的订阅。
语义化重命名：
resState改为appSchemaState内置组件数据源 collection 改造：
组件内置的逻辑：刷新 children schema、检测 children 变更。（即
CanvasCollection.vue中的setup逻辑、canvasCollection.js，迁移到configurator/src/collection-configurator中 ）迁移过来之后遗留问题： configurator 中无法感知 children 的变化（需要手动点击刷新按钮），所以无法自动提示刷新 children 中的 相关数据源字段。
新旧版本皆有的遗留问题：数据源刷新了，表格数据无法刷新（需要刷新页面）
【PR 分拆任务】 （待确认）
由于本次 PR 改动基础 api。涉及的改动点较多，代码量也较大，为了避免单次PR合入过多特性，我们将一些新特性分拆到后续的 PR 中逐次迭代演进。简要梳理如下：
schemaChange事件记录堆栈。并支持batchChange批量事件的历史堆栈记录。（即 schema 历史堆栈自动化记录、配置化能力支持）。useProperties相关的 api 整改。整改为以node节点为基础的对象实例，自带增删查改的相关 api。Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
New Features
Bug Fixes
Documentation
Style
Chores