feat: materials new protocol #940
Conversation
WalkthroughThis pull request introduces a comprehensive restructuring of how component dependencies are managed across the application. The changes primarily focus on replacing Changes
Possibly related PRs
Suggested Reviewers
Poem
Finishing Touches
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: 8
🔭 Outside diff range comments (3)
packages/canvas/common/src/utils.js (2)
Line range hint
107-116: Handle undefined modules when setting componentsWhen
dynamicImportComponentsreturns an empty object,modules[exportName]may beundefined. This could lead to issues when assigning components. Consider checking ifmodules[exportName]exists before assigning it.Apply this diff to fix the issue:
export const setComponents = async ({ package: pkg, script, components }) => { if (!pkg) return const modules = await dynamicImportComponents(pkg, script) Object.entries(components).forEach(([componentId, exportName]) => { if (!window.TinyLowcodeComponent[componentId]) { + if (modules[exportName]) { window.TinyLowcodeComponent[componentId] = modules[exportName] + } else { + console.warn(`Component "${exportName}" not found in module "${pkg}".`) + } } }) }
Line range hint
125-131: Fix incorrect argument passing todynamicImportComponentsIn
updateDependencies, thescripts.map(dynamicImportComponents)call passes the entire script object as the first argument, which doesn't match the expected parameters. Destructure the script objects to pass the correct arguments.Apply this diff to fix the issue:
export const updateDependencies = ({ detail }) => { const { scripts = [], styles = [] } = detail || {} const { styles: canvasStyles } = window.componentsDepsMap const newStyles = [...styles].filter((item) => !canvasStyles.has(item)) newStyles.forEach((item) => canvasStyles.add(item)) - const promises = [...newStyles].map((src) => addStyle(src)).concat(scripts.map(dynamicImportComponents)) + const promises = [...newStyles] + .map((src) => addStyle(src)) + .concat(scripts.map(({ package: pkg, script }) => dynamicImportComponents(pkg, script))) Promise.allSettled(promises) }packages/plugins/materials/src/composable/useMaterial.js (1)
Line range hint
417-437: Avoid mutating state while iterating ingetBlockDepsModifying
materialState.componentsDepsMap.scriptswhile iterating overscriptsmay lead to unexpected behavior. Collect new scripts separately before updating the state.Apply this diff:
const getBlockDeps = (dependencies = {}) => { const { scripts = [], styles = [] } = dependencies + const newScripts = [] scripts.length && scripts.forEach((npm) => { const { package: pkg, script, css, components } = npm const npmInfo = materialState.componentsDepsMap.scripts.find((item) => item.package === pkg) if (!npmInfo || !npmInfo.script) { - materialState.componentsDepsMap.scripts.push({ package: pkg, script, css, components }) + newScripts.push({ package: pkg, script, css, components }) } else { const components = npmInfo.components || {} npmInfo.components = { ...components, ...npm.components } } }) + materialState.componentsDepsMap.scripts.push(...newScripts) styles?.forEach((item) => materialState.componentsDepsMap.styles.add(item)) }
🧹 Nitpick comments (10)
packages/canvas/common/src/utils.js (1)
79-100: Improve error handling indynamicImportComponentsIf neither the import map includes the package nor a script URL is provided, the
modulesobject remains empty, which may cause issues downstream. Consider adding a warning or error handling to notify about the missing module.You can add a warning message when the module fails to load:
const dynamicImportComponents = async (pkg, script) => { if (window.TinyComponentLibs[pkg]) { return window.TinyComponentLibs[pkg] } let modules = {} try { // 优先从importmap导入，兼容npm.script字段定义的cdn地址 if (getImportMapKeys().includes(pkg)) { modules = await import(/* @vite-ignore */ pkg) } else if (script) { modules = await import(/* @vite-ignore */ script) + } else { + console.warn(`Module "${pkg}" not found in import map and no script URL provided.`) } } catch (error) { modules = {} } window.TinyComponentLibs[pkg] = modules return modules }packages/plugins/materials/src/composable/useMaterial.js (1)
295-318: Consolidate dependency aggregation logicThe logic in
setCanvasDepsfor aggregating dependencies is similar to other parts of the code. Consider creating a utility function to handle dependency aggregation to avoid duplication.packages/canvas/DesignCanvas/src/importMap.js (2)
Line range hint
3-46: Update function documentation to reflect new parameterThe function
getImportMapDatanow accepts a second parametercanvasDeps. Update the JSDoc comment to include this parameter and its purpose.Apply this diff:
/** * Generates import map data for the canvas. + * @param {Object} overrideVersions - Optional versions to override default import map versions. + * @param {Object} canvasDeps - Dependencies from the canvas, including scripts and styles. */ export function getImportMapData(overrideVersions = {}, canvasDeps = { scripts: [], styles: [] }) {
30-35: InitializematerialRequireimports properlyEnsure that
materialRequire.importsis always an object to prevent potentialundefinederrors when spreading intoimportMap.imports.Apply this diff:
const materialRequire = canvasDeps.scripts.reduce((imports, { package: pkg, script }) => { imports[pkg] = script return imports }, {}) const importMap = { imports: { vue: `${VITE_CDN_DOMAIN}/vue@${importMapVersions.vue}/dist/vue.runtime.esm-browser.prod.js`, 'vue-i18n': `${VITE_CDN_DOMAIN}/vue-i18n@${importMapVersions.vueI18n}/dist/vue-i18n.esm-browser.js`, ...blockRequire.imports, ...tinyVueRequire.imports, + ...materialRequire } }packages/vue-generator/src/plugins/genDependenciesPlugin.js (1)
48-54: Consider adding package validationWhile the package processing logic is correct, it might benefit from additional validation to ensure package integrity.
Consider adding validation:
packages.forEach((item) => { const { package: packageName, version } = item + if (!packageName) { + console.warn('Invalid package entry:', item) + return + } + + if (version && !isValidSemver(version)) { + console.warn(`Invalid version '${version}' for package '${packageName}'`) + return + } if (packageName && !resDeps[packageName]) { resDeps[packageName] = version || 'latest' } })You'll need to add the following import:
import { valid as isValidSemver } from 'semver'packages/canvas/DesignCanvas/src/DesignCanvas.vue (3)
29-29: Consider grouping related importsConsider grouping
useMessagewith other related hooks for better code organization.
61-74: Consider extracting initialization logicThe canvas initialization logic could be extracted into a separate function for better maintainability and reusability.
+const initCanvasDeps = (deps) => { + if (canvasSrc) { + return + } + const { importMap, importStyles } = getImportMapData( + getMergeMeta('engine.config')?.importMapVersion, + deps + ) + canvasSrcDoc.value = initCanvas(importMap, importStyles).html +} + useMessage().subscribe({ topic: 'init_canvas_deps', - callback: (deps) => { - if (canvasSrc) { - return - } - const { importMap, importStyles } = getImportMapData( - getMergeMeta('engine.config')?.importMapVersion, - deps - ) - canvasSrcDoc.value = initCanvas(importMap, importStyles).html - } + callback: initCanvasDeps })
Line range hint
82-147: Consider extracting complex conditionsThe watch logic contains complex nested conditions. Consider extracting these into named functions or constants for better readability.
Example approach:
const shouldShowConfirm = (isSaved, pageSchema, oldPageSchema) => (!isSaved || pageSchema !== oldPageSchema) && !showModal const isValidPageStatus = (pageStatus, pageSchema) => ![PAGE_STATUS.Guest, PAGE_STATUS.Occupy].includes(pageStatus.state) && pageSchema?.componentNamedesigner-demo/public/mock/bundle.json (2)
43-43: Consider using object notation for shortcuts and contextMenu actions.The current array syntax for properties and actions could be converted to object notation for better maintainability and type safety.
- "properties": ["type", "size"] + "properties": { + "type": true, + "size": true + } - "actions": ["copy", "remove", "insert", "updateAttr", "bindEevent", "createBlock"] + "actions": { + "copy": true, + "remove": true, + "insert": true, + "updateAttr": true, + "bindEevent": true, + "createBlock": true + }Also applies to: 46-46, 49-49
Line range hint
13258-13271: Consider adding value validation for checkbox group.The default values and options should be validated to ensure they match.
"modelValue": ["name1", "name2"], +"validator": { + "type": "array", + "items": { + "enum": ["name1", "name2", "name3"] + } +}
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
designer-demo/public/mock/bundle.json(51 hunks)mockServer/src/mock/get/app-center/v1/apps/schema/918.json(1 hunks)packages/canvas/DesignCanvas/src/DesignCanvas.vue(2 hunks)packages/canvas/DesignCanvas/src/importMap.js(2 hunks)packages/canvas/common/src/utils.js(2 hunks)packages/canvas/container/src/CanvasContainer.vue(1 hunks)packages/canvas/container/src/container.js(1 hunks)packages/canvas/render/src/RenderMain.js(1 hunks)packages/canvas/render/src/runner.js(2 hunks)packages/common/js/preview.js(1 hunks)packages/design-core/src/preview/src/preview/Preview.vue(1 hunks)packages/plugins/bridge/src/js/resource.js(2 hunks)packages/plugins/materials/src/composable/useMaterial.js(9 hunks)packages/toolbars/generate-code/src/Main.vue(1 hunks)packages/vue-generator/src/plugins/genDependenciesPlugin.js(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/canvas/container/src/container.js
🔇 Additional comments (13)
packages/plugins/materials/src/composable/useMaterial.js (1)
403-413:
Initialize materialState.dependencies to ensure it is an array
In fetchMaterial, materialState.dependencies may not be properly initialized if packages is undefined. Ensure it is always an array to prevent potential errors.
Apply this diff:
const packages = []
materials.forEach((response) => {
if (response.status === 'fulfilled' && response.value.materials) {
addMaterials(response.value.materials)
packages.push(...(response.value.materials.packages || []))
}
})
- materialState.dependencies = packages
+ materialState.dependencies = packages || []
setCanvasDeps()Likely invalid or redundant comment.
packages/common/js/preview.js (1)
Line range hint 29-36: LGTM! Verify dependency mapping consistency
The renaming from thirdPartyDeps to componentsDepsMap is consistent with the PR's objectives. The logic for processing scripts and styles is maintained correctly.
Let's verify the consistency of this change across the codebase:
✅ Verification successful
Dependency mapping change is consistently implemented
The renaming from thirdPartyDeps to componentsDepsMap has been thoroughly verified. The new componentsDepsMap is properly integrated across the codebase:
- Used in material state management (
useMaterial.js) - Correctly propagated to canvas components
- Properly handled in preview functionality
- No lingering references to the old
thirdPartyDepsname
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for any remaining references to thirdPartyDeps
rg "thirdPartyDeps" --type js --type vue
# Verify the new componentsDepsMap usage
rg "componentsDepsMap" --type js --type vue
Length of output: 140
Script:
#!/bin/bash
# Check for any remaining references to thirdPartyDeps, searching all text files
rg "thirdPartyDeps"
# Verify the new componentsDepsMap usage, searching all text files
rg "componentsDepsMap"
Length of output: 1749
packages/canvas/render/src/runner.js (1)
14-14: LGTM! Import statement updated correctly
The import statement has been updated to include setComponents, maintaining consistency with the new dependency management approach.
packages/vue-generator/src/plugins/genDependenciesPlugin.js (2)
25-25: LGTM! Schema destructuring updated correctly
The addition of the packages array to the schema destructuring aligns with the new material asset package protocol.
48-54: Verify package resolution strategy
The current implementation might need to consider package resolution conflicts between different sources (utils, componentsMap, and packages).
Let's analyze the package resolution strategy:
✅ Verification successful
Let me analyze the current implementation and verification results:
- The code in question handles package dependencies by storing them in
resDepsobject - From the search results, I can see:
- The package resolution happens in multiple places (utils, componentsMap, packages)
- There are multiple components using the same package with different versions (e.g. @opentiny/vue has versions 0.1.16, 0.1.17, 3.2.0, 3.4.1)
Let me check the full implementation of the dependency resolution:
After analyzing the full implementation, I can now provide a final response:
Package resolution strategy is correctly implemented
The current implementation has a well-defined precedence order that prevents conflicts:
- Utils dependencies are processed first, with duplicate prevention via
resDeps[packageName]check - Component dependencies are added only if:
- The package isn't already in resDeps
- The component is actually used (verified via componentsSet)
- Additional packages are added last, also with duplicate prevention
- Special handling for @opentiny/vue-icon to match @opentiny/vue version
This strategy ensures that:
- No duplicate packages are added
- Earlier declarations take precedence
- Only dependencies for used components are included
- Version consistency is maintained for related packages
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential package conflicts in the codebase
rg -A 5 "package: .*," --type js --type json
# Look for existing version resolution logic
rg "version.*latest" --type js
Length of output: 85133
Script:
#!/bin/bash
# Check the full implementation of genDependenciesPlugin
cat packages/vue-generator/src/plugins/genDependenciesPlugin.js
Length of output: 2744
packages/plugins/bridge/src/js/resource.js (3)
14-21: LGTM! Well-organized imports
The addition of useMaterial import aligns with the PR's objective of modifying the material asset package protocol.
188-221: Well-structured async/await implementation with proper error handling
The conversion to async/await and addition of try/catch improves error handling. The conditional handling of npm type resources with setCanvasDeps() aligns with the new material package protocol.
224-227: LGTM! Clear user feedback implementation
The success notification implementation provides clear feedback with dynamic messages based on the action type.
packages/design-core/src/preview/src/preview/Preview.vue (1)
83-83: LGTM! Simplified import map handling
The direct usage of store.setImportMap simplifies the code by removing unnecessary intermediate steps, making it more maintainable.
packages/toolbars/generate-code/src/Main.vue (1)
148-149: LGTM: Addition of packages array to appSchema
The addition of the packages array aligns with the PR objectives to support the new material asset package protocol.
Let's verify the packages array structure in other files:
✅ Verification successful
LGTM: Package array structure is consistent across the codebase
Based on the search results, the packages array structure is consistently defined and used across the codebase:
- Mock data in
designer-demo/public/mock/bundle.jsonshows the expected structure withname,package,version, andscriptproperties - The structure is properly consumed in
vue-generatorfor dependency generation - The implementation in
useMaterial.jscorrectly handles and propagates the packages array
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check the structure of packages array across the codebase
# Expected: Consistent structure with name, npm package name, version, and CDN links
# Search for packages array definitions
rg -A 5 "packages.*\["
Length of output: 3097
mockServer/src/mock/get/app-center/v1/apps/schema/918.json (1)
2095-2096: Verify the axios version for security concerns
The addition of CDN link for axios is good, but we should verify if version 1.7.9 is the latest stable version without known vulnerabilities.
Let's check the version:
✅ Verification successful
Axios version 1.7.9 is secure and up-to-date
The version check reveals that:
- 1.7.9 is the current latest stable version from npm
- The most recent high severity vulnerability affected versions <= 1.7.3 (patched in 1.7.4)
- The specified version 1.7.9 is above the vulnerable range and includes all security patches
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify axios version security
# Expected: No known vulnerabilities for the specified version
# Check for security advisories
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "axios", severities: [CRITICAL, HIGH]) {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
# Get latest version from npm
curl -s https://registry.npmjs.org/axios/latest | jq '.version'
Length of output: 1066
designer-demo/public/mock/bundle.json (2)
14081-14088: Verify package dependency configuration.
The new packages array introduces element-plus dependency configuration. While the structure looks good, we should verify the version consistency across components using this package.
✅ Verification successful
Package version consistency verified across the codebase
The version "2.4.2" of element-plus is consistently used across all instances in the codebase, including:
- designer-demo/public/mock/bundle.json
- mockServer/src/assets/json/appinfo.json
- packages/engine-cli/template/designer/public/mock/bundle.json
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if all element-plus component definitions use consistent version
rg -A 1 '"package": "element-plus"' | grep -A 1 'version'
Length of output: 1561
635-635: Validate form item nesting rules.
The nesting rules for ElForm should be verified to ensure proper form structure.
Also applies to: 644-644, 647-647, 650-650
✅ Verification successful
Form nesting rules are properly defined in the configuration
The configuration correctly specifies that ElFormItem components are allowed as children within ElForm components through the childWhitelist property. This is consistently defined across multiple configuration files in the codebase, indicating proper form structure validation rules are in place.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check if ElFormItem is properly nested under ElForm
ast-grep --pattern 'componentName: "ElForm" {
$$$
componentName: "ElFormItem"
$$$
}'
Length of output: 92
Script:
#!/bin/bash
# Search for form nesting rules in bundle.json
rg -A 5 '"childWhitelist".*ElFormItem' public/mock/bundle.json
Length of output: 128
Script:
#!/bin/bash
# Search for component definitions related to ElForm and ElFormItem
rg -A 10 '"componentName":\s*"ElForm"' public/mock/bundle.json
Length of output: 128
Script:
#!/bin/bash
# Look for any validation or nesting rules
rg -i "form.*item.*nest|nest.*form.*item" public/mock/bundle.json
Length of output: 131
Script:
#!/bin/bash
# Find the correct path to bundle.json
fd bundle.json
Length of output: 216
Script:
#!/bin/bash
# Search for ElForm in all json files
rg -A 5 '"componentName":\s*"ElForm"' --type json
Length of output: 574
Script:
#!/bin/bash
# Search for childWhitelist in all json files
rg -A 5 '"childWhitelist".*ElFormItem' --type json
Length of output: 1522
packages/canvas/common/src/utils.js
Outdated
| const getImportMapKeys = () => { | ||
| try { | ||
| const importMaps = document.querySelector('script[type="importmap"]').textContent | ||
| const importMapObject = JSON.parse(importMaps) | ||
|
|
||
| return Object.keys(importMapObject.import) | ||
| } catch (error) { | ||
| return [] | ||
| } | ||
| } |
There was a problem hiding this comment.
Add null check to prevent potential TypeError in getImportMapKeys
The document.querySelector('script[type="importmap"]') might return null if no import map is found, leading to a TypeError when accessing .textContent. Consider adding a null check before accessing .textContent.
Apply this diff to fix the issue:
const getImportMapKeys = () => {
try {
- const importMaps = document.querySelector('script[type="importmap"]').textContent
+ const importMapElement = document.querySelector('script[type="importmap"]')
+ if (!importMapElement) {
+ return []
+ }
+ const importMaps = importMapElement.textContent
const importMapObject = JSON.parse(importMaps)
return Object.keys(importMapObject.import)
} catch (error) {
return []
}
}📝 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 getImportMapKeys = () => { | |
| try { | |
| const importMaps = document.querySelector('script[type="importmap"]').textContent | |
| const importMapObject = JSON.parse(importMaps) | |
| return Object.keys(importMapObject.import) | |
| } catch (error) { | |
| return [] | |
| } | |
| } | |
| const getImportMapKeys = () => { | |
| try { | |
| const importMapElement = document.querySelector('script[type="importmap"]') | |
| if (!importMapElement) { | |
| return [] | |
| } | |
| const importMaps = importMapElement.textContent | |
| const importMapObject = JSON.parse(importMaps) | |
| return Object.keys(importMapObject.import) | |
| } catch (error) { | |
| return [] | |
| } | |
| } |
| const results = await Promise.allSettled(npmUtils.map((item) => import(/* @vite-ignore */ item.content.package))) | ||
|
|
||
| results.forEach((res, index) => { | ||
| if (res.status !== 'fulfilled') { | ||
| return | ||
| } | ||
|
|
||
| const module = res.value | ||
| const { name, content } = npmUtils[index] | ||
| const { exportName, destructuring } = content | ||
|
|
||
| utilsCollection[name] = destructuring ? module[exportName] : module.default | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Handle rejected promises when importing npm utilities
In setUtils, the code currently ignores rejected promises from failed imports. To improve robustness, consider handling rejected promises and logging errors.
Apply this diff to handle rejected imports:
const results = await Promise.allSettled(npmUtils.map((item) => import(/* @vite-ignore */ item.content.package)))
results.forEach((res, index) => {
if (res.status !== 'fulfilled') {
+ console.error(`Failed to import npm utility "${npmUtils[index].name}":`, res.reason)
return
}
const module = res.value
const { name, content } = npmUtils[index]
const { exportName, destructuring } = content
utilsCollection[name] = destructuring ? module[exportName] : module.default
})📝 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 results = await Promise.allSettled(npmUtils.map((item) => import(/* @vite-ignore */ item.content.package))) | |
| results.forEach((res, index) => { | |
| if (res.status !== 'fulfilled') { | |
| return | |
| } | |
| const module = res.value | |
| const { name, content } = npmUtils[index] | |
| const { exportName, destructuring } = content | |
| utilsCollection[name] = destructuring ? module[exportName] : module.default | |
| }) | |
| const results = await Promise.allSettled(npmUtils.map((item) => import(/* @vite-ignore */ item.content.package))) | |
| results.forEach((res, index) => { | |
| if (res.status !== 'fulfilled') { | |
| console.error(`Failed to import npm utility "${npmUtils[index].name}":`, res.reason) | |
| return | |
| } | |
| const module = res.value | |
| const { name, content } = npmUtils[index] | |
| const { exportName, destructuring } = content | |
| utilsCollection[name] = destructuring ? module[exportName] : module.default | |
| }) |
| const setUtils = async (data, clear, isForceRefresh) => { | ||
| if (clear) { | ||
| reset(utils) | ||
| } | ||
| const utilsCollection = {} | ||
| // 目前画布还不具备远程加载utils工具类的功能，目前只能加载TinyVue组件库中的组件工具 | ||
| data?.forEach((item) => { | ||
| const util = TinyVue[item.content.exportName] | ||
| if (util) { | ||
| utilsCollection[item.name] = util | ||
| } | ||
|
|
||
| // 此处需要把工具类中的icon图标也加入utils上下文环境 | ||
| const utilIcon = TinyVueIcon[item.content.exportName] | ||
| if (utilIcon) { | ||
| utilsCollection[item.name] = utilIcon | ||
| } | ||
| const utilsCollection = {} | ||
|
|
||
| // 解析函数式的工具类 | ||
| if (item.type === 'function') { | ||
| data | ||
| .filter((item) => item.type === 'function') | ||
| .forEach((item) => { | ||
| const defaultFn = () => {} | ||
| utilsCollection[item.name] = generateFunction(item.content.value, context) || defaultFn | ||
| }) | ||
|
|
||
| const npmUtils = data.filter((item) => item.type === 'npm' && item.content.cdnLink) | ||
| const results = await Promise.allSettled(npmUtils.map((item) => import(/* @vite-ignore */ item.content.package))) | ||
|
|
||
| results.forEach((res, index) => { | ||
| if (res.status !== 'fulfilled') { | ||
| return | ||
| } | ||
|
|
||
| const module = res.value | ||
| const { name, content } = npmUtils[index] | ||
| const { exportName, destructuring } = content | ||
|
|
||
| utilsCollection[name] = destructuring ? module[exportName] : module.default | ||
| }) | ||
|
|
There was a problem hiding this comment.
💡 Codebase verification
Missing await keywords in async function calls
The verification reveals several locations where setUtils is called without proper await keywords:
packages/canvas/container/src/container.js:- Line 794:
getRenderer().setUtils(utils, clear, isForceRefresh) - Line 936:
setUtils(useResource().resState.utils)
- Line 794:
packages/canvas/render/src/RenderMain.js:- Line 108:
setUtils(data, false, true) - Line 117:
setUtils([], false, true)
- Line 108:
These calls need to be updated to properly handle the Promise returned by the async setUtils function. Each calling function should be marked as async and use the await keyword when calling setUtils.
🔗 Analysis chain
Ensure proper handling of asynchronous setUtils function
With setUtils now being asynchronous, ensure all calls to setUtils handle the returned Promise correctly. This may involve adding await where setUtils is called or adjusting code to handle the Promise.
Run the following script to find all calls to setUtils and check if they handle the Promise:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find all calls to `setUtils` that may not await the Promise
# Search for `setUtils` calls without `await`
rg --type-add 'vue:*.vue' --files-with-matches 'setUtils' | xargs rg -A 1 '^\s*(?!await\s)(\w+\.)?setUtils\('
Length of output: 350
Script:
#!/bin/bash
# Let's try a different approach to find setUtils calls
# First, find files containing setUtils
rg --type-add 'vue:*.vue' -l 'setUtils'
# Then look for actual usage patterns
echo "=== Searching for setUtils calls ==="
rg -p 'setUtils\('
# Also search for await setUtils to compare
echo "=== Searching for await setUtils calls ==="
rg -p 'await\s+setUtils'
Length of output: 604
Script:
#!/bin/bash
# Let's examine the context around these calls to understand if they're in async functions
echo "=== Context in container.js ==="
rg -p -B 5 -A 5 'getRenderer\(\)\.setUtils|setUtils\(' packages/canvas/container/src/container.js
echo "=== Context in RenderMain.js ==="
rg -p -B 5 -A 5 'setUtils\(' packages/canvas/render/src/RenderMain.js
Length of output: 1490
| materialState.componentsDepsMap.scripts.push(...scripts) | ||
| styles.forEach((item) => materialState.componentsDepsMap.styles.add(item)) |
There was a problem hiding this comment.
Avoid direct mutation of reactive properties
Directly pushing to materialState.componentsDepsMap.scripts may not trigger reactivity. Consider replacing the array to ensure updates are reactive.
Apply this diff to fix the issue:
materialState.componentsDepsMap.scripts.push(...scripts)
+ materialState.componentsDepsMap.scripts = [...materialState.componentsDepsMap.scripts]Committable suggestion skipped: line range outside the PR's diff.
| const getUtilsDependencies = () => { | ||
| const { utils } = useResource().resState | ||
|
|
||
| return utils | ||
| .filter((item) => item.type === 'npm' && item.content.cdnLink) | ||
| .map((item) => { | ||
| const { package: pkg, cdnLink } = item.content | ||
|
|
||
| return { | ||
| package: pkg, | ||
| script: cdnLink | ||
| } | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Check for undefined properties in getUtilsDependencies
In getUtilsDependencies, be cautious of utilities that may not have content or cdnLink properties, which could lead to runtime errors. Add checks to ensure these properties exist before accessing them.
Apply this diff:
return utils
.filter((item) => item.type === 'npm' && item.content && item.content.cdnLink)
.map((item) => {
const { package: pkg, cdnLink } = item.content
return {
package: pkg,
script: cdnLink
}
})📝 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 getUtilsDependencies = () => { | |
| const { utils } = useResource().resState | |
| return utils | |
| .filter((item) => item.type === 'npm' && item.content.cdnLink) | |
| .map((item) => { | |
| const { package: pkg, cdnLink } = item.content | |
| return { | |
| package: pkg, | |
| script: cdnLink | |
| } | |
| }) | |
| } | |
| const getUtilsDependencies = () => { | |
| const { utils } = useResource().resState | |
| return utils | |
| .filter((item) => item.type === 'npm' && item.content && item.content.cdnLink) | |
| .map((item) => { | |
| const { package: pkg, cdnLink } = item.content | |
| return { | |
| package: pkg, | |
| script: cdnLink | |
| } | |
| }) | |
| } |
packages/canvas/render/src/runner.js
Outdated
| const { scripts: componentsScriptsDeps = [], styles: componentsStylesDeps = [] } = window.componentsDepsMap || {} | ||
|
|
||
| Promise.all([ | ||
| ...thirdScripts.map(dynamicImportComponents), | ||
| ...scripts.map((src) => addScript(src)).concat([...thirdStyles, ...styles].map((src) => addStyle(src))) | ||
| ...componentsScriptsDeps.map(setComponents), | ||
| ...scripts.map((src) => addScript(src)).concat([...componentsStylesDeps, ...styles].map((src) => addStyle(src))) |
There was a problem hiding this comment.
Consider implementing proper error handling
While the dependency loading logic is sound, the error handling is currently disabled. This could mask potential issues during component loading.
Consider implementing proper error handling:
Promise.all([
...componentsScriptsDeps.map(setComponents),
...scripts.map((src) => addScript(src)).concat([...componentsStylesDeps, ...styles].map((src) => addStyle(src)))
- ]).finally(() => create(config))
+ ]).then(() => {
+ create(config)
+ }).catch((error) => {
+ console.error('Failed to load dependencies:', error)
+ dispatch('canvasError', { detail: { 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 { scripts: componentsScriptsDeps = [], styles: componentsStylesDeps = [] } = window.componentsDepsMap || {} | |
| Promise.all([ | |
| ...thirdScripts.map(dynamicImportComponents), | |
| ...scripts.map((src) => addScript(src)).concat([...thirdStyles, ...styles].map((src) => addStyle(src))) | |
| ...componentsScriptsDeps.map(setComponents), | |
| ...scripts.map((src) => addScript(src)).concat([...componentsStylesDeps, ...styles].map((src) => addStyle(src))) | |
| const { scripts: componentsScriptsDeps = [], styles: componentsStylesDeps = [] } = window.componentsDepsMap || {} | |
| Promise.all([ | |
| ...componentsScriptsDeps.map(setComponents), | |
| ...scripts.map((src) => addScript(src)).concat([...componentsStylesDeps, ...styles].map((src) => addStyle(src))) | |
| ]).then(() => { | |
| create(config) | |
| }).catch((error) => { | |
| console.error('Failed to load dependencies:', error) | |
| dispatch('canvasError', { detail: { error } }) | |
| }) |
| export const saveResource = async (data, callback, emit) => { | ||
| let result = {} | ||
|
|
There was a problem hiding this comment.
Fix variable shadowing issue
The result variable is declared but never used at the top level, and then shadowed within the try block.
Apply this fix:
-let result = {}
+const result = {}Committable suggestion skipped: line range outside the PR's diff.
| @@ -113,7 +113,7 @@ export default { | |||
| const beforeCanvasReady = () => { | |||
| if (iframe.value) { | |||
| const win = iframe.value.contentWindow | |||
| win.thirdPartyDeps = useMaterial().materialState.thirdPartyDeps | |||
| win.componentsDepsMap = useMaterial().materialState.componentsDepsMap | |||
There was a problem hiding this comment.
💡 Codebase verification
Warning: Incomplete renaming detected in the codebase
The renaming from thirdPartyDeps to componentsDepsMap is incomplete. While the property usage has been updated, there are still related function names using the old terminology in useMaterial.js:
generateThirdPartyDepssetThirdPartyDeps
These function names should also be updated to maintain consistency with the new naming convention:
packages/plugins/materials/src/composable/useMaterial.js: Update function names to match the newcomponentsDepsterminology
🔗 Analysis chain
LGTM: Consistent renaming of dependency property
The renaming from thirdPartyDeps to componentsDepsMap improves clarity and aligns with the new dependency management approach.
Let's verify all references have been updated:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to thirdPartyDeps
# Expected: No remaining references to old property name
# Search for any remaining references to thirdPartyDeps
rg "thirdPartyDeps"
Length of output: 19
Script:
#!/bin/bash
# Let's search for both old and new property names to understand the context better
# and ensure the change is consistent
echo "Searching for componentsDepsMap references:"
rg "componentsDepsMap" -A 2
echo -e "\nSearching for any potential variations of thirdPartyDeps:"
rg -i "third.*deps"
Length of output: 3994
There was a problem hiding this comment.
Actionable comments posted: 4
🔭 Outside diff range comments (1)
packages/canvas/common/src/utils.js (1)
Line range hint
110-117: Add error handling for undefined components and module exports to prevent runtime exceptionsIn the
setComponentsfunction, there's a possibility thatcomponentsormodules[exportName]may beundefined. Attempting to access or assign these values without validation could lead to runtime errors. Please add checks to ensure that these variables are defined before proceeding.Apply this diff to add necessary checks:
export const setComponents = async ({ package: pkg, script, components }) => { if (!pkg) return const modules = await dynamicImportComponents(pkg, script) + if (!components || typeof components !== 'object') { + console.error('Components object is undefined or invalid') + return + } Object.entries(components).forEach(([componentId, exportName]) => { + if (!modules || !modules[exportName]) { + console.error(`Module '${exportName}' not found in package '${pkg}'`) + return + } if (!window.TinyLowcodeComponent[componentId]) { window.TinyLowcodeComponent[componentId] = modules[exportName] } }) }
🧹 Nitpick comments (4)
packages/canvas/common/src/utils.js (1)
63-71: Add a null check fordocument.querySelectorto prevent potential errors when the import map is missingIn the
getImportMapKeysfunction, if the<script type="importmap">element does not exist in the document,document.querySelectorwill returnnull, leading to aTypeErrorwhen attempting to accesstextContent. While thetry...catchblock handles the error by returning an empty array, it's better to explicitly check for the element's existence to avoid unnecessary exceptions.Apply this diff to add a null check:
const getImportMapKeys = () => { try { - const importMaps = document.querySelector('script[type="importmap"]').textContent + const importMapScript = document.querySelector('script[type="importmap"]') + if (!importMapScript) { + return [] + } + const importMaps = importMapScript.textContent const importMapObject = JSON.parse(importMaps) return Object.keys(importMapObject.import) } catch (error) { return [] } }packages/plugins/materials/src/composable/useMaterial.js (2)
Line range hint
212-255: RenamesetThirdPartyDepstosetComponentsDepsfor consistencyThe function
generateThirdPartyDepshas been renamed togenerateComponentsDeps, butsetThirdPartyDepsstill retains the old naming convention. For consistency and clarity, consider renamingsetThirdPartyDepstosetComponentsDeps.Apply this diff to rename the function:
-const setThirdPartyDeps = (components) => { +const setComponentsDeps = (components) => { const { scripts = [], styles = [] } = generateComponentsDeps(components) - materialState.componentsDepsMap.scripts.push(...scripts) + materialState.componentsDepsMap.scripts.push(...scripts) styles.forEach((item) => materialState.componentsDepsMap.styles.add(item)) }Don't forget to update all references to
setThirdPartyDepsin the codebase:#!/bin/bash # Description: Update references from `setThirdPartyDeps` to `setComponentsDeps` # Replace all instances in the codebase rg 'setThirdPartyDeps' --files-with-matches | xargs sed -i 's/setThirdPartyDeps/setComponentsDeps/g'
295-319: Handle duplicate packages when setting canvas dependenciesIn the
setCanvasDepsfunction, when reducingallPackages, duplicate packages are filtered out based on the package name. However, if different versions or scripts are provided for the same package, this could lead to inconsistent behavior. Consider handling version conflicts or consolidating scripts for the same package.Apply this diff to improve duplicate package handling:
- if (scripts.find((item) => item.package === pkg)) { + const existingPackage = scripts.find((item) => item.package === pkg) + if (existingPackage) { + // Merge components and other properties if necessary + existingPackage.components = { ...existingPackage.components, ...npm.components } + return { scripts, styles } + }designer-demo/public/mock/bundle.json (1)
Line range hint
13258-13271: Consider adding validation for checkbox group values.The default modelValue array contains strings but the schema doesn't enforce value types. Consider adding type validation.
"modelValue": ["name1", "name2"], +"type": { + "type": "array", + "items": { + "type": "string" + } +}, "type": "checkbox",
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (15)
designer-demo/public/mock/bundle.json(51 hunks)mockServer/src/mock/get/app-center/v1/apps/schema/918.json(1 hunks)packages/canvas/DesignCanvas/src/DesignCanvas.vue(2 hunks)packages/canvas/DesignCanvas/src/importMap.js(2 hunks)packages/canvas/common/src/utils.js(2 hunks)packages/canvas/container/src/CanvasContainer.vue(1 hunks)packages/canvas/container/src/container.js(1 hunks)packages/canvas/render/src/RenderMain.js(1 hunks)packages/canvas/render/src/runner.js(2 hunks)packages/common/js/preview.js(1 hunks)packages/design-core/src/preview/src/preview/Preview.vue(1 hunks)packages/plugins/bridge/src/js/resource.js(2 hunks)packages/plugins/materials/src/composable/useMaterial.js(9 hunks)packages/toolbars/generate-code/src/Main.vue(1 hunks)packages/vue-generator/src/plugins/genDependenciesPlugin.js(2 hunks)
✅ Files skipped from review due to trivial changes (1)
- packages/canvas/container/src/container.js
🔇 Additional comments (15)
packages/canvas/common/src/utils.js (2)
79-101: Verify that removing the export statement from dynamicImportComponents does not affect external modules
The dynamicImportComponents function was previously exported but is now declared without the export keyword. This change could break functionality in other modules that import this function. Please verify that no external modules rely on this export, or consider re-exporting the function if necessary.
Run the following script to check for external imports of dynamicImportComponents:
125-127: Ensure window.componentsDepsMap is properly initialized before accessing its properties
In the updateDependencies function, window.componentsDepsMap is used to access styles. Please verify that window.componentsDepsMap is initialized and contains the expected structure to prevent potential undefined errors.
Run the following script to check the initialization of window.componentsDepsMap:
packages/canvas/DesignCanvas/src/importMap.js (1)
Line range hint 3-47: LGTM!
The getImportMapData function has been correctly updated to accept canvasDeps, allowing dynamic inclusion of external dependencies in the import map and styles. The default parameters and object destructuring are used appropriately.
packages/common/js/preview.js (1)
Line range hint 29-36: Verify handling of empty or malformed dependency maps
The code assumes componentsDepsMap will always have valid scripts and styles arrays. Consider adding defensive checks.
- const { scripts, styles } = useMaterial().materialState.componentsDepsMap
+ const { scripts = [], styles = [] } = useMaterial().materialState.componentsDepsMap || {}
params.scripts = {}
scripts
.filter((item) => item.script)
.forEach((item) => {
+ if (!item.package) return
params.scripts[item.package] = item.script
})
params.styles = [...styles]Let's verify the structure of componentsDepsMap across the codebase:
packages/vue-generator/src/plugins/genDependenciesPlugin.js (1)
48-54: Consider version conflict resolution strategy
The current implementation might lead to version conflicts when the same package is defined in multiple places (utils, componentsMap, packages).
Let's verify potential version conflicts in existing material definitions:
Consider implementing a version resolution strategy:
- Define priority order (e.g., packages > componentsMap > utils)
- Add version compatibility checks
- Log warnings for potential conflicts
packages/plugins/bridge/src/js/resource.js (3)
14-21: LGTM! Clean import organization
The addition of useMaterial aligns with the new material asset package protocol implementation.
189-203:
Fix variable shadowing in try block
The result variable is declared both in the outer scope and inside the try block, which could lead to unexpected behavior.
Apply this diff to fix the variable shadowing:
- const result = await requestUpdateReSource(data)
+ result = await requestUpdateReSource(data)
- const result = await requestAddReSource(data)
+ result = await requestAddReSource(data)Likely invalid or redundant comment.
213-221: Verify canvas dependency updates
The conditional handling of npm resources with setCanvasDeps() looks good, but let's verify that all canvas instances are properly updated.
✅ Verification successful
Let me gather more information about the canvas dependency update mechanisms to ensure completeness.
Based on the analysis of the shell script results, I can now provide the final response:
Canvas dependency updates are properly handled
The verification shows that the canvas dependency update mechanism is correctly implemented:
setCanvasDeps()inuseMaterial()handles npm dependencies by updating both material dependencies and utility dependencies- The
elsebranch withupdateUtils([result])correctly handles non-npm resources by updating the canvas utility functions - Both paths trigger appropriate canvas refreshes
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Verify the usage of setCanvasDeps across the codebase
# Expected: setCanvasDeps should be called consistently after npm resource changes
# Check for setCanvasDeps usage
rg -A 5 "setCanvasDeps"
# Check for other npm resource handling patterns
ast-grep --pattern 'if ($expr.type === "npm") { $$$ }'
Length of output: 1945
Script:
#!/bin/bash
# Check for updateUtils usage and related canvas update patterns
rg -A 5 "updateUtils"
# Check for canvas refresh/update related functions
rg -A 5 "canvasApi"
# Look for npm type handling in material state
ast-grep --pattern 'type === "npm"'
Length of output: 46980
packages/design-core/src/preview/src/preview/Preview.vue (1)
83-83: Verify cleanup of old import map utilities
The simplified import map management looks good, but we should verify that all references to the old utility functions have been removed.
✅ Verification successful
Based on the search results from our verification attempts, we can make a conclusion:
Import map utilities have been properly cleaned up
The searches show that:
- Only the new
setImportMapmethod is being used in Preview.vue - No instances of the old
addUtilsImportMapwere found anywhere in the codebase - The import map handling is now centralized through the store's
setImportMapmethod - All other import map related code appears to be using the new simplified approach
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check for any remaining references to old import map utilities
# Expected: No references to addUtilsImportMap should remain
# Search for any remaining references to the old function
rg "addUtilsImportMap"
# Check for any similar utility functions that might need updating
ast-grep --pattern 'function $name($_, $_) {
$$$
importMap
$$$
}'
Length of output: 97
Script:
#!/bin/bash
# Let's try a broader search for import map related functions and their usage
# Search for files containing 'importMap' keyword
rg -l "importMap"
# Then let's look for specific patterns related to import map manipulation
rg "\.importMap" -A 2 -B 2
# Search for store operations related to import maps
rg "setImportMap|addImportMap|updateImportMap" -A 2 -B 2
Length of output: 2076
Script:
#!/bin/bash
# Let's check the history of the importMap.js files to understand the changes
rg -A 10 -B 10 "addUtilsImportMap|setImportMap" packages/design-core/src/preview/src/preview/importMap.js packages/canvas/DesignCanvas/src/importMap.js
# Also check the store implementation to see how import maps are handled
rg -A 5 -B 5 "class.*Store" packages/design-core/src/preview/src/preview/
# Look for any utility functions related to import maps
ast-grep --pattern 'function $name($_) {
$$$
importMap
$$$
}'
Length of output: 299
packages/canvas/DesignCanvas/src/DesignCanvas.vue (1)
61-61: LGTM! Good use of Vue's reactivity system
Converting canvasSrcDoc to a ref ensures proper reactivity for canvas updates.
mockServer/src/mock/get/app-center/v1/apps/schema/918.json (1)
2095-2096: Verify axios CDN link compatibility.
The addition of the CDN link for axios should be verified for:
- Version compatibility with the application
- CORS policy compliance
- Availability and reliability of the CDN
designer-demo/public/mock/bundle.json (2)
14081-14088: Verify the package configuration for element-plus.
The package configuration looks good but let's verify the unpkg URLs are accessible and the version is stable.
✅ Verification successful
Element-plus package configuration is secure and accessible
The verification shows:
- Both CDN URLs (CSS and MJS) are accessible with 200 status codes
- Version 2.4.2 is well above the patched version (2.0.6) for the only known vulnerability
- The vulnerability was from April 2022 and has been addressed
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Verify element-plus package URLs and version
curl -I "https://unpkg.com/[email protected]/dist/index.full.mjs"
curl -I "https://unpkg.com/[email protected]/dist/index.css"
# Check for any security advisories
gh api graphql -f query='
{
securityVulnerabilities(first: 5, ecosystem: NPM, package: "element-plus") {
nodes {
advisory {
summary
severity
publishedAt
}
vulnerableVersionRange
firstPatchedVersion {
identifier
}
}
}
}'
Length of output: 2676
635-638: Verify nesting rule configuration for form components.
The nesting rules for ElForm and ElFormItem look correct, but let's verify there are no other form components that should be included in the whitelist.
Also applies to: 768-771
✅ Verification successful
Nesting rules for form components are correctly configured
The verification shows that only ElForm and ElFormItem are the form-related components in the bundle, and the current nesting rule configuration correctly allows ElFormItem as a child of ElForm. No additional form components need to be included in the whitelist.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other form-related components that may need to be included in nesting rules
ast-grep --pattern 'component: "El$_Form$_"'
Length of output: 46
Script:
#!/bin/bash
# Search for form-related components in the bundle.json file
rg -A 5 '"component":\s*"El.*Form' public/mock/bundle.json
# Also search for any form-related nesting rules
rg -A 5 '"childWhitelist".*Form' public/mock/bundle.json
Length of output: 247
Script:
#!/bin/bash
# Search for form-related components in the bundle.json file
rg -A 5 '"component":\s*"El.*Form' designer-demo/public/mock/bundle.json
# Also search for any form-related nesting rules
rg -A 5 '"childWhitelist".*Form' designer-demo/public/mock/bundle.json
Length of output: 804
packages/toolbars/generate-code/src/Main.vue (1)
148-149: Verify handling of new packages property in code generation
The addition of packages property to appSchema aligns with the new material asset protocol. However, we should ensure proper handling in the code generation process.
Let's verify the code generation handling:
Consider adding JSDoc documentation for the appSchema structure to help maintainers understand the new protocol format.
✅ Verification successful
The packages property is properly handled in code generation
The code review reveals that the packages property is safely integrated into the code generation flow:
- It's part of the
appSchemaobject which is passed togenerateAppCode - The spread operator with fallback (
[...(appData.packages || [])]) ensures safe handling of undefined values - The property is used alongside other essential schema properties like
componentsMap,pageSchema, andblockSchema - The generated code is properly processed through
generateAppCodeand transformed into file content for local saving
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Check if the code generation process handles the new packages property
# Search for packages property usage in code generation
rg -A 5 "packages.*=.*appSchema\.packages"
# Search for potential validation or processing of packages
ast-grep --pattern 'function $_(appSchema) {
$$$
packages
$$$
}'
Length of output: 117
Script:
#!/bin/bash
# Let's broaden our search to understand the usage of packages in code generation
# Search for any references to packages in the codebase
rg -A 5 "packages.*=" packages/toolbars/generate-code/
# Look for appSchema usage patterns
rg -A 5 "appSchema\." packages/toolbars/generate-code/
# Search for code generation related functions
ast-grep --pattern 'function generate$_($$$) {
$$$
}'
# Check for any schema or type definitions
fd "\.d\.ts$|types?\.ts$" packages/toolbars/generate-code/ --exec cat {}
Length of output: 248
Script:
#!/bin/bash
# Let's check the broader context of the file and its usage
# Get the content of Main.vue to understand the context
cat packages/toolbars/generate-code/src/Main.vue
# Look for files that might consume the generated output
fd . packages/toolbars/generate-code/ -e .vue -e .ts -e .js --exclude "*.test.*" --exclude "*.spec.*" --exec rg -l "getPreGenerateInfo|appSchema"
# Search for any package-related types or interfaces
rg -g "*.ts" -g "*.d.ts" -g "*.vue" "interface.*App.*Schema|type.*App.*Schema" packages/toolbars/generate-code/
Length of output: 7776
packages/canvas/container/src/CanvasContainer.vue (1)
116-116:
Breaking Change: Verify migration from thirdPartyDeps to componentsDepsMap
The change from thirdPartyDeps to componentsDepsMap is a breaking change that requires careful verification of all consumers.
Let's verify the migration completeness:
Please ensure all consumers of this window property are updated. Consider:
- Adding a migration guide for downstream dependencies
- Documenting the new
componentsDepsMapstructure - Adding a deprecation warning for any transitional period
| const setUtils = async (data, clear, isForceRefresh) => { | ||
| if (clear) { | ||
| reset(utils) | ||
| } | ||
| const utilsCollection = {} | ||
| // 目前画布还不具备远程加载utils工具类的功能，目前只能加载TinyVue组件库中的组件工具 | ||
| data?.forEach((item) => { | ||
| const util = TinyVue[item.content.exportName] | ||
| if (util) { | ||
| utilsCollection[item.name] = util | ||
| } | ||
|
|
||
| // 此处需要把工具类中的icon图标也加入utils上下文环境 | ||
| const utilIcon = TinyVueIcon[item.content.exportName] | ||
| if (utilIcon) { | ||
| utilsCollection[item.name] = utilIcon | ||
| } | ||
| const utilsCollection = {} | ||
|
|
||
| // 解析函数式的工具类 | ||
| if (item.type === 'function') { | ||
| data | ||
| .filter((item) => item.type === 'function') | ||
| .forEach((item) => { | ||
| const defaultFn = () => {} | ||
| utilsCollection[item.name] = generateFunction(item.content.value, context) || defaultFn | ||
| }) | ||
|
|
||
| const npmUtils = data.filter((item) => item.type === 'npm' && item.content.cdnLink) | ||
| const results = await Promise.allSettled(npmUtils.map((item) => import(/* @vite-ignore */ item.content.package))) | ||
|
|
||
| results.forEach((res, index) => { | ||
| if (res.status !== 'fulfilled') { | ||
| return | ||
| } | ||
|
|
||
| const module = res.value | ||
| const { name, content } = npmUtils[index] | ||
| const { exportName, destructuring } = content | ||
|
|
||
| utilsCollection[name] = destructuring ? module[exportName] : module.default | ||
| }) | ||
|
|
||
| Object.assign(utils, utilsCollection) | ||
|
|
||
| // 因为工具类并不具有响应式行为，所以需要通过修改key来强制刷新画布 |
There was a problem hiding this comment.
💡 Codebase verification
Missing await keyword in calls to async setUtils function
The verification has found three locations where setUtils is called without awaiting its result:
packages/canvas/render/src/RenderMain.js:108:setUtils(data, false, true)inupdateUtilsfunctionpackages/canvas/render/src/RenderMain.js:117:setUtils([], false, true)indeleteUtilsfunctionpackages/canvas/container/src/container.js:936:setUtils(useResource().resState.utils)in initialization code
These calls need to be updated to handle the asynchronous behavior by either:
- Making the calling functions async and using the
awaitkeyword - Handling the returned Promise with
.then()chains
🔗 Analysis chain
Ensure all invocations of setUtils handle the asynchronous behavior correctly
The setUtils function has been updated to be asynchronous to accommodate dynamic imports using await. Please verify that all calls to setUtils either await the function or handle the returned Promise appropriately. Failure to do so may result in unexpected behavior due to the asynchronous execution.
Run the following script to find all usages of setUtils and check if they are awaited:
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Description: Find calls to `setUtils` not using `await`
# Test: Search for `setUtils` calls not preceded by `await` or handled as a Promise
rg '[^\.]\bsetUtils\((?!.*await)' --glob 'packages/**/*.js'
Length of output: 310
Script:
#!/bin/bash
# Let's try a different approach using ast-grep to find function calls
ast-grep --pattern 'setUtils($$$)'
# Also search for async/await patterns to compare
ast-grep --pattern 'await setUtils($$$)'
Length of output: 315
Script:
#!/bin/bash
# Let's check the context of these calls to understand if they're in async functions
ast-grep --pattern 'async function $_($$$) {
$$$
setUtils($$$)
$$$
}'
# Also check for async arrow functions
ast-grep --pattern 'const $_ = async ($$$) => {
$$$
setUtils($$$)
$$$
}'
# And check the containing function for the method call in container.js
rg -B 10 'setUtils\(useResource\(\)' packages/canvas/container/src/container.js
Length of output: 605
Script:
#!/bin/bash
# Let's check the implementation of these functions to see their async context
rg -B 15 -A 5 'setUtils\(data, false, true\)' packages/canvas/render/src/RenderMain.js
# Also get the full context of the function in container.js
ast-grep --pattern 'function $_($$$) {
$$$
setUtils(useResource().resState.utils)
$$$
}'
Length of output: 655
packages/canvas/render/src/runner.js
Outdated
| const { scripts: componentsScriptsDeps = [], styles: componentsStylesDeps = [] } = window.componentsDepsMap || {} | ||
|
|
||
| Promise.all([ | ||
| ...thirdScripts.map(dynamicImportComponents), | ||
| ...scripts.map((src) => addScript(src)).concat([...thirdStyles, ...styles].map((src) => addStyle(src))) | ||
| ...componentsScriptsDeps.map(setComponents), | ||
| ...scripts.map((src) => addScript(src)).concat([...componentsStylesDeps, ...styles].map((src) => addStyle(src))) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for component loading failures
The Promise.all chain should handle individual component loading failures gracefully to prevent complete render failure.
const { scripts: componentsScriptsDeps = [], styles: componentsStylesDeps = [] } = window.componentsDepsMap || {}
Promise.all([
- ...componentsScriptsDeps.map(setComponents),
- ...scripts.map((src) => addScript(src)).concat([...componentsStylesDeps, ...styles].map((src) => addStyle(src)))
+ ...componentsScriptsDeps.map(dep =>
+ setComponents(dep).catch(err => {
+ console.error(`Failed to load component: ${dep.package}`, err);
+ return null;
+ })
+ ),
+ ...scripts.map(src =>
+ addScript(src).catch(err => {
+ console.error(`Failed to load script: ${src}`, err);
+ return null;
+ })
+ ),
+ ...[...componentsStylesDeps, ...styles].map(src =>
+ addStyle(src).catch(err => {
+ console.error(`Failed to load style: ${src}`, err);
+ return null;
+ })
+ )
]).finally(() => create(config))Let's verify the implementation of setComponents:
#!/bin/bash
# Search for setComponents implementation and error handling
rg -A 10 "export const setComponents = |function setComponents" --type js| @@ -22,7 +22,7 @@ const getComponentsSet = (schema) => { | |||
| } | |||
|
|
|||
| const parseSchema = (schema) => { | |||
| const { utils = [], componentsMap = [] } = schema | |||
| const { utils = [], componentsMap = [], packages = [] } = schema | |||
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add validation for package entries
The packages array processing lacks validation for required fields and version format.
- const { utils = [], componentsMap = [], packages = [] } = schema
+ const { utils = [], componentsMap = [], packages = [] } = validateSchema(schema)
+const validateSchema = (schema) => {
+ const { packages = [] } = schema;
+ return {
+ ...schema,
+ packages: packages.map(item => {
+ if (!item.package) {
+ console.warn('Package entry missing required "package" field:', item);
+ return null;
+ }
+ if (item.version && !isValidSemver(item.version)) {
+ console.warn(`Invalid version format for package ${item.package}:`, item.version);
+ item.version = 'latest';
+ }
+ return item;
+ }).filter(Boolean)
+ };
+}Also applies to: 48-54
| useMessage().subscribe({ | ||
| topic: 'init_canvas_deps', | ||
| callback: (deps) => { | ||
| if (canvasSrc) { | ||
| return | ||
| } | ||
| const { importMap, importStyles } = getImportMapData(getMergeMeta('engine.config')?.importMapVersion, deps) | ||
| canvasSrcDoc.value = initCanvas(importMap, importStyles).html | ||
| } | ||
| }) |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for dependency initialization
While the message subscription implementation looks good, it should handle potential errors during import map generation.
Consider applying this improvement:
useMessage().subscribe({
topic: 'init_canvas_deps',
callback: (deps) => {
if (canvasSrc) {
return
}
+ try {
const { importMap, importStyles } = getImportMapData(getMergeMeta('engine.config')?.importMapVersion, deps)
canvasSrcDoc.value = initCanvas(importMap, importStyles).html
+ } catch (error) {
+ console.error('Failed to initialize canvas dependencies:', error)
+ // Consider showing a user-friendly error message
+ }
}
})📝 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.
| useMessage().subscribe({ | |
| topic: 'init_canvas_deps', | |
| callback: (deps) => { | |
| if (canvasSrc) { | |
| return | |
| } | |
| const { importMap, importStyles } = getImportMapData(getMergeMeta('engine.config')?.importMapVersion, deps) | |
| canvasSrcDoc.value = initCanvas(importMap, importStyles).html | |
| } | |
| }) | |
| useMessage().subscribe({ | |
| topic: 'init_canvas_deps', | |
| callback: (deps) => { | |
| if (canvasSrc) { | |
| return | |
| } | |
| try { | |
| const { importMap, importStyles } = getImportMapData(getMergeMeta('engine.config')?.importMapVersion, deps) | |
| canvasSrcDoc.value = initCanvas(importMap, importStyles).html | |
| } catch (error) { | |
| console.error('Failed to initialize canvas dependencies:', error) | |
| // Consider showing a user-friendly error message | |
| } | |
| } | |
| }) |
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (5)
packages/canvas/render/src/RenderMain.js (3)
70-82: Consider enhancing error handling for function utilities.The default function
() => {}might silently hide errors when the function generation fails. Consider logging these failures or notifying users about malformed function definitions.- const defaultFn = () => {} - utilsCollection[item.name] = generateFunction(item.content.value, context) || defaultFn + try { + const fn = generateFunction(item.content.value, context) + if (!fn) { + globalNotify({ + type: 'warning', + message: `Function utility "${item.name}" could not be generated` + }) + } + utilsCollection[item.name] = fn || (() => {}) + } catch (error) { + globalNotify({ + type: 'error', + message: `Failed to generate function utility "${item.name}": ${error.message}` + }) + utilsCollection[item.name] = () => {} + }
84-102: Consider internationalizing error messages and adding retry mechanism.The implementation handles npm imports well but has a few areas for improvement:
- Error message is hardcoded in Chinese
- No retry mechanism for failed imports
- globalNotify({ - type: 'error', - message: `工具类 ${npmUtils[index].name} 加载失败，请检查CDN地址是否配置正确` - }) + const message = t('error.npm_utility_load_failed', { + name: npmUtils[index].name + }) + globalNotify({ + type: 'error', + message, + action: { + text: t('action.retry'), + onClick: () => retryImport(npmUtils[index]) + } + })
Line range hint
441-486: Document breaking changes in API signatures.The transition of
setUtilsand related functions from synchronous to asynchronous is a breaking change that affects API consumers. This needs to be clearly documented.Consider:
- Adding JSDoc comments to document the async nature of these functions
- Updating the API documentation
- Adding migration guides for existing consumers
Example JSDoc:
+/** + * Sets utility functions and npm modules in the canvas context. + * @async + * @param {Array} data - Array of utility configurations + * @param {boolean} clear - Whether to clear existing utilities + * @param {boolean} isForceRefresh - Whether to force a canvas refresh + * @returns {Promise<void>} + * @throws {Error} When npm module import fails + */ const setUtils = async (data, clear, isForceRefresh) => {packages/canvas/common/src/utils.js (2)
59-61: Enhance JSDoc documentation.The current documentation could be more descriptive. Consider adding
@returns {string[]}and more detailed description./** - * 从页面importmap中获取模块的名称 - * @returns importmap中模块的名称集合 + * Retrieves module names from the page's import map configuration + * @returns {string[]} Array of module names defined in the import map, or empty array if not found + */
Line range hint
113-124: Add error handling for missing exports and consider cache invalidation.The function should verify that the requested component exports exist and consider cache invalidation strategy.
Consider these improvements:
export const getComponents = async ({ package: pkg, script, components }) => { if (!pkg) return const modules = await dynamicImportComponentLib({ pkg, script }) Object.entries(components).forEach(([componentId, exportName]) => { + if (!(exportName in modules)) { + console.warn(`Component "${exportName}" not found in package "${pkg}"`) + return + } if (!window.TinyLowcodeComponent[componentId]) { window.TinyLowcodeComponent[componentId] = modules[exportName] } }) }Also, consider adding a cache invalidation mechanism to handle component updates.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
packages/canvas/common/src/utils.js(2 hunks)packages/canvas/render/src/RenderMain.js(1 hunks)
🔇 Additional comments (2)
packages/canvas/render/src/RenderMain.js (1)
Line range hint 109-120: Add missing await keywords to async function calls.
The updateUtils and deleteUtils functions call the async setUtils without awaiting its completion. This could lead to race conditions.
Apply this diff to fix the async calls:
-const updateUtils = (data) => {
+const updateUtils = async (data) => {
- setUtils(data, false, true)
+ await setUtils(data, false, true)
}
-const deleteUtils = (data) => {
+const deleteUtils = async (data) => {
data?.forEach((item) => {
if (utils[item.name]) {
delete utils[item.name]
}
})
- setUtils([], false, true)
+ await setUtils([], false, true)
}packages/canvas/common/src/utils.js (1)
73-73: Verify the correct property name in importMapObject.
The code accesses importMapObject.imports but comments mention "import". Verify that "imports" is the correct property name according to the Import Maps specification.
✅ Verification successful
Let me gather more information about import maps usage and specification in the codebase.
Let me check the content of the import map JSON file to verify the property name.
Property name "imports" is correct according to Import Maps specification
The code correctly uses importMapObject.imports as the property name. This is verified by:
- The import map JSON file which uses "imports" as the root property
- Multiple consistent usages across the codebase in files like
externalDeps.js,copyImportMap.js, and others - This follows the standard Import Maps specification where "imports" is the correct property name for specifying module specifier remapping
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Search for other import map usages to verify property name
rg -g '*.{js,ts,vue}' -A 2 -B 2 'importmap.*imports'
Length of output: 408
Script:
#!/bin/bash
# Search for importmap related files and configurations
rg -g '*.{js,ts,vue,json}' -i 'importmap'
# Look for any import map JSON files
fd -e json 'importmap'
# Search for import map related documentation or comments
rg -g '*.{js,ts,vue,md}' -i 'import map' -C 3
Length of output: 11260
Script:
#!/bin/bash
# Check the content of the import map JSON file
cat packages/design-core/src/preview/src/preview/importMap.json
# Look for any Import Maps specification references in documentation
rg -g '*.md' -i 'import maps? specification'
Length of output: 1893
| const { styles: canvasStyles } = window.componentsDepsMap | ||
| const newStyles = [...styles].filter((item) => !canvasStyles.has(item)) | ||
|
|
||
| newStyles.forEach((item) => canvasStyles.add(item)) | ||
|
|
||
| const promises = [...newStyles].map((src) => addStyle(src)).concat(scripts.map(dynamicImportComponents)) | ||
| const promises = [...newStyles].map((src) => addStyle(src)).concat(scripts.map(getComponents)) | ||
|
|
||
| Promise.allSettled(promises) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling for failed dependencies.
The function should handle and report failed dependency loads from Promise.allSettled.
Consider this improvement:
export const updateDependencies = ({ detail }) => {
const { scripts = [], styles = [] } = detail || {}
const { styles: canvasStyles } = window.componentsDepsMap
const newStyles = [...styles].filter((item) => !canvasStyles.has(item))
newStyles.forEach((item) => canvasStyles.add(item))
const promises = [...newStyles].map((src) => addStyle(src)).concat(scripts.map(getComponents))
- Promise.allSettled(promises)
+ Promise.allSettled(promises).then((results) => {
+ const failures = results
+ .filter(({ status }) => status === 'rejected')
+ .map(({ reason }) => reason)
+ if (failures.length > 0) {
+ console.error('Failed to load some dependencies:', failures)
+ }
+ })
}📝 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 { styles: canvasStyles } = window.componentsDepsMap | |
| const newStyles = [...styles].filter((item) => !canvasStyles.has(item)) | |
| newStyles.forEach((item) => canvasStyles.add(item)) | |
| const promises = [...newStyles].map((src) => addStyle(src)).concat(scripts.map(dynamicImportComponents)) | |
| const promises = [...newStyles].map((src) => addStyle(src)).concat(scripts.map(getComponents)) | |
| Promise.allSettled(promises) | |
| } | |
| const { styles: canvasStyles } = window.componentsDepsMap | |
| const newStyles = [...styles].filter((item) => !canvasStyles.has(item)) | |
| newStyles.forEach((item) => canvasStyles.add(item)) | |
| const promises = [...newStyles].map((src) => addStyle(src)).concat(scripts.map(getComponents)) | |
| Promise.allSettled(promises).then((results) => { | |
| const failures = results | |
| .filter(({ status }) => status === 'rejected') | |
| .map(({ reason }) => reason) | |
| if (failures.length > 0) { | |
| console.error('Failed to load some dependencies:', failures) | |
| } | |
| }) | |
| } |
| const dynamicImportComponentLib = async ({ pkg, script }) => { | ||
| if (window.TinyComponentLibs[pkg]) { | ||
| return window.TinyComponentLibs[pkg] | ||
| } | ||
|
|
||
| if (!window.TinyComponentLibs[pkg]) { | ||
| const modules = await import(/* @vite-ignore */ scriptUrl) | ||
| let modules = {} | ||
|
|
||
| window.TinyComponentLibs[pkg] = modules | ||
| try { | ||
| // 优先从importmap导入，兼容npm.script字段定义的cdn地址 | ||
| if (getImportMapKeys().includes(pkg)) { | ||
| modules = await import(/* @vite-ignore */ pkg) | ||
| } else if (script) { | ||
| modules = await import(/* @vite-ignore */ script) | ||
| } | ||
| } catch (error) { | ||
| modules = {} | ||
| } | ||
|
|
||
| Object.entries(components).forEach(([componentId, exportName]) => { | ||
| const modules = window.TinyComponentLibs[pkg] | ||
| window.TinyComponentLibs[pkg] = modules | ||
|
|
||
| return modules | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling and input validation.
The current implementation has several areas for improvement:
- Silent error handling might hide critical issues
- Missing input parameter validation
- No timeout handling for dynamic imports
Consider applying these improvements:
-const dynamicImportComponentLib = async ({ pkg, script }) => {
+const dynamicImportComponentLib = async ({ pkg, script } = {}) => {
+ if (!pkg) {
+ throw new Error('Package name is required')
+ }
+
if (window.TinyComponentLibs[pkg]) {
return window.TinyComponentLibs[pkg]
}
let modules = {}
+ const importWithTimeout = (url) => {
+ const timeout = 30000 // 30 seconds
+ return Promise.race([
+ import(/* @vite-ignore */ url),
+ new Promise((_, reject) =>
+ setTimeout(() => reject(new Error(`Import timeout: ${url}`)), timeout)
+ )
+ ])
+ }
try {
if (getImportMapKeys().includes(pkg)) {
- modules = await import(/* @vite-ignore */ pkg)
+ modules = await importWithTimeout(pkg)
} else if (script) {
- modules = await import(/* @vite-ignore */ script)
+ modules = await importWithTimeout(script)
}
} catch (error) {
- modules = {}
+ console.error(`Failed to load component library ${pkg}:`, error)
+ throw error
}
window.TinyComponentLibs[pkg] = modules
return modules
}📝 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 dynamicImportComponentLib = async ({ pkg, script }) => { | |
| if (window.TinyComponentLibs[pkg]) { | |
| return window.TinyComponentLibs[pkg] | |
| } | |
| if (!window.TinyComponentLibs[pkg]) { | |
| const modules = await import(/* @vite-ignore */ scriptUrl) | |
| let modules = {} | |
| window.TinyComponentLibs[pkg] = modules | |
| try { | |
| // 优先从importmap导入，兼容npm.script字段定义的cdn地址 | |
| if (getImportMapKeys().includes(pkg)) { | |
| modules = await import(/* @vite-ignore */ pkg) | |
| } else if (script) { | |
| modules = await import(/* @vite-ignore */ script) | |
| } | |
| } catch (error) { | |
| modules = {} | |
| } | |
| Object.entries(components).forEach(([componentId, exportName]) => { | |
| const modules = window.TinyComponentLibs[pkg] | |
| window.TinyComponentLibs[pkg] = modules | |
| return modules | |
| } | |
| const dynamicImportComponentLib = async ({ pkg, script } = {}) => { | |
| if (!pkg) { | |
| throw new Error('Package name is required') | |
| } | |
| if (window.TinyComponentLibs[pkg]) { | |
| return window.TinyComponentLibs[pkg] | |
| } | |
| let modules = {} | |
| const importWithTimeout = (url) => { | |
| const timeout = 30000 // 30 seconds | |
| return Promise.race([ | |
| import(/* @vite-ignore */ url), | |
| new Promise((_, reject) => | |
| setTimeout(() => reject(new Error(`Import timeout: ${url}`)), timeout) | |
| ) | |
| ]) | |
| } | |
| try { | |
| if (getImportMapKeys().includes(pkg)) { | |
| modules = await importWithTimeout(pkg) | |
| } else if (script) { | |
| modules = await importWithTimeout(script) | |
| } | |
| } catch (error) { | |
| console.error(`Failed to load component library ${pkg}:`, error) | |
| throw error | |
| } | |
| window.TinyComponentLibs[pkg] = modules | |
| return modules | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
🔭 Outside diff range comments (2)
packages/plugins/materials/src/meta/block/src/BlockGroupPanel.vue (1)
Line range hint
135-143: Consider improving promise chain handlingThe current promise chain could be optimized for better error handling and race condition prevention:
- }).then(() => { + }).then(async () => { isRefresh.value = true state.searchValue = '' selectedBlockArray.value.length = 0 - useResource().fetchResource({ isInit: false }) // 添加区块分组，不需要重新init页面或者区块。 + await useResource().fetchResource({ isInit: false }) // 添加区块分组，不需要重新init页面或者区块。 useNotify({ message: '添加区块成功', type: 'success' })This change ensures that:
- The resource fetch completes before showing the success notification
- Any errors in resource fetching are properly caught by the catch block
packages/plugins/materials/src/composable/useMaterial.js (1)
Line range hint
451-471: Improve type safety ingetBlockDepsThe function should validate input types and handle potential undefined values.
Apply this diff:
const getBlockDeps = (dependencies = {}) => { const { scripts = [], styles = [] } = dependencies + if (!Array.isArray(scripts) || !Array.isArray(styles)) { + return + } scripts.length && scripts.forEach((npm) => { + if (!npm?.package) { + return + } const { package: pkg, script, css, components } = npm const npmInfo = materialState.componentsDepsMap.scripts.find((item) => item.package === pkg) if (!npmInfo || !npmInfo.script) { materialState.componentsDepsMap.scripts.push({ package: pkg, script, css, components }) } else { const components = npmInfo.components || {} npmInfo.components = { ...components, ...npm.components } } }) styles?.forEach((item) => materialState.componentsDepsMap.styles.add(item)) }
🧹 Nitpick comments (4)
packages/plugins/materials/src/meta/block/src/BlockGroupPanel.vue (2)
Line range hint
28-39: Consider extracting block map logic to a composableThe block map initialization logic could be moved to a separate composable for better reusability and maintainability.
Consider creating a new composable
useBlockMap.js:// useBlockMap.js import { ref } from 'vue' export function useBlockMap() { const blockMap = ref(new Map()) const initGroupBlockMap = (groups = []) => { blockMap.value.clear() for (let group of groups) { const groupBlock = group?.blocks || [] for (let block of groupBlock) { blockMap.value.set(block.id, block) } } } const includesBlockInGroups = (blockId) => blockMap.value.has(blockId) return { initGroupBlockMap, includesBlockInGroups } }This would:
- Improve code organization
- Make the functionality more reusable
- Make testing easier
Line range hint
213-234: Consider optimizing API calls and data processingThe current implementation makes multiple separate API calls and processes results individually. This could be optimized for better performance.
- Promise.allSettled([fetchTenants(), fetchUsers(), fetchTags()]).then((results) => { + const staticTenants = [ + { name: '对所有组织开放', id: '1' }, + { name: '对当前组织开放', id: '2' } + ] + + Promise.all([fetchUsers(), fetchTags()]) + .then(([users, tags]) => { state.filters[0].children = [ - { - name: '对所有组织开放', - id: '1' - }, - { - name: '对当前组织开放', - id: '2' - } + ...staticTenants ] - state.filters[1].children = - results[1].status === 'fulfilled' - ? results[1].value.map((item) => ({ - name: item?.username, - id: item?.id - })) - : [] - state.filters[2].children = - results[2].status === 'fulfilled' ? results[2].value.map((item) => ({ name: item })) : [] - blockUsers.value = state.filters[1].children + state.filters[1].children = users.map(item => ({ + name: item?.username, + id: item?.id + })) + state.filters[2].children = tags.map(item => ({ name: item })) + blockUsers.value = state.filters[1].children + }) + .catch(error => { + message({ + message: `Failed to fetch filters: ${error.message || error}`, + status: 'error' + }) })This change:
- Removes unnecessary
fetchTenantscall since the data is static- Uses
Promise.allinstead ofPromise.allSettledfor simpler error handling- Adds proper error handling
- Simplifies the data processing logic
packages/plugins/materials/src/composable/useMaterial.js (2)
446-447: Consider error handling infetchMaterialThe function should handle potential errors when calling
setCanvasDeps.Apply this diff:
- setCanvasDeps() + try { + setCanvasDeps() + } catch (error) { + console.error('Failed to set canvas dependencies:', error) + }
485-485: Ensure safe access to canvas APIThe optional chaining on
canvasApi.valueis good, but consider adding a null check fordetail.Apply this diff:
- useCanvas().canvasApi.value?.canvasDispatch('updateDependencies', { detail: materialState.componentsDepsMap }) + const deps = materialState.componentsDepsMap + if (deps?.scripts?.length || deps?.styles?.size) { + useCanvas().canvasApi.value?.canvasDispatch('updateDependencies', { detail: deps }) + }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (8)
packages/canvas/container/src/CanvasContainer.vue(2 hunks)packages/canvas/container/src/container.js(0 hunks)packages/canvas/render/src/runner.js(2 hunks)packages/common/js/preview.js(2 hunks)packages/plugins/materials/src/composable/useMaterial.js(8 hunks)packages/plugins/materials/src/composable/useResource.js(1 hunks)packages/plugins/materials/src/meta/block/src/BlockGroupPanel.vue(1 hunks)packages/vue-generator/src/plugins/genDependenciesPlugin.js(1 hunks)
💤 Files with no reviewable changes (1)
- packages/canvas/container/src/container.js
🚧 Files skipped from review as they are similar to previous changes (4)
- packages/canvas/render/src/runner.js
- packages/common/js/preview.js
- packages/vue-generator/src/plugins/genDependenciesPlugin.js
- packages/canvas/container/src/CanvasContainer.vue
🔇 Additional comments (4)
packages/plugins/materials/src/composable/useResource.js (1)
36-37: LGTM: New state property for canvas dependencies
The addition of canvasDeps with separate collections for scripts and styles is a good approach. Using Set for styles ensures uniqueness and prevents duplicates.
packages/plugins/materials/src/composable/useMaterial.js (3)
41-42: Update state management for dependencies
Good refactoring to replace thirdPartyDeps with componentsDepsMap. The new structure better represents the relationship between components and their dependencies.
274-288:
Enhance error handling in getUtilsDependencies
The function should handle potential undefined properties in the utility content.
Apply this diff:
return utils
- .filter((item) => item.type === 'npm' && item.content.cdnLink)
+ .filter((item) => item.type === 'npm' && item.content?.cdnLink)
.map((item) => {
- const { package: pkg, version, cdnLink } = item.content
+ const { package: pkg = '', version = '', cdnLink = '' } = item.content || {}
return {
package: pkg,
version,
script: cdnLink
}
})Likely invalid or redundant comment.
293-309: Verify dependency updates in setCanvasDeps
The function needs to ensure that duplicate scripts are not added and that the message payload is properly structured.
✅ Verification successful
Duplicate prevention and message structure are properly implemented
The code implementation in setCanvasDeps is correct and safe:
- It properly checks for duplicates using
scripts.find()before adding new utilities - The message payload structure is consistent with its consumer in DesignCanvas.vue
- The scripts array is properly initialized and managed throughout the file
- The spread operator
[...styles]creates a new array, preventing unintended mutations
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash
# Check for potential duplicate script entries
ast-grep --pattern 'useMessage().publish({ topic: "init_canvas_deps", data: $_ })'
Length of output: 84
Script:
#!/bin/bash
# Let's try a different pattern to match the message publishing
ast-grep --pattern 'useMessage().publish({
topic: "init_canvas_deps",
data: $$$
})'
# Also search for other usages of init_canvas_deps
rg "init_canvas_deps" -A 3
# Check the structure of getUtilsDependencies
ast-grep --pattern 'getUtilsDependencies = () => $$$'
Length of output: 796
Script:
#!/bin/bash
# Let's check how scripts array is populated and used
rg "scripts\." -A 3 packages/plugins/materials/src/composable/useMaterial.js
# Check the structure of useResource and resState
ast-grep --pattern 'const { scripts, styles } = useResource().resState.canvasDeps'
# Look for the definition of scripts array
rg "scripts =" packages/plugins/materials/src/composable/useMaterial.js
Length of output: 1489
| const parseMaterialsDependencies = (materialBundle) => { | ||
| const { packages, components } = materialBundle | ||
|
|
||
| const { scripts: scriptsDeps, styles: stylesDeps } = useResource().resState.canvasDeps | ||
|
|
||
| packages?.forEach((pkg) => { | ||
| if (scriptsDeps.find((item) => item.package === pkg.package)) { | ||
| return | ||
| } | ||
|
|
||
| scriptsDeps.push(pkg) | ||
|
|
||
| if (!pkg.css) { | ||
| return | ||
| } | ||
|
|
||
| if (Array.isArray(pkg.css)) { | ||
| pkg.css.map(stylesDeps.add) | ||
| } else { | ||
| stylesDeps.add(pkg.css) | ||
| } | ||
| }) | ||
|
|
||
| // 解析组件npm字段 | ||
| const { scripts, styles } = generateThirdPartyDeps(components) | ||
| // 合并到canvasDeps中 | ||
| scripts.forEach((item) => { | ||
| const dep = scriptsDeps.find((dep) => dep.package === item.package) | ||
|
|
||
| if (dep) { | ||
| dep.components = { ...dep.components, ...item.components } | ||
| } | ||
| }) | ||
|
|
||
| if (!styles) { | ||
| return | ||
| } | ||
|
|
||
| if (Array.isArray(styles)) { | ||
| styles.map(stylesDeps.add) | ||
| } else { | ||
| stylesDeps.add(styles) | ||
| } | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Improve error handling in parseMaterialsDependencies
The function should handle edge cases and validate input data structure.
Apply this diff:
const parseMaterialsDependencies = (materialBundle) => {
+ if (!materialBundle || typeof materialBundle !== 'object') {
+ return
+ }
const { packages = [], components = [] } = materialBundle
const { scripts: scriptsDeps, styles: stylesDeps } = useResource().resState.canvasDeps
packages?.forEach((pkg) => {
+ if (!pkg?.package) {
+ return
+ }
if (scriptsDeps.find((item) => item.package === pkg.package)) {
return
}
scriptsDeps.push(pkg)
if (!pkg.css) {
return
}
if (Array.isArray(pkg.css)) {
pkg.css.map(stylesDeps.add)
} else {
stylesDeps.add(pkg.css)
}
})Committable suggestion skipped: line range outside the PR's diff.
| window.TinyComponentLibs[pkg] = modules | ||
| try { | ||
| // 优先从importmap导入，兼容npm.script字段定义的cdn地址 | ||
| if (getImportMapKeys().includes(pkg)) { |
There was a problem hiding this comment.
这里建议做一下缓存，不然每次调用 getImportMapKeys 都要重新读取一次。
| @@ -113,7 +113,7 @@ export default { | |||
| const beforeCanvasReady = () => { | |||
| if (iframe.value) { | |||
| const win = iframe.value.contentWindow | |||
| win.thirdPartyDeps = useMaterial().materialState.thirdPartyDeps | |||
| win.componentsDeps = useResource().resState.canvasDeps.scripts.filter((item) => item.components) | |||
There was a problem hiding this comment.
冲突要解一下， resState 变成 appSchemaState 了
| }) | ||
|
|
||
| const npmUtils = data.filter((item) => item.type === 'npm' && item.content.cdnLink) | ||
| const results = await Promise.allSettled(npmUtils.map((item) => import(/* @vite-ignore */ item.content.package))) |
There was a problem hiding this comment.
这里要区分情况，一种情况是：在 importMap 已经存在，一种是不存在，不存在的情况下，我理解是 import 是需要读取 cdnLink 的
| return { | ||
| package: pkg, | ||
| version, | ||
| script: cdnLink |
| } | ||
|
|
||
| /** | ||
| * 组装画布的依赖，通知画布初始化或更新importmap |
There was a problem hiding this comment.
importmap 没法更新，只能销毁整个 iframe 再重新构建
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (9)
packages/canvas/render/src/RenderMain.js (4)
99-103: Internationalize error messagesThe error message is hardcoded in Chinese. Consider using i18n for better internationalization support.
- message: `工具类 ${npmUtils[index].name} 加载失败，请检查CDN地址是否配置正确` + message: t('utils.loadError', { name: npmUtils[index].name })
97-105: Enhance error handling and loggingConsider adding detailed error logging and structured error handling.
results.forEach((res, index) => { if (res.status !== 'fulfilled') { + console.error(`Failed to load utility ${npmUtils[index].name}:`, res.reason) globalNotify({ type: 'error', - message: `工具类 ${npmUtils[index].name} 加载失败，请检查CDN地址是否配置正确` + message: t('utils.loadError', { name: npmUtils[index].name }), + details: res.reason?.message }) return }
87-92: Validate function-type utilitiesConsider adding validation for function-type utilities to ensure they meet expected interfaces.
data .filter((item) => item.type === 'function') .forEach((item) => { const defaultFn = () => {} + const fn = generateFunction(item.content.value, context) + if (fn && typeof fn !== 'function') { + console.warn(`Utility ${item.name} is not a function`) + return + } - utilsCollection[item.name] = generateFunction(item.content.value, context) || defaultFn + utilsCollection[item.name] = fn || defaultFn })
106-111: Validate npm module exportsConsider adding validation for npm module exports to ensure they meet expected interfaces.
const module = res.value const { name, content } = npmUtils[index] const { exportName, destructuring } = content +const exportedValue = destructuring ? module[exportName] : module.default +if (exportedValue === undefined) { + console.warn(`Export ${exportName} not found in module ${name}`) + return +} -utilsCollection[name] = destructuring ? module[exportName] : module.default +utilsCollection[name] = exportedValuepackages/plugins/materials/src/composable/useMaterial.js (2)
304-320: Add debouncing for canvas dependency updates.The
setCanvasDepsfunction might be called frequently during initialization. Consider debouncing the message publishing.Apply this diff to add debouncing:
+const debouncedPublish = utils.debounce((data) => { + useMessage().publish({ + topic: 'init_canvas_deps', + data + }) +}, 100) const setCanvasDeps = () => { const { scripts, styles } = useResource().resState.canvasDeps getUtilsDependencies().forEach((util) => { if (scripts.find((item) => util.package === item.package)) { return } scripts.push(util) }) - useMessage().publish({ - topic: 'init_canvas_deps', - data: { scripts: scripts, styles: [...styles] } - }) + debouncedPublish({ scripts: scripts, styles: [...styles] }) }
468-479: Ensure atomic updates for dependency maps.The block dependency handling should ensure atomic updates to prevent race conditions.
Apply this diff to ensure atomic updates:
- materialState.componentsDepsMap.scripts.push({ package: pkg, script, css, components }) + const newDeps = [...materialState.componentsDepsMap.scripts] + newDeps.push({ package: pkg, script, css, components }) + materialState.componentsDepsMap.scripts = newDepsdesigner-demo/public/mock/bundle.json (3)
14041-14047: Consider adding integrity hashes for CDN resourcesFor the TinyVue package CDN resources, consider adding integrity hashes to prevent tampering:
{ "name": "TinyVue组件库", "package": "@opentiny/vue", "version": "3.14.0", "destructuring": true, "script": "https://unpkg.com/@opentiny/vue@~3.14/runtime/tiny-vue.mjs", + "scriptIntegrity": "<sha384-hash>", "css": "https://unpkg.com/@opentiny/vue-theme@~3.14/index.css", + "cssIntegrity": "<sha384-hash>" }
14048-14054: Consider pinning exact versions for CDN URLsThe Element Plus package uses tilde ranges in CDN URLs which could potentially pull in breaking changes. Consider pinning to exact versions:
{ "name": "element-plus组件库", "package": "element-plus", "version": "2.4.2", - "script": "https://unpkg.com/[email protected]/dist/index.full.mjs", + "script": "https://unpkg.com/[email protected]/dist/index.full.mjs", "css": "https://unpkg.com/[email protected]/dist/index.css" }
14039-14054: Consider adding package validation metadataThe packages section could benefit from additional metadata to help validate package integrity and compatibility:
"packages": [ { "name": "TinyVue组件库", "package": "@opentiny/vue", "version": "3.14.0", + "engines": { + "node": ">=14.0.0", + "npm": ">=6.0.0" + }, + "peerDependencies": { + "vue": "^3.0.0" + } } // ... other packages ]
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (13)
designer-demo/public/mock/bundle.json(33 hunks)packages/canvas/DesignCanvas/src/DesignCanvas.vue(1 hunks)packages/canvas/DesignCanvas/src/importMap.js(2 hunks)packages/canvas/container/src/CanvasContainer.vue(2 hunks)packages/canvas/container/src/container.js(0 hunks)packages/canvas/render/src/RenderMain.js(2 hunks)packages/canvas/render/src/runner.js(2 hunks)packages/common/js/preview.js(2 hunks)packages/design-core/src/preview/src/preview/Preview.vue(1 hunks)packages/plugins/materials/src/composable/useMaterial.js(8 hunks)packages/plugins/materials/src/composable/useResource.js(1 hunks)packages/plugins/materials/src/meta/block/src/BlockGroupPanel.vue(1 hunks)packages/toolbars/generate-code/src/Main.vue(1 hunks)
💤 Files with no reviewable changes (1)
- packages/canvas/container/src/container.js
🚧 Files skipped from review as they are similar to previous changes (6)
- packages/toolbars/generate-code/src/Main.vue
- packages/canvas/DesignCanvas/src/importMap.js
- packages/plugins/materials/src/meta/block/src/BlockGroupPanel.vue
- packages/canvas/container/src/CanvasContainer.vue
- packages/common/js/preview.js
- packages/plugins/materials/src/composable/useResource.js
🔇 Additional comments (10)
packages/canvas/render/src/RenderMain.js (1)
Line range hint
71-114: Consider handling importMap existence scenariosBased on the previous review comments, the code should handle two scenarios:
- When importMap exists
- When importMap doesn't exist (should use cdnLink)
Let's verify the importMap handling:
Consider refactoring the npm utils import logic to handle both scenarios:
const npmUtils = data.filter((item) => item.type === 'npm' && item.content.cdnLink) -const results = await Promise.allSettled(npmUtils.map((item) => import(/* @vite-ignore */ item.content.package))) +const results = await Promise.allSettled(npmUtils.map(async (item) => { + const { content } = item + const importPath = window.host?.importMap?.[content.package] || content.cdnLink + return import(/* @vite-ignore */ importPath) +}))packages/canvas/render/src/runner.js (2)
14-14: LGTM! Import statement updated to support new dependency management.The import statement has been updated to include new utilities that support the revised dependency management approach.
84-85: 🛠️ Refactor suggestionVerify error handling for dependency loading.
While the dependency loading logic is sound, consider adding error handling to gracefully handle component loading failures.
Apply this diff to improve error handling:
Promise.all([ - ...window.componentsDeps.map(getComponents), - ...scripts.map((src) => addScript(src)).concat(styles.map((src) => addStyle(src))) + ...window.componentsDeps.map(dep => + getComponents(dep).catch(err => { + console.error(`Failed to load component:`, err); + return null; + }) + ), + ...scripts.map(src => + addScript(src).catch(err => { + console.error(`Failed to load script: ${src}`, err); + return null; + }) + ), + ...styles.map(src => + addStyle(src).catch(err => { + console.error(`Failed to load style: ${src}`, err); + return null; + }) + ) ]).finally(() => create(config))packages/design-core/src/preview/src/preview/Preview.vue (1)
83-83: LGTM! Simplified import map handling.The direct usage of
store.setImportMap(importMapData)improves code clarity by removing unnecessary complexity.packages/canvas/DesignCanvas/src/DesignCanvas.vue (2)
62-62: LGTM! Enhanced reactivity with ref().Converting
canvasSrcDocto useref()ensures proper reactivity in Vue's composition API.
64-75: 🛠️ Refactor suggestionAdd error handling for dependency initialization.
The message subscription implementation should handle potential errors during import map generation.
Apply this diff to improve error handling:
useMessage().subscribe({ topic: 'init_canvas_deps', callback: (deps) => { if (canvasSrc) { return } + try { const { importMap, importStyles } = getImportMapData(getMergeMeta('engine.config')?.importMapVersion, deps) canvasSrcDoc.value = initCanvas(importMap, importStyles).html + } catch (error) { + console.error('Failed to initialize canvas dependencies:', error) + // Consider emitting an error event to notify the parent + useMessage().publish({ + topic: 'canvas_init_error', + data: { error } + }) + } } })Likely invalid or redundant comment.
packages/plugins/materials/src/composable/useMaterial.js (3)
42-43: LGTM! Improved state management structure.The new
componentsDepsMapstructure provides better organization for scripts and styles dependencies.
285-299: 🛠️ Refactor suggestionAdd validation for utility dependencies.
The
getUtilsDependenciesfunction should validate the utility structure before accessing properties.Apply this diff to add validation:
const getUtilsDependencies = () => { const { utils } = useResource().resState return utils - .filter((item) => item.type === 'npm' && item.content.cdnLink) + .filter((item) => { + return item?.type === 'npm' && + item?.content?.cdnLink && + item?.content?.package + }) .map((item) => { const { package: pkg, version, cdnLink } = item.content return { package: pkg, version, script: cdnLink } }) }Likely invalid or redundant comment.
323-366: 🛠️ Refactor suggestionAdd input validation for material dependencies.
The
parseMaterialsDependenciesfunction should validate the input structure.Apply this diff to add validation:
const parseMaterialsDependencies = (materialBundle) => { + if (!materialBundle || typeof materialBundle !== 'object') { + console.warn('Invalid material bundle provided') + return + } const { packages = [], components = [] } = materialBundle + if (!Array.isArray(packages) || !Array.isArray(components)) { + console.warn('Invalid packages or components structure') + return + } const { scripts: scriptsDeps, styles: stylesDeps } = useResource().resState.canvasDeps packages?.forEach((pkg) => { + if (!pkg?.package) { + console.warn('Invalid package structure:', pkg) + return + } if (scriptsDeps.find((item) => item.package === pkg.package)) { return } scriptsDeps.push(pkg) if (!pkg.css) { return } if (Array.isArray(pkg.css)) { pkg.css.map(stylesDeps.add) } else { stylesDeps.add(pkg.css) } })Likely invalid or redundant comment.
designer-demo/public/mock/bundle.json (1)
14040-14054: Verify package versions and security advisoriesThe package configurations reference specific versions that should be verified for security and compatibility.
✅ Verification successful
Consider updating package versions for improved features and security
Both packages have newer versions available that include bug fixes and improvements:
- @opentiny/vue: 3.14.0 → 3.21.0
- element-plus: 2.4.2 → 2.9.2
No critical security vulnerabilities found in current versions.
🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Check for security advisories and latest versions of the specified packages # Check latest versions echo "Checking @opentiny/vue latest version..." curl -s https://registry.npmjs.org/@opentiny/vue/latest | jq '.version' echo "Checking element-plus latest version..." curl -s https://registry.npmjs.org/element-plus/latest | jq '.version' # Check for security advisories echo "Checking security advisories..." gh api graphql -f query=' { securityVulnerabilities(first: 10, ecosystem: NPM, package: "element-plus") { nodes { advisory { summary severity publishedAt } vulnerableVersionRange firstPatchedVersion { identifier } } } }'Length of output: 998
There was a problem hiding this comment.
Actionable comments posted: 4
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/plugins/bridge/src/js/resource.js(2 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (3)
packages/plugins/bridge/src/js/resource.js (3)
14-14: LGTM! Well-structured importsThe import statement is properly organized and includes all necessary utilities from the meta-register package.
178-179: LGTM! Improved code readabilityGood use of async/await and the
isEditvariable makes the code more maintainable.Also applies to: 181-182
215-221: 🛠️ Refactor suggestionMove success notification after all operations complete
The success notification should only be shown after confirming all operations (including canvas updates) have completed successfully.
Consider moving the success notification and state updates into a
finallyblock:- useNotify({ - type: 'success', - message: `${isEdit ? '修改' : '创建'}成功` - }) - emit('refresh', state.type) - state.refresh = true - callback() + try { + // ... existing try blocks ... + } catch (error) { + // ... error handling ... + } finally { + if (!error) { + useNotify({ + type: 'success', + message: `${isEdit ? '修改' : '创建'}成功` + }) + emit('refresh', state.type) + state.refresh = true + callback() + } + }Likely invalid or redundant comment.
| const result = await requestAddReSource(data) | ||
|
|
||
| // 更新画布工具函数环境，保证渲染最新工具类返回值, 并触发画布的强制刷新 | ||
| generateBridgeUtil(getAppId()) | ||
| useNotify({ | ||
| type: 'success', | ||
| message: '创建成功' | ||
| }) | ||
| emit('refresh', state.type) | ||
| state.refresh = true | ||
| callback() | ||
| } | ||
| }) | ||
| if (result) { | ||
| useResource().appSchemaState[data.category].push(result) | ||
| } |
There was a problem hiding this comment.
Add error handling for resource addition
Similar to the edit flow, the resource addition lacks proper error handling.
Here's a suggested improvement:
- const result = await requestAddReSource(data)
-
- if (result) {
- useResource().appSchemaState[data.category].push(result)
- }
+ try {
+ const result = await requestAddReSource(data)
+ if (!result) {
+ throw new Error('Add resource request failed')
+ }
+ useResource().appSchemaState[data.category].push(result)
+ } catch (error) {
+ useNotify({
+ type: 'error',
+ message: `创建失败: ${error.message}`
+ })
+ return
+ }📝 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 result = await requestAddReSource(data) | |
| // 更新画布工具函数环境，保证渲染最新工具类返回值, 并触发画布的强制刷新 | |
| generateBridgeUtil(getAppId()) | |
| useNotify({ | |
| type: 'success', | |
| message: '创建成功' | |
| }) | |
| emit('refresh', state.type) | |
| state.refresh = true | |
| callback() | |
| } | |
| }) | |
| if (result) { | |
| useResource().appSchemaState[data.category].push(result) | |
| } | |
| try { | |
| const result = await requestAddReSource(data) | |
| if (!result) { | |
| throw new Error('Add resource request failed') | |
| } | |
| useResource().appSchemaState[data.category].push(result) | |
| } catch (error) { | |
| useNotify({ | |
| type: 'error', | |
| message: `创建失败: ${error.message}` | |
| }) | |
| return | |
| } |
| if (isEdit) { | ||
| data.id = state.resource.id | ||
| const result = await requestUpdateReSource(data) | ||
|
|
||
| generateBridgeUtil(getAppId()) | ||
| if (result) { | ||
| const index = useResource().appSchemaState[data.category].findIndex((item) => item.name === result.name) | ||
|
|
||
| if (index === -1) { | ||
| useNotify({ | ||
| type: 'success', | ||
| message: '修改成功' | ||
| type: 'error', | ||
| message: '修改失败' | ||
| }) | ||
|
|
||
| emit('refresh', state.type) | ||
| state.refresh = true | ||
| callback() | ||
| return | ||
| } | ||
| }) | ||
|
|
||
| useResource().appSchemaState[data.category][index] = result | ||
| } |
There was a problem hiding this comment.
Add error handling for async operations
The async operations lack proper error handling. Consider wrapping the code in try-catch blocks to handle potential failures gracefully.
Here's a suggested improvement:
if (isEdit) {
data.id = state.resource.id
- const result = await requestUpdateReSource(data)
-
- if (result) {
- const index = useResource().appSchemaState[data.category].findIndex((item) => item.name === result.name)
-
- if (index === -1) {
- useNotify({
- type: 'error',
- message: '修改失败'
- })
-
- return
- }
-
- useResource().appSchemaState[data.category][index] = result
- }
+ try {
+ const result = await requestUpdateReSource(data)
+ if (!result) {
+ throw new Error('Update resource request failed')
+ }
+
+ const index = useResource().appSchemaState[data.category].findIndex((item) => item.name === result.name)
+ if (index === -1) {
+ throw new Error('Resource not found in app schema')
+ }
+
+ useResource().appSchemaState[data.category][index] = result
+ } catch (error) {
+ useNotify({
+ type: 'error',
+ message: `修改失败: ${error.message}`
+ })
+ return
+ }📝 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.
| if (isEdit) { | |
| data.id = state.resource.id | |
| const result = await requestUpdateReSource(data) | |
| generateBridgeUtil(getAppId()) | |
| if (result) { | |
| const index = useResource().appSchemaState[data.category].findIndex((item) => item.name === result.name) | |
| if (index === -1) { | |
| useNotify({ | |
| type: 'success', | |
| message: '修改成功' | |
| type: 'error', | |
| message: '修改失败' | |
| }) | |
| emit('refresh', state.type) | |
| state.refresh = true | |
| callback() | |
| return | |
| } | |
| }) | |
| useResource().appSchemaState[data.category][index] = result | |
| } | |
| if (isEdit) { | |
| data.id = state.resource.id | |
| try { | |
| const result = await requestUpdateReSource(data) | |
| if (!result) { | |
| throw new Error('Update resource request failed') | |
| } | |
| const index = useResource().appSchemaState[data.category].findIndex((item) => item.name === result.name) | |
| if (index === -1) { | |
| throw new Error('Resource not found in app schema') | |
| } | |
| useResource().appSchemaState[data.category][index] = result | |
| } catch (error) { | |
| useNotify({ | |
| type: 'error', | |
| message: `修改失败: ${error.message}` | |
| }) | |
| return | |
| } |
| // 更新画布工具函数环境，保证渲染最新工具类返回值, 并触发画布的强制刷新 | ||
| generateBridgeUtil(getAppId()) | ||
|
|
||
| if (data.type === 'npm') { | ||
| // 更新画布import并刷新画布 | ||
| useMaterial().setCanvasDeps() | ||
| } |
There was a problem hiding this comment.
Handle potential failures in canvas updates
The canvas-related operations should be wrapped in try-catch blocks to handle potential failures.
Here's a suggested improvement:
- // 更新画布工具函数环境，保证渲染最新工具类返回值, 并触发画布的强制刷新
- generateBridgeUtil(getAppId())
-
- if (data.type === 'npm') {
- // 更新画布import并刷新画布
- useMaterial().setCanvasDeps()
- }
+ try {
+ // 更新画布工具函数环境，保证渲染最新工具类返回值, 并触发画布的强制刷新
+ await generateBridgeUtil(getAppId())
+
+ if (data.type === 'npm') {
+ // 更新画布import并刷新画布
+ await useMaterial().setCanvasDeps()
+ }
+ } catch (error) {
+ useNotify({
+ type: 'error',
+ message: `画布更新失败: ${error.message}`
+ })
+ return
+ }Committable suggestion skipped: line range outside the PR's diff.
| export const saveResource = async (data, callback, emit) => { | ||
| const isEdit = getActionType() === ACTION_TYPE.Edit | ||
|
|
||
| useResource().appSchemaState[data.category][index] = result | ||
|
|
||
| // 更新画布工具函数环境，保证渲染最新工具类返回值, 并触发画布的强制刷新 | ||
| if (isEdit) { | ||
| data.id = state.resource.id | ||
| const result = await requestUpdateReSource(data) | ||
|
|
||
| generateBridgeUtil(getAppId()) | ||
| if (result) { | ||
| const index = useResource().appSchemaState[data.category].findIndex((item) => item.name === result.name) | ||
|
|
||
| if (index === -1) { | ||
| useNotify({ | ||
| type: 'success', | ||
| message: '修改成功' | ||
| type: 'error', | ||
| message: '修改失败' | ||
| }) | ||
|
|
||
| emit('refresh', state.type) | ||
| state.refresh = true | ||
| callback() | ||
| return | ||
| } | ||
| }) | ||
|
|
||
| useResource().appSchemaState[data.category][index] = result | ||
| } | ||
| } else { | ||
| requestAddReSource(data).then((result) => { | ||
| if (result) { | ||
| useResource().appSchemaState[data.category].push(result) | ||
| const result = await requestAddReSource(data) | ||
|
|
||
| // 更新画布工具函数环境，保证渲染最新工具类返回值, 并触发画布的强制刷新 | ||
| generateBridgeUtil(getAppId()) | ||
| useNotify({ | ||
| type: 'success', | ||
| message: '创建成功' | ||
| }) | ||
| emit('refresh', state.type) | ||
| state.refresh = true | ||
| callback() | ||
| } | ||
| }) | ||
| if (result) { | ||
| useResource().appSchemaState[data.category].push(result) | ||
| } | ||
| } | ||
|
|
||
| // 更新画布工具函数环境，保证渲染最新工具类返回值, 并触发画布的强制刷新 | ||
| generateBridgeUtil(getAppId()) | ||
|
|
||
| if (data.type === 'npm') { | ||
| // 更新画布import并刷新画布 | ||
| useMaterial().setCanvasDeps() | ||
| } | ||
|
|
||
| useNotify({ | ||
| type: 'success', | ||
| message: `${isEdit ? '修改' : '创建'}成功` | ||
| }) | ||
| emit('refresh', state.type) | ||
| state.refresh = true | ||
| callback() |
There was a problem hiding this comment.
Add input validation for npm package data
The function should validate npm package names and versions before processing them to prevent potential security issues.
Consider adding validation:
export const saveResource = async (data, callback, emit) => {
+ // Validate npm package data
+ if (data.type === 'npm') {
+ const { package: pkgName, version } = data.content
+
+ // Validate package name format
+ if (!/^(@[a-z0-9-~][a-z0-9-._~]*\/)?[a-z0-9-~][a-z0-9-._~]*$/.test(pkgName)) {
+ useNotify({
+ type: 'error',
+ message: '无效的包名格式'
+ })
+ return
+ }
+
+ // Validate version format
+ if (!/^(\d+\.\d+\.\d+)(-[a-z]+\.\d+)?$/.test(version)) {
+ useNotify({
+ type: 'error',
+ message: '无效的版本号格式'
+ })
+ return
+ }
+ }
const isEdit = getActionType() === ACTION_TYPE.Edit
// ... rest of the function📝 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 saveResource = async (data, callback, emit) => { | |
| const isEdit = getActionType() === ACTION_TYPE.Edit | |
| useResource().appSchemaState[data.category][index] = result | |
| // 更新画布工具函数环境，保证渲染最新工具类返回值, 并触发画布的强制刷新 | |
| if (isEdit) { | |
| data.id = state.resource.id | |
| const result = await requestUpdateReSource(data) | |
| generateBridgeUtil(getAppId()) | |
| if (result) { | |
| const index = useResource().appSchemaState[data.category].findIndex((item) => item.name === result.name) | |
| if (index === -1) { | |
| useNotify({ | |
| type: 'success', | |
| message: '修改成功' | |
| type: 'error', | |
| message: '修改失败' | |
| }) | |
| emit('refresh', state.type) | |
| state.refresh = true | |
| callback() | |
| return | |
| } | |
| }) | |
| useResource().appSchemaState[data.category][index] = result | |
| } | |
| } else { | |
| requestAddReSource(data).then((result) => { | |
| if (result) { | |
| useResource().appSchemaState[data.category].push(result) | |
| const result = await requestAddReSource(data) | |
| // 更新画布工具函数环境，保证渲染最新工具类返回值, 并触发画布的强制刷新 | |
| generateBridgeUtil(getAppId()) | |
| useNotify({ | |
| type: 'success', | |
| message: '创建成功' | |
| }) | |
| emit('refresh', state.type) | |
| state.refresh = true | |
| callback() | |
| } | |
| }) | |
| if (result) { | |
| useResource().appSchemaState[data.category].push(result) | |
| } | |
| } | |
| // 更新画布工具函数环境，保证渲染最新工具类返回值, 并触发画布的强制刷新 | |
| generateBridgeUtil(getAppId()) | |
| if (data.type === 'npm') { | |
| // 更新画布import并刷新画布 | |
| useMaterial().setCanvasDeps() | |
| } | |
| useNotify({ | |
| type: 'success', | |
| message: `${isEdit ? '修改' : '创建'}成功` | |
| }) | |
| emit('refresh', state.type) | |
| state.refresh = true | |
| callback() | |
| export const saveResource = async (data, callback, emit) => { | |
| // Validate npm package data | |
| if (data.type === 'npm') { | |
| const { package: pkgName, version } = data.content | |
| // Validate package name format | |
| if (!/^(@[a-z0-9-~][a-z0-9-._~]*\/)?[a-z0-9-~][a-z0-9-._~]*$/.test(pkgName)) { | |
| useNotify({ | |
| type: 'error', | |
| message: '无效的包名格式' | |
| }) | |
| return | |
| } | |
| // Validate version format | |
| if (!/^(\d+\.\d+\.\d+)(-[a-z]+\.\d+)?$/.test(version)) { | |
| useNotify({ | |
| type: 'error', | |
| message: '无效的版本号格式' | |
| }) | |
| return | |
| } | |
| } | |
| const isEdit = getActionType() === ACTION_TYPE.Edit | |
| if (isEdit) { | |
| data.id = state.resource.id | |
| const result = await requestUpdateReSource(data) | |
| if (result) { | |
| const index = useResource().appSchemaState[data.category].findIndex((item) => item.name === result.name) | |
| if (index === -1) { | |
| useNotify({ | |
| type: 'error', | |
| message: '修改失败' | |
| }) | |
| return | |
| } | |
| useResource().appSchemaState[data.category][index] = result | |
| } | |
| } else { | |
| const result = await requestAddReSource(data) | |
| if (result) { | |
| useResource().appSchemaState[data.category].push(result) | |
| } | |
| } | |
| // 更新画布工具函数环境，保证渲染最新工具类返回值, 并触发画布的强制刷新 | |
| generateBridgeUtil(getAppId()) | |
| if (data.type === 'npm') { | |
| // 更新画布import并刷新画布 | |
| useMaterial().setCanvasDeps() | |
| } | |
| useNotify({ | |
| type: 'success', | |
| message: `${isEdit ? '修改' : '创建'}成功` | |
| }) | |
| emit('refresh', state.type) | |
| state.refresh = true | |
| callback() |
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
packages/plugins/materials/src/composable/useMaterial.js (3)
42-43: Consider using TypeScript interfaces for state objects.The
componentsDepsMapandpackagesproperties would benefit from TypeScript interfaces to ensure type safety and provide better documentation of the expected structure.interface ComponentsDepsMap { scripts: Array<{ package: string; script: string; components?: Record<string, string>; }>; styles: Set<string>; } interface MaterialState { components: any[]; blocks: any[]; componentsDepsMap: ComponentsDepsMap; packages: Array<{ package: string; version: string; script?: string; css?: string | string[]; }>; }
304-320: Consider adding error handling for message publishing.The function should handle potential errors during message publishing and provide feedback.
const setCanvasDeps = () => { const { scripts, styles } = useResource().appSchemaState.canvasDeps getUtilsDependencies().forEach((util) => { if (scripts.find((item) => util.package === item.package)) { return } scripts.push(util) }) + try { useMessage().publish({ topic: 'init_canvas_deps', data: { scripts: scripts, styles: [...styles] } }) + } catch (error) { + console.error('Failed to publish canvas dependencies:', error) + } }
467-471: Consider memory management for Set operations.The Set of styles could grow indefinitely. Consider implementing a cleanup mechanism.
const getBlockDeps = (dependencies = {}) => { const { scripts = [], styles = [] } = dependencies + const MAX_STYLES = 1000 // Adjust based on your needs scripts.length && scripts.forEach((npm) => { const { package: pkg, script, css, components } = npm const npmInfo = materialState.componentsDepsMap.scripts.find((item) => item.package === pkg) if (!npmInfo || !npmInfo.script) { materialState.componentsDepsMap.scripts.push({ package: pkg, script, css, components }) } else { const components = npmInfo.components || {} npmInfo.components = { ...components, ...npm.components } } }) - styles?.forEach((item) => materialState.componentsDepsMap.styles.add(item)) + styles?.forEach((item) => { + materialState.componentsDepsMap.styles.add(item) + if (materialState.componentsDepsMap.styles.size > MAX_STYLES) { + const iterator = materialState.componentsDepsMap.styles.values() + materialState.componentsDepsMap.styles.delete(iterator.next().value) + } + }) }Also applies to: 479-479
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/canvas/container/src/CanvasContainer.vue(2 hunks)packages/common/js/preview.js(2 hunks)packages/plugins/materials/src/composable/useMaterial.js(8 hunks)
🚧 Files skipped from review as they are similar to previous changes (2)
- packages/common/js/preview.js
- packages/canvas/container/src/CanvasContainer.vue
🔇 Additional comments (2)
packages/plugins/materials/src/composable/useMaterial.js (2)
285-299:⚠️ Potential issueAdd null checks for nested properties.
The function assumes
item.contentalways exists and has acdnLinkproperty. This could lead to runtime errors.Apply this diff to add proper null checks:
return utils - .filter((item) => item.type === 'npm' && item.content.cdnLink) + .filter((item) => item.type === 'npm' && item.content?.cdnLink) .map((item) => { - const { package: pkg, version, cdnLink } = item.content + const { package: pkg, version = '', cdnLink = '' } = item.content || {} return { package: pkg, version, script: cdnLink } })Likely invalid or redundant comment.
323-366:⚠️ Potential issueAdd input validation and error handling.
The function should validate the input structure and handle potential errors.
Apply this diff to add proper validation:
const parseMaterialsDependencies = (materialBundle) => { + if (!materialBundle || typeof materialBundle !== 'object') { + console.warn('Invalid material bundle provided') + return + } const { packages = [], components = [] } = materialBundle const { scripts: scriptsDeps, styles: stylesDeps } = useResource().appSchemaState.canvasDeps packages?.forEach((pkg) => { + if (!pkg?.package) { + console.warn('Invalid package configuration:', pkg) + return + } if (scriptsDeps.find((item) => item.package === pkg.package)) { return }Likely invalid or redundant comment.
| @@ -408,7 +490,7 @@ const updateCanvasDependencies = (blocks) => { | |||
| getBlockDeps(block.content.dependencies) | |||
| }) | |||
|
|
|||
| useCanvas().canvasApi.value?.canvasDispatch('updateDependencies', { detail: materialState.thirdPartyDeps }) | |||
| useCanvas().canvasApi.value?.canvasDispatch('updateDependencies', { detail: materialState.componentsDepsMap }) | |||
There was a problem hiding this comment.
Add error handling for canvas dispatch.
The canvas dispatch operation should handle potential errors when the canvas API is not available.
- useCanvas().canvasApi.value?.canvasDispatch('updateDependencies', { detail: materialState.componentsDepsMap })
+ try {
+ const canvasApi = useCanvas().canvasApi.value
+ if (!canvasApi?.canvasDispatch) {
+ throw new Error('Canvas API not available')
+ }
+ canvasApi.canvasDispatch('updateDependencies', { detail: materialState.componentsDepsMap })
+ } catch (error) {
+ console.error('Failed to update canvas dependencies:', 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.
| useCanvas().canvasApi.value?.canvasDispatch('updateDependencies', { detail: materialState.componentsDepsMap }) | |
| try { | |
| const canvasApi = useCanvas().canvasApi.value | |
| if (!canvasApi?.canvasDispatch) { | |
| throw new Error('Canvas API not available') | |
| } | |
| canvasApi.canvasDispatch('updateDependencies', { detail: materialState.componentsDepsMap }) | |
| } catch (error) { | |
| console.error('Failed to update canvas dependencies:', error) | |
| } |
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (1)
packages/plugins/materials/src/composable/useMaterial.js (1)
493-493:⚠️ Potential issueAdd error handling for canvas dispatch.
The canvas dispatch operation should handle potential errors when the canvas API is not available.
🧹 Nitpick comments (2)
packages/plugins/materials/src/composable/useMaterial.js (2)
42-43: Consider using TypeScript interfaces for state objects.The reactive state objects would benefit from TypeScript interfaces to clearly define the shape of
componentsDepsMapandpackages. This would improve maintainability and provide better IDE support.interface ComponentsDepsMap { scripts: Array<{ package: string; script: string; components?: Record<string, string>; }>; styles: Set<string>; } interface MaterialState { components: any[]; blocks: any[]; componentsDepsMap: ComponentsDepsMap; packages: any[]; // Define specific package structure }
540-540: Document the new exported functions.The newly exported
setCanvasDepsfunction lacks JSDoc documentation. Add documentation to maintain consistency with other exported functions.+ /** + * Sets canvas dependencies including material and utility dependencies. + * Notifies the canvas to refresh the importmap. + */ setCanvasDeps,
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/plugins/materials/src/composable/useMaterial.js(8 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
🔇 Additional comments (3)
packages/plugins/materials/src/composable/useMaterial.js (3)
285-299:⚠️ Potential issueAdd error handling for missing or malformed utils.
The
getUtilsDependenciesfunction should handle potential errors when accessingitem.contentproperties.const getUtilsDependencies = () => { const { utils } = useResource().appSchemaState return utils - .filter((item) => item.type === 'npm' && item.content.cdnLink) + .filter((item) => item.type === 'npm' && item.content?.cdnLink) .map((item) => { + if (!item.content?.package) { + console.warn(`Invalid util item: missing package name`, item) + return null + } const { package: pkg, version, cdnLink } = item.content return { package: pkg, version, script: cdnLink } - }) + }) + .filter(Boolean)Likely invalid or redundant comment.
468-479:⚠️ Potential issueEnsure atomic updates for reactive state.
Direct mutations to
materialState.componentsDepsMapmay not trigger reactivity properly. Consider using atomic updates.-materialState.componentsDepsMap.scripts.push({ package: pkg, script, css, components }) +const updatedScripts = [...materialState.componentsDepsMap.scripts, { package: pkg, script, css, components }] +materialState.componentsDepsMap.scripts = updatedScripts -styles?.forEach((item) => materialState.componentsDepsMap.styles.add(item)) +const updatedStyles = new Set([...materialState.componentsDepsMap.styles, ...(styles || [])]) +materialState.componentsDepsMap.styles = updatedStylesLikely invalid or redundant comment.
323-366:⚠️ Potential issueAdd validation for material bundle structure.
The
parseMaterialsDependenciesfunction should validate the material bundle structure before processing.const parseMaterialsDependencies = (materialBundle) => { + if (!materialBundle || typeof materialBundle !== 'object') { + console.error('Invalid material bundle:', materialBundle) + return + } + const { packages, components } = materialBundle const { scripts: scriptsDeps, styles: stylesDeps } = useResource().appSchemaState.canvasDeps packages?.forEach((pkg) => { + if (!pkg?.package) { + console.warn('Invalid package in material bundle:', pkg) + return + } if (scriptsDeps.find((item) => item.package === pkg.package)) { return }Likely invalid or redundant comment.
| const setCanvasDeps = () => { | ||
| const { scripts, styles } = useResource().appSchemaState.canvasDeps | ||
|
|
||
| // 将utils依赖添加到canvasDeps中 | ||
| getUtilsDependencies().forEach((util) => { | ||
| if (scripts.find((item) => util.package === item.package)) { | ||
| return | ||
| } | ||
|
|
||
| scripts.push(util) | ||
| }) | ||
|
|
||
| useMessage().publish({ | ||
| topic: 'init_canvas_deps', | ||
| data: { scripts: scripts, styles: [...styles] } | ||
| }) | ||
| } |
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add retry mechanism for canvas dependency initialization.
The setCanvasDeps function should handle potential initialization failures and implement a retry mechanism.
const setCanvasDeps = () => {
const { scripts, styles } = useResource().appSchemaState.canvasDeps
// 将utils依赖添加到canvasDeps中
getUtilsDependencies().forEach((util) => {
if (scripts.find((item) => util.package === item.package)) {
return
}
scripts.push(util)
})
+ const initDeps = (retries = 3) => {
useMessage().publish({
topic: 'init_canvas_deps',
data: { scripts: scripts, styles: [...styles] }
})
+ .catch((error) => {
+ if (retries > 0) {
+ console.warn(`Retrying canvas deps initialization. Attempts left: ${retries-1}`)
+ setTimeout(() => initDeps(retries - 1), 1000)
+ } else {
+ console.error('Failed to initialize canvas dependencies:', error)
+ }
+ })
+ }
+
+ initDeps()
}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?
Issue Number: N/A
What is the new behavior?
物料资产包协议修改。涉及画布渲染、预览、出码
json { "data": { "framework": "Vue", "materials": { "components": [ { "id": '', "component": 'TinyButton' "npm": { "pacakge": "@opentiny/vue", // "version": "3.11.0", // 移除 // "script" : "", // 移除 // "css": "", // 移除 "exportName": "Button" } } ], "blocks": [], "snippets": [], "packages": [ // 新增字段 { "name": "TinyVue组件库", // 名称 "package": "@opentiny/vue", // npm包名 "version": "3.11.0", // npm包版本 "script": "https://xxxxx", // esm格式的js文件cdn地址 "css": ["https://xxxxx"], // 样式表文件cdn地址 ] }, { "name": "xxx工具库", // 名称 "package": "lodash", // npm包名 "version": "3.11.0", // npm包版本 "script": "https://xxxxx", // esm格式的js文件cdn地址 "css": ["https://xxxxx"], // 样式表文件cdn地址 "needDeclare": true // 出码时是否需要声明到package.json的依赖，默认为true。当物料npm包中已声明的依赖且不需要在宿主工程声明时，可设置为false }, ] } } }Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit
Release Notes
New Features
Improvements
Bug Fixes
Refactoring