Closed
Conversation
# Conflicts: # designer-demo/public/mock/bundle.json resolved by [CherryPick] version
Contributor
WalkthroughThe changes update two scripts. In scripts/buildMaterials.mjs, a new constant loads material package data from a Changes
Sequence Diagram(s)sequenceDiagram
participant G as generateComponents
participant M as materials/packages.json (materialInfo)
participant N as material.npm Object
G->>M: Load and parse packages.json
G->>N: Check for package property
alt Package exists
G->>M: Retrieve matching package details
G->>N: Merge latest package properties (exclude name)
else
G-->>N: Continue without merge
end
sequenceDiagram
participant DB as initDB
participant MC as MysqlConnection
DB->>MC: Call isComponentDifferent(newComponent, existingComponent)
note right of MC: Normalize JSON values for comparison
alt Components differ
MC-->>DB: Return true (update required)
else
MC-->>DB: Return false (no update)
end
Possibly related PRs
Suggested labels
Suggested reviewers
Poem
✨ Finishing Touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. 🪧 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}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🔭 Outside diff range comments (1)
scripts/connection.mjs (1)
451-470:⚠️ Potential issuePrevent SQL injection vulnerability
The SQL query is built by directly inserting the component name into the query string without sanitization, which could lead to SQL injection attacks.
- const selectSqlContent = `SELECT * FROM ${this.config.database}.${componentsTableName} WHERE name_en = '${component.component}'` + const selectSqlContent = `SELECT * FROM ${this.config.database}.${componentsTableName} WHERE name_en = ?` - this.query(selectSqlContent) + this.query(selectSqlContent, [component.component])This requires updating the
querymethod to support parameterized queries:query(sql, params = []) { return new Promise((resolve, reject) => { this.connection.query(sql, params, (error, result) => { if (error) { reject(error) } else { resolve(result) } }) }) }
🧹 Nitpick comments (3)
scripts/buildMaterials.mjs (1)
120-129: Verify package data before mergingThe code assumes materialInfo.packages is an array. It should verify this before using array methods to avoid potential runtime errors.
if (materialPackageInfo?.package) { - const latestPackageInfo = materialInfo.packages.find((item) => item.package === materialPackageInfo.package) + const packages = Array.isArray(materialInfo.packages) ? materialInfo.packages : []; + const latestPackageInfo = packages.find((item) => item.package === materialPackageInfo.package); if (latestPackageInfo) { const { name: _name, ...rest } = latestPackageInfo material.npm = Object.assign(material.npm, rest) } }scripts/connection.mjs (2)
97-133: Improve JSON normalization to handle arraysThe
normalizeJsonValuemethod only checks for JSON objects (starting with '{' and ending with '}'). It should also handle JSON arrays.// 如果解析结果仍然是字符串，且看起来像JSON，再次尝试解析 - if (typeof parsed === 'string' && parsed.startsWith('{') && parsed.endsWith('}')) { + if (typeof parsed === 'string' && + ((parsed.startsWith('{') && parsed.endsWith('}')) || + (parsed.startsWith('[') && parsed.endsWith(']')))) { try { return JSON.parse(parsed) } catch (e) { // 如果再次解析失败，返回第一次解析的结果 return parsed } }
458-465: Pass file path to updateComponent for better error reportingWhen calling
updateComponentafter detecting differences, the file path should be passed to ensure proper error reporting.if (needUpdate) { - this.updateComponent(component) + this.updateComponent(component, 'Database update') }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (2)
scripts/buildMaterials.mjs(2 hunks)scripts/connection.mjs(3 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (1)
- GitHub Check: push-check
Comment on lines
+20
to
+21
| // 从materials目录下的packages.json读取物料包配置 | ||
| const materialInfo = fsExtra.readJSONSync(path.join(process.cwd(), materialsDir, 'packages.json')) |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Add error handling for reading packages.json
The code reads the packages.json file synchronously without any error handling. If the file doesn't exist or contains invalid JSON, the script will crash.
- const materialInfo = fsExtra.readJSONSync(path.join(process.cwd(), materialsDir, 'packages.json'))
+ let materialInfo = { packages: [] };
+ try {
+ const packagePath = path.join(process.cwd(), materialsDir, 'packages.json');
+ if (fsExtra.existsSync(packagePath)) {
+ materialInfo = fsExtra.readJSONSync(packagePath);
+ } else {
+ logger.warn('packages.json not found in materials directory');
+ }
+ } catch (error) {
+ logger.error(`Failed to read packages.json: ${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.
Suggested change
| // 从materials目录下的packages.json读取物料包配置 | |
| const materialInfo = fsExtra.readJSONSync(path.join(process.cwd(), materialsDir, 'packages.json')) | |
| // 从materials目录下的packages.json读取物料包配置 | |
| let materialInfo = { packages: [] }; | |
| try { | |
| const packagePath = path.join(process.cwd(), materialsDir, 'packages.json'); | |
| if (fsExtra.existsSync(packagePath)) { | |
| materialInfo = fsExtra.readJSONSync(packagePath); | |
| } else { | |
| logger.warn('packages.json not found in materials directory'); | |
| } | |
| } catch (error) { | |
| logger.error(`Failed to read packages.json: ${error}`); | |
| } |
Comment on lines
+141
to
+186
| isComponentDifferent(newComponent, existingComponent) { | ||
| // 需要比较的字段列表 | ||
| const fieldsToCompare = ['name', 'icon', 'npm', 'group', 'category', 'snippets', 'schema', 'configure'] | ||
|
|
||
| // 检查每个字段是否有差异 | ||
| for (const field of fieldsToCompare) { | ||
| if (!newComponent[field] && !existingComponent[this.fieldTransform(field)]) { | ||
| // 两者都为空或未定义，视为相同 | ||
| continue | ||
| } | ||
|
|
||
| const dbField = this.fieldTransform(field) | ||
| const newValue = newComponent[field] | ||
| const dbValue = existingComponent[dbField] | ||
|
|
||
| // 对于对象类型的字段，需要标准化后比较 | ||
| if (typeof newValue === 'object' && newValue !== null) { | ||
| // 标准化两个值，确保它们在相同的格式下进行比较 | ||
| const normalizedNewValue = this.normalizeJsonValue(newValue) | ||
| const normalizedDbValue = this.normalizeJsonValue(dbValue) | ||
|
|
||
| // 将两个对象序列化为JSON字符串进行比较 | ||
| const newValueStr = JSON.stringify(normalizedNewValue) | ||
| const dbValueStr = JSON.stringify(normalizedDbValue) | ||
|
|
||
| if (newValueStr !== dbValueStr) { | ||
| return true | ||
| } | ||
| } else { | ||
| // 对于基本类型，直接比较 | ||
| const dbField = this.fieldTransform(field) | ||
| const newValue = String(newComponent[field]) | ||
| const dbValue = String(existingComponent[dbField]) | ||
|
|
||
| if (newValue !== dbValue) { | ||
| console.log(`${newComponent.component}字段 ${field} 需要更新: | ||
| 新值: ${newValue} | ||
| 旧值: ${dbValue}`) | ||
| return true | ||
| } | ||
| } | ||
| } | ||
|
|
||
| // 所有字段都相同，不需要更新 | ||
| return false | ||
| } |
Contributor
There was a problem hiding this comment.
🛠️ Refactor suggestion
Fix code duplication and use logger instead of console.log
The method has a redundant redefinition of dbField in the else block. Also, it uses console.log which is inconsistent with the rest of the code that uses the logger object.
} else {
// 对于基本类型，直接比较
- const dbField = this.fieldTransform(field)
const newValue = String(newComponent[field])
const dbValue = String(existingComponent[dbField])
if (newValue !== dbValue) {
- console.log(`${newComponent.component}字段 ${field} 需要更新:
- 新值: ${newValue}
- 旧值: ${dbValue}`)
+ logger.info(`${newComponent.component}字段 ${field} 需要更新: 新值: ${newValue} 旧值: ${dbValue}`)
return true
}
}📝 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.
Suggested change
| isComponentDifferent(newComponent, existingComponent) { | |
| // 需要比较的字段列表 | |
| const fieldsToCompare = ['name', 'icon', 'npm', 'group', 'category', 'snippets', 'schema', 'configure'] | |
| // 检查每个字段是否有差异 | |
| for (const field of fieldsToCompare) { | |
| if (!newComponent[field] && !existingComponent[this.fieldTransform(field)]) { | |
| // 两者都为空或未定义，视为相同 | |
| continue | |
| } | |
| const dbField = this.fieldTransform(field) | |
| const newValue = newComponent[field] | |
| const dbValue = existingComponent[dbField] | |
| // 对于对象类型的字段，需要标准化后比较 | |
| if (typeof newValue === 'object' && newValue !== null) { | |
| // 标准化两个值，确保它们在相同的格式下进行比较 | |
| const normalizedNewValue = this.normalizeJsonValue(newValue) | |
| const normalizedDbValue = this.normalizeJsonValue(dbValue) | |
| // 将两个对象序列化为JSON字符串进行比较 | |
| const newValueStr = JSON.stringify(normalizedNewValue) | |
| const dbValueStr = JSON.stringify(normalizedDbValue) | |
| if (newValueStr !== dbValueStr) { | |
| return true | |
| } | |
| } else { | |
| // 对于基本类型，直接比较 | |
| const dbField = this.fieldTransform(field) | |
| const newValue = String(newComponent[field]) | |
| const dbValue = String(existingComponent[dbField]) | |
| if (newValue !== dbValue) { | |
| console.log(`${newComponent.component}字段 ${field} 需要更新: | |
| 新值: ${newValue} | |
| 旧值: ${dbValue}`) | |
| return true | |
| } | |
| } | |
| } | |
| // 所有字段都相同，不需要更新 | |
| return false | |
| } | |
| isComponentDifferent(newComponent, existingComponent) { | |
| // 需要比较的字段列表 | |
| const fieldsToCompare = ['name', 'icon', 'npm', 'group', 'category', 'snippets', 'schema', 'configure'] | |
| // 检查每个字段是否有差异 | |
| for (const field of fieldsToCompare) { | |
| if (!newComponent[field] && !existingComponent[this.fieldTransform(field)]) { | |
| // 两者都为空或未定义，视为相同 | |
| continue | |
| } | |
| const dbField = this.fieldTransform(field) | |
| const newValue = newComponent[field] | |
| const dbValue = existingComponent[dbField] | |
| // 对于对象类型的字段，需要标准化后比较 | |
| if (typeof newValue === 'object' && newValue !== null) { | |
| // 标准化两个值，确保它们在相同的格式下进行比较 | |
| const normalizedNewValue = this.normalizeJsonValue(newValue) | |
| const normalizedDbValue = this.normalizeJsonValue(dbValue) | |
| // 将两个对象序列化为JSON字符串进行比较 | |
| const newValueStr = JSON.stringify(normalizedNewValue) | |
| const dbValueStr = JSON.stringify(normalizedDbValue) | |
| if (newValueStr !== dbValueStr) { | |
| return true | |
| } | |
| } else { | |
| // 对于基本类型，直接比较 | |
| const newValue = String(newComponent[field]) | |
| const dbValue = String(existingComponent[dbField]) | |
| if (newValue !== dbValue) { | |
| logger.info(`${newComponent.component}字段 ${field} 需要更新: 新值: ${newValue} 旧值: ${dbValue}`) | |
| return true | |
| } | |
| } | |
| } | |
| // 所有字段都相同，不需要更新 | |
| return false | |
| } |
chilingling
reviewed
Mar 24, 2025
Member
bundle.json 中：
|
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Conflicts:
designer-demo/public/mock/bundle.json resolved by [CherryPick] version
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?
materials/packages.json 下更新依赖信息后，可通过 buildMaterial 指令同步更新到数据库
Does this PR introduce a breaking change?
Other information
Summary by CodeRabbit