feat(mergeProps): utility to merge component props and global config #645
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Walkthrough本次更改引入了一个名为 Changes
Sequence Diagram(s)sequenceDiagram
participant Caller
participant mergeProps
participant classnames
participant isPlainObject
Caller->>mergeProps: 传入多个对象
loop 遍历对象属性
alt 属性为 className
mergeProps->>classnames: 合并 className 字符串
else 属性为 classNames
mergeProps->>mergeProps: 递归合并 classNames
else 属性为 plain object
mergeProps->>isPlainObject: 检查是否为普通对象
mergeProps->>mergeProps: 递归合并该属性
else
mergeProps-->>mergeProps: 使用后者值覆盖
end
end
mergeProps-->>Caller: 返回合并后的对象
Poem
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/index.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.js » /node_modules/.pnpm/@umijs[email protected]/node_modules/@umijs/fabric/dist/eslint.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. tests/mergeProps.test.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.js » /node_modules/.pnpm/@umijs[email protected]/node_modules/@umijs/fabric/dist/eslint.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. src/mergeProps.tsOops! Something went wrong! :( ESLint: 8.57.1 ESLint couldn't find the plugin "eslint-plugin-react". (The package "eslint-plugin-react" was not found when loaded as a Node module from the directory "".) It's likely that the plugin isn't installed correctly. Try reinstalling by running the following: The plugin "eslint-plugin-react" was referenced from the config file in ".eslintrc.js » /node_modules/.pnpm/@umijs[email protected]/node_modules/@umijs/fabric/dist/eslint.js". If you still can't figure out the problem, please stop by https://eslint.org/chat/help to chat with the team. ✨ Finishing Touches
🧪 Generate Unit Tests
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:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. 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 (
|
|
Review the following changes in direct dependencies. Learn more about Socket for GitHub.
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🧹 Nitpick comments (2)
src/mergeProps.ts (1)
12-18: 对象检测逻辑与递归调用混淆
mergeClassName内部用typeof x === 'object'判断是否进入mergeClassNames，但这同时把null、array、Date等也算进去。考虑改用isPlainObject判断可避免误判；或直接信赖父函数的classNames键已明确限定。tests/mergeProps.test.ts (1)
34-55: 建议增加“三对象合并”用例当前仅验证两对象场景。由于实现对三个及以上对象采用递归策略，最好补充类似：
expect( mergeProps( { className: 'a' }, { className: 'b' }, { className: 'c' }, ), ).toEqual({ className: 'a b c' });可防止未来修改递归逻辑导致回归。
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (4)
package.json(1 hunks)src/index.ts(1 hunks)src/mergeProps.ts(1 hunks)tests/mergeProps.test.ts(1 hunks)
🧰 Additional context used
🧬 Code Graph Analysis (1)
tests/mergeProps.test.ts (1)
src/mergeProps.ts (1)
mergeProps(20-38)
🔇 Additional comments (1)
src/index.ts (1)
3-3: 公开导出 👍将
mergeProps暴露在入口处符合本次 PR 目标，方便外部直接引用。
| "classnames": "^2.5.1", | ||
| "is-plain-object": "^5.0.0", | ||
| "react-is": "^18.2.0" |
There was a problem hiding this comment.
is-plain-object 需要默认导入，否则运行时会是 undefined
is-plain-object 采用 CommonJS 导出方式（module.exports = fn），在 ESM/TS 中通常通过默认导入获得函数实例。当前包以命名导入形式引用，将导致 isPlainObject 为 undefined，进而在 mergeProps 中触发运行时异常。
请在 src/mergeProps.ts 内改为默认导入，或在此处添加 @types/is-plain-object 并更新导入方式。参考补丁见下方代码评论。
🤖 Prompt for AI Agents
In package.json around lines 41 to 43, the dependency "is-plain-object" is
listed but the import usage in the code is incorrect due to its CommonJS export
style. To fix this, ensure that in src/mergeProps.ts the import of
"is-plain-object" is changed to a default import rather than a named import, or
alternatively add the @types/is-plain-object package and update the import
accordingly to avoid runtime undefined errors.
| import classNames from 'classnames'; | ||
| import { isPlainObject } from 'is-plain-object'; |
There was a problem hiding this comment.
修正 is-plain-object 的导入方式
-import { isPlainObject } from 'is-plain-object';
+import isPlainObject from 'is-plain-object';如前所述，is-plain-object 采用默认导出，命名导入将拿到 undefined。
📝 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.
| import classNames from 'classnames'; | |
| import { isPlainObject } from 'is-plain-object'; | |
| import classNames from 'classnames'; | |
| import isPlainObject from 'is-plain-object'; |
🤖 Prompt for AI Agents
In src/mergeProps.ts at lines 1 to 2, the import of isPlainObject from
'is-plain-object' is incorrect because the package uses a default export. Change
the import statement to use a default import instead of a named import to
correctly import the function.
| Object.keys(list[1]).forEach(key => { | ||
| if (key === 'className') { | ||
| result[key] = classNames(result[key], list[1][key]); | ||
| } else if (key === 'classNames') { | ||
| result[key] = mergeClassNames(result[key], list[1][key]); | ||
| } else if (isPlainObject(list[1][key])) { | ||
| result[key] = mergeProps(result[key], list[1][key]); | ||
| } else { |
There was a problem hiding this comment.
递归合并时基对象可能为 undefined，需降级为空对象
- } else if (isPlainObject(list[1][key])) {
- result[key] = mergeProps(result[key], list[1][key]);
+ } else if (isPlainObject(list[1][key])) {
+ result[key] = mergeProps(result[key] ?? {}, list[1][key]);若 result[key] 尚未定义，直接展开会抛错。
📝 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.
| Object.keys(list[1]).forEach(key => { | |
| if (key === 'className') { | |
| result[key] = classNames(result[key], list[1][key]); | |
| } else if (key === 'classNames') { | |
| result[key] = mergeClassNames(result[key], list[1][key]); | |
| } else if (isPlainObject(list[1][key])) { | |
| result[key] = mergeProps(result[key], list[1][key]); | |
| } else { | |
| Object.keys(list[1]).forEach(key => { | |
| if (key === 'className') { | |
| result[key] = classNames(result[key], list[1][key]); | |
| } else if (key === 'classNames') { | |
| result[key] = mergeClassNames(result[key], list[1][key]); | |
| } else if (isPlainObject(list[1][key])) { | |
| result[key] = mergeProps(result[key] ?? {}, list[1][key]); | |
| } else { | |
| // … | |
| } | |
| }); |
🤖 Prompt for AI Agents
In src/mergeProps.ts around lines 26 to 33, when recursively merging properties,
the base object result[key] might be undefined, causing errors when passed to
mergeProps. To fix this, ensure result[key] is defaulted to an empty object if
undefined before passing it to mergeProps, preventing runtime exceptions during
recursive merging.
| function mergeClassNames<T>(classNamesA: T, classNamesB: T): T { | ||
| const result = { ...classNamesA }; | ||
| Object.keys(classNamesB).forEach(key => { | ||
| result[key] = mergeClassName(classNamesA[key], classNamesB[key]); | ||
| }); | ||
| return result; | ||
| } |
There was a problem hiding this comment.
mergeClassNames 未对 undefined 做保护，传播时会崩溃
当 result[key] 初始为 undefined（最常见场景是只有第二个对象携带 classNames）时，{ ...classNamesA } 会抛出 TypeError。
-function mergeClassNames<T>(classNamesA: T, classNamesB: T): T {
- const result = { ...classNamesA };
- Object.keys(classNamesB).forEach(key => {
+function mergeClassNames<T extends Record<string, any>>(
+ classNamesA: T = {} as T,
+ classNamesB: T = {} as T,
+): T {
+ const result: Record<string, any> = { ...classNamesA };
+ Object.keys(classNamesB || {}).forEach(key => {
result[key] = mergeClassName(classNamesA[key], classNamesB[key]);
});
- return result;
+ return result as T;
}这样可兼容首参缺失场景，并补全类型。
📝 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.
| function mergeClassNames<T>(classNamesA: T, classNamesB: T): T { | |
| const result = { ...classNamesA }; | |
| Object.keys(classNamesB).forEach(key => { | |
| result[key] = mergeClassName(classNamesA[key], classNamesB[key]); | |
| }); | |
| return result; | |
| } | |
| function mergeClassNames<T extends Record<string, any>>( | |
| classNamesA: T = {} as T, | |
| classNamesB: T = {} as T, | |
| ): T { | |
| const result: Record<string, any> = { ...classNamesA }; | |
| Object.keys(classNamesB || {}).forEach(key => { | |
| result[key] = mergeClassName(classNamesA[key], classNamesB[key]); | |
| }); | |
| return result as T; | |
| } |
🤖 Prompt for AI Agents
In src/mergeProps.ts around lines 4 to 10, the mergeClassNames function does not
handle undefined inputs, causing a TypeError when classNamesA is undefined. To
fix this, add checks to ensure classNamesA and classNamesB default to empty
objects if undefined before spreading or accessing their keys. This will prevent
crashes when the first argument is missing and maintain proper typing.
用通用的语法来取代目前组件中各种手动合并 props 的代码，减少代码量，更易读。
Summary by CodeRabbit
mergeProps工具函数，用于深度合并多个对象属性，支持对className和classNames的特殊合并处理。classnames和is-plain-object运行时依赖。mergeProps函数的多种用例测试，覆盖 className、classNames、样式、布尔值、数字及特殊对象等情况。