chore: sync bugfix of develop #922
Conversation
…one (opentiny#757) * fix: switch selected component event name list still show the origin one 修复画布切换选择组件后，高级面板事件列表仍然显示原组件事件列表的bug close opentiny#747 * fix: handle potential undefined values in renderEventList * fix: use locale instead of zh_CN attr
修复 mockService 区块详情缺失 data 嵌套结构导致无法正确加载的 bug
* fix: page or block save update breadcrumb uncorrectly 1. 修复页面名称更新时，面包屑页名称没有同步更新的 bug 2. 修复区块保存更新时，强行切换当前画布到正在编辑区块的 bug 3. 修复区块保存更新时，如果画布不是当前编辑的区块，仍取截图的 bug * fix: add handlePageUpdate mehtod missing params
…ntiny#660) * feat(vue-generator): add public floder, support image generation * feat(vue-generator): code add public floder, support image generation
* fix(block): 新建区块保存后发布区块，发布无效 * fix: del useless null check --------- Co-authored-by: chilingling <[email protected]>
* fix:table notch saving error message * fix: change slotparams cause warning * fix: change by review * fix: typo parma -> param --------- Co-authored-by: chilingling <[email protected]>
* fix: change namespace 2 export module * fix: add missing type params
* fix: blockName regexp incorrect * fix: change regexp by review * fix: change by review * fix: change by review comment
…tiny#847) 修复远程数据源面板请求结果输入框在编辑之后，重新点击发送请求，如果请求结果一样，无法重新填充到请求结果输入框的 bug
* fix: parse template name error * fix: add detection for test env
|
Warning Rate limit exceeded@chilingling has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 9 minutes and 31 seconds before requesting another review. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe pull request includes several modifications across various files, primarily focusing on enhancing functionality, improving error handling, and standardizing response formats. Notable changes involve the replacement of service references in Changes
Assessment against linked issues
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 (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (17)
packages/engine-cli/template/designer/public/mock/bundle.json (1)
10416-10416: Verify component export name casingThe export name has been changed from "PopEditor" to "Popeditor". While this works functionally, consider using consistent casing across component export names.
Consider using PascalCase for component export names to maintain consistency with Vue.js conventions:
- "exportName": "Popeditor", + "exportName": "PopEditor",packages/configurator/src/js-slot-configurator/JsSlotConfigurator.vue (4)
89-95: Enhance error message clarity.The validation logic is correct, but the error message could be more specific about the format requirements.
- callback(new Error('仅支持JavaScript中有效的变量名')) + callback(new Error('请输入有效的JavaScript变量名，多个参数用逗号分隔'))
97-120: Consider adding type checking for params.The new slot parameter management functions are well-structured and handle reactivity correctly. Consider adding type checking for the params parameter to enhance robustness.
- const setSlotParams = ({ name, params = '' }) => { + const setSlotParams = ({ name, params = '' }: { name: string; params?: string }) => {
122-157: Consider handling edge cases in toggleSlot.The toggle functionality is well-implemented, but consider:
- Validating componentName in the slot value
- Handling modal cancellation explicitly
slotInfo[name].params = params.split(',') } + // Validate component name + if (!slotInfo[name].value?.[0]?.componentName) { + console.warn('Invalid slot value: missing componentName') + return + } + updateSlotParams({ ...(props.modelValue || {}), ...slotInfo }) }
160-165: Add error handling for validation failures.Consider adding error handling to provide feedback when validation fails.
slotRef.value.validateField([paramsPath], (error) => { if (!error) { slot.bind && setSlotParams(slot) + } else { + console.warn('Slot parameter validation failed:', error) } })mockServer/src/services/app.js (1)
18-19: Consider adding error handling for service callsThe service reference update looks good, but consider adding try-catch blocks around the service calls to handle potential failures gracefully.
async lock(query) { const { id, state } = query - const occupier = state === 'occupy' ? mockService.pageService.userInfo : null - await mockService.pageService.update(id, { occupier }) + try { + const occupier = state === 'occupy' ? mockService.pageService.userInfo : null + await mockService.pageService.update(id, { occupier }) + return getResponseData({ + operate: 'success', + occupier + }) + } catch (error) { + return getResponseData({ error: error.message }, false) + } - return getResponseData({ - operate: 'success', - occupier - }) }mockServer/src/services/block.js (1)
75-76: Remove unnecessary empty lineThe empty line before the return statement is inconsistent with the code style in other methods.
async detail(blockId) { const result = await this.db.findOneAsync({ _id: blockId }) - return getResponseData(result) }packages/plugins/datasource/src/DataSourceRemoteDataResult.vue (1)
73-75: Consider adding error handling for JSON parsingThe
handleChangehandler should validate JSON format since this is used for data source configuration.const handleChange = (val) => { + try { + JSON.parse(val) state.value = val + } catch (e) { + useNotify({ + type: 'warning', + message: '请输入有效的 JSON 格式' + }) + } }packages/plugins/block/src/BlockSetting.vue (1)
220-224: Consider adding error handling for screenshot captureThe block screenshot capture should handle potential failures gracefully.
const currentId = useBlock().getCurrentBlock()?.id if (block.id === currentId) { // 获取区块截图 - block.screenshot = await getBlockBase64() + try { + block.screenshot = await getBlockBase64() + } catch (error) { + useNotify({ + type: 'warning', + message: '截图获取失败，将使用现有截图' + }) + } }packages/vue-generator/src/index.d.ts (2)
2-4: Consider replacing 'any' types with specific interfaces for better type safetyThe function
generateCodeusesanytypes forpageInfoand returns a result of type{ [key: string]: any }. Providing explicit interfaces instead ofanyenhances type safety and code clarity.
Line range hint
58-70: Enhance type definitions by specifying concrete types instead of using 'any' and 'object'In the
IAppSchemainterface, properties likei18n,utils,dataSource, andglobalStateuseRecord<string, any>orobject. Replacing these with concrete interfaces will improve type safety and maintainability.packages/vue-generator/src/plugins/formatCodePlugin.js (1)
32-32: LGTM! Consider adding return type annotation.The type import change improves type resolution. While we're updating types, consider adding a return type annotation to the
runmethod for completeness.- * @returns + * @returns {void}packages/vue-generator/src/plugins/parseSchemaPlugin.js (1)
Line range hint
33-41: Consider improving infinite loop protection.The current depth limit of 1000 could be made more explicit and configurable. Consider extracting it as a named constant or configuration option.
+ const MAX_PAGE_DEPTH = 1000; // Maximum depth for page hierarchy let depth = 0; - while (curParentId !== '0' && depth < 1000) { + while (curParentId !== '0' && depth < MAX_PAGE_DEPTH) {packages/vue-generator/src/plugins/genI18nPlugin.js (1)
Line range hint
24-24: Consider improving null safety handling.The optional chaining on
schema?.i18ncould be combined with a default empty object to simplify the code.- const i18n = schema?.i18n || [] + const i18n = schema?.i18n ?? {}packages/vue-generator/src/templates/vue-template/index.js (2)
41-70: LGTM! Consider enhancing error messages.The base64ToBlob implementation is robust with proper error handling. However, the error messages could be more descriptive.
Consider enhancing the error messages:
- throw new Error('Invalid base64 data') + throw new Error('Invalid base64 data: Missing MIME type in data URI') - throw new Error('Failed to decode base64 string') + throw new Error('Failed to decode base64 string: Invalid base64 encoding')
163-178: Track the FIXME comment as a technical debt item.The FIXME comment indicates a known issue with base64 data during testing that should be properly tracked and addressed.
Would you like me to create a GitHub issue to track this technical debt? The issue would focus on investigating and fixing the base64 data handling during vitest testing.
packages/plugins/page/src/PageSetting.vue (1)
Potential issues with page restoration edge cases
The code review reveals several concerns:
- There's no validation of
pageData.pagebefore using it as an ID, which could lead to undefined behavior if the page history item is malformed.- The deletion of fields using
deleteoperator is performed without checking if they exist.- While there's a confirmation dialog before restoration, there's no error handling in the restoration process itself.
Recommended changes:
- Add validation for
pageData.pagebefore assignment- Add error handling for the
updatePageoperation- Consider using object destructuring instead of field deletion to avoid potential side effects
🔗 Analysis chain
Line range hint
219-232: Verify page restoration edge cases.While the page restoration logic looks correct, we should verify the handling of edge cases:
- What happens if pageData.page is undefined?
- Are there any side effects from deleting the fields?
Let me gather more information about the page restoration flow and error handling.
Let me check one more aspect about the page data validation before making the final assessment.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for all usages of restorePage to understand potential edge cases rg -A 5 "restorePage" # Search for any error handling related to undefined page IDs ast-grep --pattern 'if ($page === undefined || $page === null) { $$$ }'Length of output: 2102
Script:
#!/bin/bash # Check the implementation of updatePage function and its error handling ast-grep --pattern 'updatePage($$$) { $$$ }' # Look for any validation or error handling in PageSetting.vue rg -A 5 "catch|error|throw" packages/plugins/page/src/PageSetting.vue # Check how pageData is structured when emitted from PageHistory ast-grep --pattern 'emit($_, $pageData)'Length of output: 488
Script:
#!/bin/bash # Check how pageData is validated or processed before restoration rg -B 5 -A 10 "pageData\.page" packages/plugins/page/src/PageSetting.vue # Look for any validation in the PageHistory component before emitting the data rg -B 5 -A 10 "emit\('restorePage'" packages/plugins/page/src/PageHistory.vueLength of output: 854
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
⛔ Files ignored due to path filters (1)
packages/vue-generator/src/templates/vue-template/templateFiles/public/favicon.icois excluded by!**/*.ico
📒 Files selected for processing (29)
mockServer/src/services/app.js(1 hunks)mockServer/src/services/block.js(1 hunks)packages/common/component/BlockDeployDialog.vue(2 hunks)packages/common/js/http.js(3 hunks)packages/common/js/verification.js(1 hunks)packages/configurator/src/js-slot-configurator/JsSlotConfigurator.vue(3 hunks)packages/engine-cli/template/designer/public/mock/bundle.json(1 hunks)packages/plugins/block/src/BlockSetting.vue(2 hunks)packages/plugins/block/src/Main.vue(2 hunks)packages/plugins/block/src/js/blockSetting.jsx(2 hunks)packages/plugins/datasource/src/DataSourceRecordList.vue(1 hunks)packages/plugins/datasource/src/DataSourceRemoteDataResult.vue(2 hunks)packages/plugins/page/src/PageSetting.vue(1 hunks)packages/settings/events/src/components/BindEvents.vue(7 hunks)packages/toolbars/save/src/js/index.js(1 hunks)packages/vue-generator/src/generator/generateApp.js(1 hunks)packages/vue-generator/src/index.d.ts(7 hunks)packages/vue-generator/src/plugins/formatCodePlugin.js(1 hunks)packages/vue-generator/src/plugins/genBlockPlugin.js(1 hunks)packages/vue-generator/src/plugins/genDataSourcePlugin.js(1 hunks)packages/vue-generator/src/plugins/genDependenciesPlugin.js(1 hunks)packages/vue-generator/src/plugins/genGlobalState.js(1 hunks)packages/vue-generator/src/plugins/genI18nPlugin.js(1 hunks)packages/vue-generator/src/plugins/genPagePlugin.js(1 hunks)packages/vue-generator/src/plugins/genRouterPlugin.js(1 hunks)packages/vue-generator/src/plugins/genTemplatePlugin.js(1 hunks)packages/vue-generator/src/plugins/genUtilsPlugin.js(1 hunks)packages/vue-generator/src/plugins/parseSchemaPlugin.js(1 hunks)packages/vue-generator/src/templates/vue-template/index.js(3 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/vue-generator/src/plugins/genGlobalState.js
🔇 Additional comments (29)
packages/engine-cli/template/designer/public/mock/bundle.json (1)
Line range hint 1-10700: Component configurations look good
The component definitions are well-structured with proper schema definitions for properties, events, and slots. The configurations follow a consistent pattern across all components.
packages/configurator/src/js-slot-configurator/JsSlotConfigurator.vue (2)
22-22: LGTM! Function names corrected for better clarity.
The typos in function names have been fixed (parmas → params), improving code readability while maintaining the same functionality.
Also applies to: 28-28
178-179: LGTM! Exports updated correctly.
The exports have been updated to reflect the renamed functions.
packages/common/js/verification.js (1)
17-17:
Breaking Change: More Restrictive Block Name Validation
The updated regex pattern enforces stricter PascalCase naming with at least two segments (e.g., HelloWorld, UserProfile). This change could potentially break existing block names that were valid under the previous pattern.
Let's verify the impact on existing block names:
✅ Verification successful
Let's also check if there are any test files or documentation that might reference block names:
And let's specifically look at the usage of this regex pattern:
Block Name Pattern Change is Safe to Proceed
Based on the verification results, the regex change appears to be aligned with existing usage and won't cause breaking changes:
-
The REGEXP_BLOCK_NAME is only used in two places:
- SaveNewBlock.vue: For validating new block names during creation
- verification.js: The pattern definition and its verification method
-
Existing block names found in the codebase (e.g., 'PortalHeader', 'PortalBlock', 'PortalHome', 'BlockFileName') already follow the new PascalCase pattern with two or more segments.
-
The change actually better enforces the validation message that's already in the UI: "两个单词以上, 且是大写开头驼峰格式" (Two or more words in PascalCase format).
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for block names that would be invalid under the new regex
# Search for block definitions in the codebase and analyze their names
# Find potential block definition files
fd -e js -e jsx -e vue . | while read -r file; do
# Extract and check block names against both old and new patterns
rg -i 'block.*name.*["\']([^"\']+)["\']' "$file" -o -r '$1' | while read -r name; do
if ! echo "$name" | grep -qP '^([A-Z][a-z0-9]*){2,}$'; then
echo "File: $file"
echo "Potentially invalid block name under new regex: $name"
fi
done
done
Length of output: 269
Script:
#!/bin/bash
# Let's try a simpler approach to find block name patterns
# Search for string patterns that look like block name assignments
# Using ripgrep to find potential block name assignments
rg -i "block.*name.*['\"]([^'\"]+)['\"]" -g "*.{js,jsx,vue}" --no-filename -o
Length of output: 664
Script:
#!/bin/bash
# Search in test files and documentation
rg -i "block" -g "*.{test.js,spec.js,md}" -C 2
Length of output: 12780
Script:
#!/bin/bash
# Find where REGEXP_BLOCK_NAME is used
rg "REGEXP_BLOCK_NAME" -C 2
Length of output: 1465
packages/plugins/datasource/src/DataSourceRemoteDataResult.vue (1)
13-19: LGTM: Monaco editor setup looks good
The Monaco editor component is properly configured with reactive bindings and change handler.
packages/common/component/BlockDeployDialog.vue (2)
84-84: LGTM: Import statement is properly organized
The import statement correctly includes all necessary functions from the meta register.
Line range hint 158-163: Verify block retrieval fallback behavior
The fallback to useBlock().getCurrentBlock() looks good, but we should verify it handles all edge cases.
✅ Verification successful
Based on the search results, I can now generate the final response:
The block retrieval fallback mechanism is properly implemented and safe
The fallback chain getEditBlock() || useBlock().getCurrentBlock() is well-designed because:
getEditBlock()is a simple state getter that returnsstate.blockgetCurrentBlock()is a reliable fallback that returns the current block from the block state- Both functions are used extensively throughout the codebase with proper null checks (note the
?.operators in usage) - The fallback pattern is consistently used in other parts of the codebase for similar scenarios
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential null/undefined handling in block retrieval
ast-grep --pattern 'getEditBlock()'
Length of output: 334
Script:
#!/bin/bash
# Let's check the implementation of getEditBlock() and getCurrentBlock()
ast-grep --pattern 'function getEditBlock() {
$$$
}'
# Also check getCurrentBlock implementation
ast-grep --pattern 'function getCurrentBlock() {
$$$
}'
# Let's also check for any error handling around these functions
rg "getEditBlock|getCurrentBlock" -A 3
Length of output: 21009
packages/plugins/block/src/BlockSetting.vue (1)
85-85: LGTM: Import statement is properly organized
The import of getMergeMeta and useBlock is correctly structured.
packages/settings/events/src/components/BindEvents.vue (3)
20-20: LGTM: Event list display improvements
The changes correctly address the event list display bug by:
- Using
renderEventListfor consistent event iteration - Properly handling localized event labels
- Safely accessing nested properties with optional chaining
Also applies to: 25-25, 42-42
126-126: Verify data consistency between customEvents and componentEvent
The refactoring from componentEvents to customEvents and the introduction of renderEventList looks good. However, let's verify there are no edge cases where events might be lost during the merge.
Also applies to: 133-133
✅ Verification successful
No data consistency issues found between customEvents and componentEvents
The code analysis shows a clean implementation where:
customEventsis initialized withcommonEventsrenderEventListproperly merges both sources using computed property:{ ...state.componentEvent, ...state.customEvents }- The
AddEventsDialogcomponent validates for duplicate event names before adding new ones - The event handling and merging logic maintains data consistency throughout the components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for any other uses of componentEvents to ensure complete migration
rg -A 3 "componentEvents" --type vue
Length of output: 64
Script:
#!/bin/bash
# Search for any uses of componentEvents in all files
rg -A 3 "componentEvents"
Length of output: 1042
Script:
#!/bin/bash
# Search for any uses of customEvents in all files
rg -A 3 "customEvents"
Length of output: 1095
Script:
#!/bin/bash
# Find all .vue files and search their content
fd -e vue -x cat {} | grep -A 3 "componentEvents\|customEvents"
Length of output: 800
143-143: Ensure proper cleanup of event bindings
The event iteration now uses renderEventList.value, but we should verify that old event bindings are properly cleaned up when switching components.
✅ Verification successful
Let me gather more information about the event binding and cleanup logic.
Let me search for the actual BindEvents component content since the previous search found a parent component.
Let me search for any cleanup or unmount related code since we now have the component content.
After analyzing the code and verification results, I can now provide the final response:
Event bindings are properly managed through reactive state
The event bindings are automatically cleaned up through Vue's reactivity system:
- The
watchEffecthook manages the event bindings by synchronizing withrenderEventList.valueandpageState.currentSchema - When switching components, the
watchEffectautomatically re-executes and updatesstate.bindActions, effectively cleaning up old bindings - The event bindings are stored in component-scoped reactive state that is automatically cleaned up when components are unmounted
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for component switching logic to verify cleanup
ast-grep --pattern 'useCanvas().canvasApi.value.getSchema()'
Length of output: 162
Script:
#!/bin/bash
# Search for event cleanup related code in BindEvents.vue
ast-grep --pattern 'function $name($$$) {
$$$
removeEventListener($$$)
$$$
}'
# Also search for any unmount or beforeDestroy hooks
ast-grep --pattern 'onBeforeUnmount($$$)'
# Look for the full content of BindEvents.vue to understand the context
rg -l "BindEvents.vue" | xargs cat
Length of output: 800
Script:
#!/bin/bash
# Search for the actual BindEvents.vue component content
fd "BindEvents.vue" --exec cat {}
# Also search for any watch or effect related to renderEventList
ast-grep --pattern 'watch($$$renderEventList$$$)'
# Look for cleanup in setup or mounted hooks
ast-grep --pattern 'onMounted($$$)'
ast-grep --pattern 'setup($$$)'
Length of output: 22425
Script:
#!/bin/bash
# Search for onBeforeUnmount or cleanup related code
ast-grep --pattern 'onBeforeUnmount($$$)'
# Look for any cleanup in watchEffect
ast-grep --pattern 'watchEffect($$$)'
# Search for any event removal or cleanup patterns
rg "removeEventListener|cleanup|dispose" -A 2
Length of output: 7679
packages/plugins/block/src/Main.vue (1)
293-294: LGTM: Fixed typo in variable name
Good cleanup: renamed isEdite to isEdit for better code readability.
Also applies to: 297-297, 309-309
packages/plugins/datasource/src/DataSourceRecordList.vue (1)
476-476: LGTM: Improved batch delete button state management
Good addition: The button state is now properly updated after batch delete operation.
packages/plugins/block/src/js/blockSetting.jsx (1)
Line range hint 438-442: LGTM! Improved null safety handling
The addition of optional chaining for public_scope_tenants access is a good defensive programming practice that prevents potential runtime errors.
packages/vue-generator/src/plugins/genTemplatePlugin.js (1)
21-21: Confirmed update to use 'options.template' instead of 'context.template'
The template source has been correctly updated to use options.template rather than context.template, ensuring that the template selection is properly configurable through plugin options.
packages/vue-generator/src/plugins/genPagePlugin.js (1)
18-18: Updated JSDoc type annotation for clarity
The type annotation for the schema parameter now explicitly imports IAppSchema from @opentiny/tiny-engine-dsl-vue, enhancing type clarity and consistency across the codebase.
packages/vue-generator/src/plugins/genDataSourcePlugin.js (1)
18-18: Updated JSDoc type annotation for consistency
The schema parameter type has been updated to import IAppSchema from @opentiny/tiny-engine-dsl-vue, aligning with type definitions used in other plugins.
packages/vue-generator/src/plugins/genBlockPlugin.js (1)
18-18: Consistent type annotation update for 'schema' parameter
The schema parameter now imports IAppSchema, maintaining consistency across the codebase and improving type safety.
packages/vue-generator/src/plugins/parseSchemaPlugin.js (1)
10-10: LGTM! Type import change is consistent.
The change to use imported type improves consistency across the codebase.
packages/vue-generator/src/plugins/genI18nPlugin.js (2)
20-20: LGTM! Type import change aligns with codebase standards.
The change to use imported type maintains consistency with other plugins.
Line range hint 1-1: Verify PR objectives alignment.
The changes in these files only update type imports and don't appear to address the event list bug mentioned in the PR objectives (#747). Please verify if:
- There are additional files in the PR that contain the actual bug fixes
- These type changes are prerequisites for the bug fixes
- The PR description needs to be updated to reflect these type-only changes
packages/vue-generator/src/plugins/genUtilsPlugin.js (1)
45-45: LGTM: Type annotation update is consistent
The change to use an imported type reference improves type clarity and matches similar updates across other files.
✅ Verification successful
Type annotation update is consistently applied across plugin files
The verification confirms that the imported type import('@opentiny/tiny-engine-dsl-vue').IAppSchema is consistently used across all plugin files in the packages/vue-generator/src/plugins/ directory, including:
- formatCodePlugin.js
- genUtilsPlugin.js
- genRouterPlugin.js
- genPagePlugin.js
- genI18nPlugin.js
- genGlobalState.js
- parseSchemaPlugin.js
- genDependenciesPlugin.js
- genBlockPlugin.js
- genDataSourcePlugin.js
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify consistent type annotation updates across related files
rg -l "import\('@opentiny/tiny-engine-dsl-vue'\)\.IAppSchema" packages/vue-generator/src/plugins/
Length of output: 651
packages/vue-generator/src/plugins/genRouterPlugin.js (1)
39-39: LGTM: Consistent type annotation update
The type annotation change aligns with the standardization across the codebase.
packages/common/js/http.js (2)
61-62: Good defensive programming with optional chaining
The addition of optional chaining operator for updateTreeData prevents potential runtime errors when the method is undefined.
40-40: Verify impact of new parameter
The addition of isCurEditPage parameter changes the function signature. Ensure all callers are updated.
✅ Verification successful
All callers have been updated with the new parameter
Both occurrences of handlePageUpdate calls in the codebase are already passing the new isCurEditPage parameter:
packages/toolbars/save/src/js/index.js: Passestrueas the fourth argumentpackages/plugins/page/src/PageSetting.vue: PassesisCurEditPageparameter directly
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Find all calls to handlePageUpdate
rg "handlePageUpdate\(" --type js
Length of output: 272
packages/vue-generator/src/generator/generateApp.js (1)
18-19: LGTM: Type definitions properly imported
The update to import types from '@opentiny/tiny-engine-dsl-vue' improves type safety and documentation clarity.
packages/vue-generator/src/plugins/genDependenciesPlugin.js (1)
66-66: LGTM: Consistent type definition update
The schema parameter type is properly updated to use imported type from '@opentiny/tiny-engine-dsl-vue', maintaining consistency with other files.
packages/toolbars/save/src/js/index.js (1)
61-61: 🛠️ Refactor suggestion
Document the boolean parameters and improve readability
The addition of boolean parameters false, true to handlePageUpdate lacks clarity and documentation. Consider using named variables to make the intent clear:
- await handlePageUpdate(currentPage.id, { ...currentPage, ...params }, false, true)
+ const isForceUpdate = false;
+ const isCurEditPage = true;
+ await handlePageUpdate(
+ currentPage.id,
+ { ...currentPage, ...params },
+ isForceUpdate,
+ isCurEditPage
+ );Let's verify the usage of these parameters across the codebase:
✅ Verification successful
Let me gather more information about the handlePageUpdate function implementation and its documentation.
Let me try again with correct file type flags:
Based on the search results, I can now provide a final response:
Document the boolean parameters and improve readability
The handlePageUpdate function is imported from @opentiny/tiny-engine-common/js/http and is used in both the save toolbar and page settings. Looking at PageSetting.vue, we can see that:
routerChangeindicates if the page's route has changedisCurEditPageindicates if the update is for the currently edited page
Suggested improvement:
- await handlePageUpdate(currentPage.id, { ...currentPage, ...params }, false, true)
+ const hasRouteChanged = false;
+ const isCurrentEditPage = true;
+ await handlePageUpdate(
+ currentPage.id,
+ { ...currentPage, ...params },
+ hasRouteChanged,
+ isCurrentEditPage
+ );🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other calls to handlePageUpdate to understand parameter usage
rg -A 2 "handlePageUpdate\("
# Search for documentation of handlePageUpdate
ast-grep --pattern 'function handlePageUpdate($_) {
$$$
}'
Length of output: 530
Script:
#!/bin/bash
# Search for handlePageUpdate function definition and its documentation
rg -B 5 -A 10 "handlePageUpdate" --type ts --type js --type vue
# Search for any JSDoc or comment blocks related to handlePageUpdate
rg -B 3 "handlePageUpdate" --type ts --type js --type vue
Length of output: 177
Script:
#!/bin/bash
# Search for handlePageUpdate function definition and its documentation
rg -B 5 -A 10 "handlePageUpdate"
# Look for any exports or declarations of handlePageUpdate
ast-grep --pattern 'export $_$ handlePageUpdate'
# Look for any JSDoc comments above handlePageUpdate
rg -B 3 "@param.*handlePageUpdate|/\*\*.*handlePageUpdate"
Length of output: 5812
packages/plugins/page/src/PageSetting.vue (1)
213-215: LGTM! The isCurEditPage check aligns with PR objectives.
The addition of isCurEditPage parameter helps track the currently edited page, which is essential for fixing the event list display bug mentioned in issue #747.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/settings/events/src/components/BindEvents.vue (2)
118-118: Add safety check for commonEvents destructuringWhile the changes look good overall, consider adding a safety check for the commonEvents destructuring to handle potential undefined values.
- const { commonEvents = {} } = getMergeMeta('engine.setting.event').options + const { options = {} } = getMergeMeta('engine.setting.event') || {} + const { commonEvents = {} } = optionsAlso applies to: 126-126, 133-133
Line range hint
231-248: Consider improving event addition structureThe handleAddEvent method could be improved to:
- Validate event name uniqueness
- Support multiple locales
- Use a more maintainable event structure
const handleAddEvent = (params) => { const { eventName, eventDescription } = params + + if (renderEventList.value[eventName]) { + // Consider showing a warning to the user + return + } Object.assign(state.customEvents, { [eventName]: { label: { - zh_CN: eventDescription + [locale]: eventDescription }, description: { - zh_CN: `${eventDescription}的回调函数` + [locale]: `${eventDescription}的回调函数` }, type: 'event', functionInfo: { params: [], returns: {} }, defaultValue: '' } })
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
packages/settings/events/src/components/BindEvents.vue(7 hunks)
🔇 Additional comments (4)
packages/settings/events/src/components/BindEvents.vue (4)
20-25: LGTM: Event list rendering improvements
The changes correctly address the event list update issue by using the computed renderEventList property and adding proper locale support. The optional chaining operators (?.) appropriately handle potential undefined values.
Line range hint 143-177: LGTM: Improved event list synchronization
The watchEffect implementation correctly synchronizes the event list with the currently selected component, addressing the core issue from PR #747.
259-261: LGTM: Necessary exports added
The addition of renderEventList and locale to the component's exports is correct and necessary for the template functionality.
135-142: Verify the fix for component switching
The implementation looks correct, but let's verify that the event list properly updates when switching components.
✅ Verification successful
Component switching implementation verified successfully
The implementation in BindEvents.vue correctly handles component switching through a robust watchEffect that:
- Properly retrieves the new component's schema and events when component changes
- Safely handles undefined values with optional chaining
- Correctly merges component events with custom events via the
renderEventListcomputed property - Properly resets and rebuilds the
bindActionsstate when switching components
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for other potential component switching issues in the codebase
# Look for similar patterns of component switching and event list updates
rg -l "currentSchema.*componentName" --type vue
# Check for any related issues or PRs
gh issue list --search "in:title event list component switch"
gh pr list --search "in:title event list component switch"
Length of output: 202
Script:
#!/bin/bash
# Search for component switching patterns using ripgrep without file type filter
rg -l "currentSchema.*componentName"
# Search for watchEffect usage with component switching
rg "watchEffect.*currentSchema" -A 5
# Look for event-related component switching logic
rg "componentEvent.*schema" -A 3
Length of output: 686
Script:
#!/bin/bash
# Check the implementation of BindEvents.vue to understand the component switching logic
cat packages/settings/events/src/components/BindEvents.vue
# Look for any related test files
fd "BindEvents.test" -t f
fd "BindEvents.spec" -t f
# Check for any material-related utility functions
rg "getMaterial" -A 3
Length of output: 21617
…one (#757)
修复画布切换选择组件后，高级面板事件列表仍然显示原组件事件列表的bug
close #747
fix: handle potential undefined values in renderEventList
fix: use locale instead of zh_CN attrEnglish | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Release Notes
New Features
DataSourceRecordList.vue.Improvements
BindEvents.vue.JsSlotConfigurator.vue.Bug Fixes