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
+23 −0
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
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
7 changes: 7 additions & 0 deletions src/mergeProps.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
export default function mergeProps<A, B, C>(
defaultProps: A,
config: B,
props: C,
): A & B & C {
return { ...defaultProps, ...config, ...props };
}
Comment on lines +1 to +7
Copy link

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

质疑这个抽象层的必要性

这个函数本质上就是对 {...defaultProps, ...config, ...props} 的简单封装。正如之前 zombieJ 提到的，直接使用原生展开语法性能更好，而且代码同样简洁易读。

考虑以下几点：

  1. 性能开销：增加了额外的函数调用层
  2. 过度抽象：对于如此简单的操作，封装可能带来的价值有限
  3. 直观性：{...a, ...b, ...c} 的意图在代码中更加直观明确

建议重新评估这个工具函数是否真的能带来足够的价值来justify其存在。

🤖 Prompt for AI Agents 
In src/mergeProps.ts lines 1 to 7, the mergeProps function is a simple wrapper
around the object spread syntax, which adds unnecessary abstraction and function
call overhead. To fix this, remove the mergeProps function entirely and replace
its usage with direct object spread expressions like {...defaultProps,
...config, ...props} in the codebase to improve performance and maintain code
clarity.

15 changes: 15 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,15 @@
import mergeProps from '../src/mergeProps';

test('merge className', () => {
expect(
mergeProps(
{ type: 'default', shape: 'round' },
{ className: 'foo', type: 'secondary' },
{ className: 'bar' },
),
).toEqual({
className: 'bar',
type: 'secondary',
shape: 'round',
});
});
Loading