-
Notifications
You must be signed in to change notification settings - Fork 354
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
feat: support building customized canvas entry #846
Conversation
* fix: 修复数据源mock数据不一致 * fix:修复构建物料资产包重复组件
* fix:修改css文件cdn地址
* fix: 修复数据源mock数据不一致 * fix:插件面板固定后画布未自适应 * fix:插件面板固定后画布未自适应 * fix:修复eslint报错
* feat(monaco-editor-completion): 隔离不同代码编辑器自动补全的注册,变量声明增加自动补全去掉js函数和state变量的自动补全提示 * feat(state): 状态管理使用JS表达式创建变量的时候提示不可使用其他变量以及JS函数
* fix: 修复数据源mock数据不一致 * fix:新增组件写入sql数据问题
* fix:修复资源管理工具类型切换
* fix(styles-spacing): 增大物料设置内外边距时点击区域 opentiny#134
* add config * update config
* feat: JS面板支持JSX语法 * fix: 画布解析JS表达式时支持JSX语法 * fix: 修复画布解析JSX表达式时会返回undefined * feat: 添加Tree和Tooltip的自定义渲染函数属性 * fix: 修改变量名
* feat(setting-stylePanel): 样式面板编辑全局样式新UI与交互 * feat(setting-stylePanel): 样式面板编辑全局样式新UI与交互 * feat(setting-stylePanel): 样式面板编辑全局样式新UI与交互-细节优化 * feat(setting-stylePanel): 样式面板编辑全局样式新UI与交互-细节优化 * fix(settingPanel-style): 样式面板新交互细节调整 * fix(setting-stylePanel): 修复样式选择器颜色不对的 bug * fix(setting-stylePanel): 样式选择器间距调整 * fix(setting-stylePanel): 调整样式选择器距离顶部距离与下拉列表滚动条颜色 * feat(setting-style): add help link button * fix(setting-style): delete empty row by code review
* docs(readme): 增加后端的启动描述 * docs(readme): 增加后端的启动描述 * docs(readme): 增加后端的启动描述 * docs(readme): 增加后端的启动描述 * docs(readme): 更新图片地址 * docs(readme): 更新图片地址 * fix(material-panel-svg): 修复物料左侧插件面板里图标旋转的问题
* fix:修复新增组件sql语句字段校验 * fix:优化脚本提示 * fix:函数名优化 * fix:修复更新的sql问题
* fix:TinyTabItem、TinyBreadcrumbItem组件配置有误 * fix:修复tiny-vue组件导出名称有误
* fix: 修复robot plugin不存在时发生的白屏错误 * styled: 修改代码风格 * fix: 修改监视意见
* fix: 修复画布选中框的显示层级不正确的问题 * fix: 修正错误注释 * fix: 选中节点后支持横向滚动到节点 * fix: 根据垂直滚动条调整工具操作条位置 * fix: 删除冗余代码 * fix: 代码优化 * fix: 修改变量名称 * fix: 调整函数顺序,消除eslint错误
* feat: 页面输入输出配置,配置内有内容时,按钮替换为展示部分文案内容。 * feat: 方法名修改更加符合语义 * fix: 1. 给画布添加根元素(与出码、预览保持一致)。2. 选中画布相当于选中根节点,右侧面板会出现可以添加属性的节点。3. 可以支持给根节点添加类名,id、ref,并修改样式。 4. 支持给根元素添加事件,循环等 * fix: 意见修复
* fix:修复修改远程字段发送请求无效 * fix:修复请求参数处理函数未生效 * fix:修复数据源表单校验提前退出 * fix:修改提示语 * fix:按review添加注释
* fix:修复第三方组件无法渲染 * fix:区分环境加载prod
* fix(chore): sync vue version * fix(chore): sync vue-i18n version * fix(chore): change vue and vue-i18n to peerdependencies * fix(chore): update block build dependencies * fix(chore): change vue-generator dependencies to update
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (2)
packages/toolbars/save/src/js/index.js (2)
100-121
: LGTM: Enhanced pre-save and save logic with proper error handling.The new logic in
openCommon
function effectively handles pre-save and save operations with proper error handling. It also addresses the previous infinite recursion issue.Consider using a more descriptive name for the
printer
variable, such aslogger
, to better reflect its purpose:- const printer = console + const logger = consoleThis change would make the code slightly more self-documenting.
Security vulnerabilities detected in data handling.
The analysis revealed the following potential security issues:
Potential XSS vulnerabilities:
packages/plugins/tree/src/Main.vue
: Usage ofinnerHTML
.packages/canvas/render/src/RenderMain.js
: Usage ofinnerHTML
.packages/canvas/container/src/container.js
: Usage ofinnerHTML
.Use of
eval
:
packages/vue-generator/src/templates/vue-template/templateFiles/src/lowcodeConfig/dataSource.js
:window.eval('var fn = ' + fnContent)
.packages/vue-generator/test/testcases/generator/expected/appdemo01/src/lowcodeConfig/dataSource.js
:window.eval('var fn = ' + fnContent)
.These issues may pose security risks and should be addressed promptly.
🔗 Analysis chain
Line range hint
1-174
: Consider a security review for data handling.While the changes themselves don't introduce obvious security issues, it's important to ensure that all data being saved or processed is properly sanitized and validated. Consider the following points:
- Verify that any user input or external data is properly sanitized before being processed or saved.
- Review the use of
JSON.parse
andJSON.stringify
to ensure they're not vulnerable to potential JSON injection attacks.- Consider implementing input validation for the
pageSchema
and other data structures being saved or processed.To help verify the security of data handling, you can run the following script:
This script will help identify potential security issues related to data handling in the codebase.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for potential security vulnerabilities in data handling # Look for uses of JSON.parse without try-catch echo "Searching for unprotected JSON.parse usage:" rg --type js 'JSON\.parse\([^)]+\)(?!.*catch)' # Look for potential XSS vulnerabilities echo "Searching for potential XSS vulnerabilities:" rg --type js 'innerHTML|outerHTML|document\.write' # Look for eval or Function constructor usage echo "Searching for eval or Function constructor usage:" rg --type js 'eval\(|new Function\('Length of output: 1482
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- packages/toolbars/preview/src/Main.vue (3 hunks)
- packages/toolbars/save/src/Main.vue (1 hunks)
- packages/toolbars/save/src/js/index.js (3 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/toolbars/preview/src/Main.vue
- packages/toolbars/save/src/Main.vue
🧰 Additional context used
🔇 Additional comments (3)
packages/toolbars/save/src/js/index.js (3)
20-20
: LGTM: New imports added for enhanced functionality.The new imports for
getOptions
andmeta
are correctly added and will be used in the updatedopenCommon
function.Also applies to: 26-26
166-173
: LGTM: Added post-save operation with proper error handling.The new logic in the
saveCommon
function effectively handles post-save operations by calling thesaved
function if it's defined. The error handling is properly implemented, ensuring that any errors during the post-save operation are logged without breaking the execution flow.
Line range hint
1-174
: LGTM: Overall structure and flow maintained with enhanced functionality.The changes in this file successfully add new functionality for pre-save, save, and post-save operations while maintaining the overall structure and flow. The modifications align well with the PR objectives and the AI-generated summary, enhancing the save and open operations without introducing breaking changes.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 0
🧹 Outside diff range and nitpick comments (1)
packages/canvas/container/src/CanvasContainer.vue (1)
67-67
: Good addition ofcanvasSrcDoc
prop andsrcAttrName
computed property.The new prop and computed property effectively support the dynamic attribute implementation in the template. This enhances the component's flexibility in handling different iframe source types.
Consider simplifying the
srcAttrName
computed property for better readability:const srcAttrName = computed(() => props.canvasSrc ? 'src' : 'srcdoc')Also applies to: 78-78
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- packages/canvas/container/src/CanvasContainer.vue (4 hunks)
🧰 Additional context used
🔇 Additional comments (2)
packages/canvas/container/src/CanvasContainer.vue (2)
14-21
: Excellent implementation of conditional rendering and dynamic attribute!The changes effectively address the loading state and implement the dynamic attribute suggestion from the previous review. This approach provides flexibility in handling different source types (
src
orsrcdoc
) for the iframe.
247-248
: Correct addition of new properties to the setup function's return object.The inclusion of
loading
andsrcAttrName
in the returned object ensures that these computed properties are accessible in the template, which is consistent with the earlier modifications.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🧹 Outside diff range and nitpick comments (1)
packages/canvas/render/src/context.js (1)
57-57
: Consider the implications of exposing the entireconditions
object.While
getConditions
provides convenient access to all conditions, it might expose internal state that should be managed more carefully.Consider these alternatives:
- Return a copy of the
conditions
object to prevent external modifications:export const getConditions = () => ({ ...conditions })- Implement methods for bulk operations instead of exposing the entire object.
- If direct access is necessary, document the risks and usage guidelines.
These approaches can help maintain better control over the
conditions
state and prevent unintended modifications.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- packages/canvas/render/src/RenderMain.js (2 hunks)
- packages/canvas/render/src/context.js (1 hunks)
🧰 Additional context used
🔇 Additional comments (5)
packages/canvas/render/src/context.js (1)
54-57
: Summary of changes and recommendationsThe additions of
getCondition
andgetConditions
enhance the module's functionality by providing more granular access to condition states. However, there are opportunities for improvement:
- Align
getCondition
withsetCondition
for consistency.- Reconsider the direct exposure of the
conditions
object throughgetConditions
.These changes will improve the overall design and maintainability of the module.
packages/canvas/render/src/RenderMain.js (4)
27-28
: LGTM: New imports added correctly.The new imports
getCondition
andgetConditions
from thecontext
module are correctly added. These imports correspond to the new functions added to theapi
export object.
446-447
: LGTM: New functions added to api export.The
getCondition
andgetConditions
functions are correctly added to theapi
export object, consistent with the new imports.
27-28
: Summary of changes: Enhanced condition management.The changes in this file introduce new functions
getCondition
andgetConditions
for improved condition management. These additions enhance the API's capabilities without disrupting the existing structure. The changes appear to be well-integrated and consistent with the module's purpose.However, please note that there's an unresolved question about potential changes to the
setCondition
function, which were mentioned in the AI summary but not visible in this diff. It's important to clarify this discrepancy to ensure all changes are properly reviewed and understood.To get a comprehensive view of all condition-related functions, you can run:
#!/bin/bash # Description: List all condition-related functions in the codebase echo "Condition-related functions in the codebase:" rg --type js "function (get|set|)Condition" packages/canvas/render/src/Also applies to: 446-447
445-447
: Clarification needed: setCondition function update.The AI summary mentions an update to the
setCondition
function's signature, but no changes are visible in this diff. Could you please clarify if there are any changes to thesetCondition
function that are not shown in this diff?To verify the
setCondition
function changes, you can run the following script:
@@ -51,3 +51,7 @@ export const getContext = () => context | |||
export const setCondition = (id, visible = false) => { | |||
conditions[id] = visible | |||
} | |||
|
|||
export const getCondition = (id) => conditions[id] !== false |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider modifying getCondition
to align with setCondition
.
The current implementation of getCondition
differs from the suggestion in the past review comment. While it correctly retrieves the condition state, it might not be consistent with setCondition
.
Consider modifying the function to explicitly return the condition value or a default:
-export const getCondition = (id) => conditions[id] !== false
+export const getCondition = (id) => conditions[id] ?? false
This change would make getCondition
more consistent with setCondition
and provide a clear default value when the condition is not set.
📝 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.
export const getCondition = (id) => conditions[id] !== false | |
export const getCondition = (id) => conditions[id] ?? false |
packages/canvas/index.js
Outdated
|
||
export { CanvasContainer, CanvasLayout, DesignCanvas } | ||
export { CanvasContainer, CanvasLayout, DesignCanvas, createRender } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
需要单独导出createRender 避免css污染
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
eslint报错、gpg签名问题需要处理
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
PR标题需要修改,不应该有unibuy字眼。==> feat: ...(feature description)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- .eslintrc.js (1 hunks)
🧰 Additional context used
🔇 Additional comments (1)
.eslintrc.js (1)
Line range hint
2-30
: LGTM: ESLint configuration is well-structuredThe ESLint configuration is well-structured and follows best practices for a Vue.js project. It includes appropriate environment settings, parser options, plugins, and rules.
.eslintrc.js
Outdated
@@ -1,4 +1,4 @@ | |||
module.exports = { | |||
export default { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Revert to CommonJS module.exports
syntax for ESLint configuration
The change from export default { ... }
to module.exports = { ... }
is necessary to maintain compatibility with ESLint. Since the project does not use ES modules ("type": "module"
is not set in package.json
), using ES6 module syntax can cause ESLint to fail in parsing the configuration file.
Please revert this change to ensure ESLint functions correctly:
-export default {
+module.exports = {
🔗 Analysis chain
Revert to CommonJS module.exports syntax for ESLint configuration
The change from module.exports = { ... }
to export default { ... }
may cause issues with ESLint parsing the configuration file. ESLint typically expects CommonJS format for its configuration files.
Please revert this change to ensure compatibility with ESLint:
-export default {
+module.exports = {
To confirm if this change affects other parts of the project, run the following script:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any imports of the ESLint config and verify the project's module system
# Test 1: Search for imports of .eslintrc.js
echo "Searching for imports of .eslintrc.js:"
rg --type js --type ts "require.*eslintrc" || echo "No CommonJS imports found"
rg --type js --type ts "import.*eslintrc" || echo "No ES6 imports found"
# Test 2: Check if the project uses ES modules
echo "Checking for 'type': 'module' in package.json:"
jq '.type' package.json || echo "No 'type' field found in package.json"
# Test 3: Check for babel configuration
echo "Checking for Babel configuration:"
fd -e js -e json -e yaml -e yml --exec grep -l "babel" {} \; || echo "No Babel configuration found"
Length of output: 1644
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- lint-staged.config.js (1 hunks)
- package.json (1 hunks)
✅ Files skipped from review due to trivial changes (1)
- lint-staged.config.js
🧰 Additional context used
@@ -1,6 +1,7 @@ | |||
{ | |||
"name": "tiny-engine", | |||
"private": true, | |||
"type": "module", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
💡 Codebase verification
Issues Identified with ES Modules Adoption
The migration to ECMAScript modules introduces several compatibility concerns:
-
CommonJS Usage in Codebase:
- Files with
require()
calls:scripts/copyToVscode.js
scripts/buildComponentSchemas.js
scripts/setup.js
mockServer/gulpfile.js
mockServer/build/dev-server.js
mockServer/src/services/app.js
mockServer/src/services/schema2code.js
packages/vue-generator/src/utils/vue-sfc-validator.js
packages/vue-generator/src/generator/page.js
packages/vue-generator/test/utils/logger/index.js
packages/vue-generator/test/testcases/full/index.js
packages/engine-cli/bin/cli.js
packages/settings/design/src/convertToSchema.js
- Files with
-
CommonJS
module.exports
Usage:mockServer/pm2.js
mockServer/src/config/config.js
mockServer/src/routes/error-routes.js
mockServer/src/middleware/ErrorRoutesCatch.js
- Various
.umd.js
files inmockServer/src/assets/js/
- ESLint configuration in
mockServer/.eslintrc.js
- Logger utilities in
packages/vue-generator/test/utils/logger/index.js
- Test configurations in
packages/vue-generator/test/testcases/full/index.config.js
- ESLint configuration in
packages/canvas/.eslintrc.js
-
Local Dependencies Missing ES Module Support:
@opentiny/tiny-engine-cli
plugin
@opentiny/tiny-engine-mock
Recommended Actions:
-
Refactor CommonJS to ES Modules:
- Replace
require()
withimport
statements. - Replace
module.exports
withexport
syntax.
- Replace
-
Update Local Dependencies:
- Ensure all local packages specify
"type": "module"
in theirpackage.json
. - Refactor local dependencies to use ES module syntax where necessary.
- Ensure all local packages specify
-
Verify Dependencies Compatibility:
- Confirm that all third-party dependencies are compatible with ES modules or provide ES module builds.
-
Adjust Build Processes:
- Update build tools and configurations to support ES modules.
-
Comprehensive Testing:
- Perform thorough testing to identify and fix any runtime issues stemming from the migration.
🔗 Analysis chain
Significant change: Project now uses ECMAScript modules
The addition of "type": "module"
to package.json
is a substantial change that affects how modules are imported and exported throughout the project. This aligns the project with modern JavaScript standards but may have far-reaching effects on the codebase.
Consider the following implications and actions:
- Compatibility: Ensure all existing code is compatible with ES modules. This may involve updating
import
andexport
statements, and removingrequire()
calls. - Dependencies: Verify that all dependencies support ES modules. Some packages may need to be updated or replaced.
- Build process: Check if the build process needs adjustments to handle ES modules correctly.
- Documentation: Update any documentation that explains how to work with the project, especially regarding module imports and exports.
To verify the impact of this change, run the following script:
This script will help identify potential issues that need to be addressed following this change. Please review the results carefully and make necessary adjustments to ensure smooth transition to ES modules.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for potential issues with ES modules adoption
# Test 1: Search for CommonJS require() calls that need to be replaced
echo "Searching for require() calls:"
rg --type js 'require\(' -g '!node_modules'
# Test 2: Check for .js files using module.exports (which is not supported in ES modules)
echo "Searching for module.exports usage:"
rg --type js 'module\.exports' -g '!node_modules'
# Test 3: Look for package.json files of local dependencies to check their "type" field
echo "Checking local dependencies for ES module support:"
fd package.json -x jq -r 'select(.type != "module") | "Package \(.name) does not specify type: module"'
# Note: This script provides a starting point. Manual review of the results is necessary.
Length of output: 5364
070e92d
to
c738e2a
Compare
English | 简体中文
PR
PR Checklist
Please check if your PR fulfills the following requirements:
PR Type
What kind of change does this PR introduce?
Background and solution
What is the current behavior?
Issue Number: N/A
What is the new behavior?
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
Configuration Changes
"type": "module"
property inpackage.json
for modern JavaScript standards.