Skip to content

feat(mergeProps): utility to merge component props and global config #645

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat(mergeProps): utility to merge component props and global config #645

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions package.json
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,8 @@
]
},
"dependencies": {
"classnames": "^2.5.1",
"is-plain-object": "^5.0.0",
"react-is": "^18.2.0"
Comment on lines +41 to 43
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

},
"devDependencies": {
Expand Down
1 change: 1 addition & 0 deletions src/index.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
export { default as useEvent } from './hooks/useEvent';
export { default as useMergedState } from './hooks/useMergedState';
export { default as mergeProps } from './mergeProps';
export { supportNodeRef, supportRef, useComposeRef } from './ref';
export { default as get } from './utils/get';
export { default as set } from './utils/set';
Expand Down
38 changes: 38 additions & 0 deletions src/mergeProps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
import classNames from 'classnames';
import { isPlainObject } from 'is-plain-object';
Comment on lines +1 to +2
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

修正 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.

Suggested change
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.


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;
}
Comment on lines +4 to +10
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

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.

Suggested change
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.


function mergeClassName(classNameA?: any, classNameB?: any): string {
if (typeof classNameA === 'object' || typeof classNameB === 'object') {
return mergeClassNames(classNameA, classNameB);
}

return classNames(classNameA, classNameB);
}

export default function mergeProps<T>(...list: T[]): T {
if (list.length > 2) {
return mergeProps(list[0], mergeProps(...list.slice(1)));
}
const result: T = { ...list[0] };
list[1] &&
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 {
Comment on lines +26 to +33
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

递归合并时基对象可能为 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.

Suggested change
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.

result[key] = list[1][key] ?? result[key];
}
});
return result;
}
104 changes: 104 additions & 0 deletions tests/mergeProps.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,104 @@
import mergeProps from '../src/mergeProps';

test('merge className', () => {
expect(mergeProps({ className: 'foo' }, { className: 'bar' })).toEqual({
className: 'foo bar',
});
});

test('merge classNames', () => {
expect(
mergeProps(
{
classNames: {
body: 'bam',
footer: 'foo',
},
},
{
classNames: {
footer: 'bar',
header: 'boo',
},
},
),
).toEqual({
classNames: {
body: 'bam',
footer: 'foo bar',
header: 'boo',
},
});
});

test('merge style', () => {
expect(
mergeProps(
{
style: {
background: '#000',
color: '#666',
},
},
{
style: {
background: '#fff',
},
},
),
).toEqual({
style: {
background: '#fff',
color: '#666',
},
});
});

test('merge boolean prop', () => {
expect(
mergeProps(
{
disabled: true,
loading: false,
},
{
disabled: false,
},
),
).toEqual({
disabled: false,
loading: false,
});
});

test('merge number prop', () => {
expect(
mergeProps(
{
value: 1,
},
{
value: 2,
},
),
).toEqual({
value: 2,
});
});

test('merge non-plain object prop', () => {
const dateObj = new Date();
const urlObj = new URL('https://example.com/');
expect(
mergeProps(
{
value: dateObj,
},
{
value: urlObj,
},
),
).toEqual({
value: urlObj,
});
});