Skip to content
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

chore: clear any #684

Merged
merged 6 commits into from
Apr 30, 2024
+2 −6
Merged

chore: clear any #684

Changes from 3 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
097547c
chore: rm defaultProps
li-jia-nan Apr 30, 2024
a760502
chore: rm defaultProps
li-jia-nan Apr 30, 2024
2320a1f
Merge branch 'master' into rm-defaultProps
li-jia-nan Apr 30, 2024
1226d7f
fix: fix
li-jia-nan Apr 30, 2024
dfd8a44
fix: fix
li-jia-nan Apr 30, 2024
6459cdd
fix: fix
li-jia-nan Apr 30, 2024
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
11 changes: 3 additions & 8 deletions src/Field.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -111,11 +111,6 @@ export interface FieldState {
class Field extends React.Component<InternalFieldProps, FieldState> implements FieldEntity {
public static contextType = FieldContext;

public static defaultProps = {
trigger: 'onChange',
valuePropName: 'value',
};

public state = {
resetCount: 0,
};
Expand Down Expand Up @@ -562,11 +557,11 @@ class Field extends React.Component<InternalFieldProps, FieldState> implements F
public getControlled = (childProps: ChildProps = {}) => {
const {
name,
trigger,
trigger = 'onChange',
Copy link
Member

Choose a reason for hiding this comment

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

这不等价吧，class component 的默认值只有 defaultProps 这个统一入口，去掉之后每次从 this.props 取都需要给默认值

Copy link
Member Author

Choose a reason for hiding this comment

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

是每次都要给默认值，但是我检查了一遍，只有这一个地方用到了 this.props.trigger

Copy link
Member

Choose a reason for hiding this comment

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

不利于维护

Copy link
Member Author

Choose a reason for hiding this comment

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

我去年把 antd 所有的 defaultProps 都改成默认值了，就差这里了

Copy link
Member

Choose a reason for hiding this comment

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

antd 都是 FC，不用 defaultProps 的

Copy link
Member

Choose a reason for hiding this comment

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

https://react.dev/reference/react/Component#static-defaultprops
没说废弃吧，和 Component 一起放在 legacy api 里

Copy link
Member Author

Choose a reason for hiding this comment

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

啊，legacy api 不就可以慢慢干掉了么😕

Copy link
Member

@MadCcc MadCcc Apr 30, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

这样理解：

  1. defaultProps 在 class component 下没有说明废弃，也没有更好的替代方案
  2. 整个 Component API 是 legacy，要干掉是一起干掉
    放在一起就是说明 Component API 是这么用的

Copy link
Member

Choose a reason for hiding this comment

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

那我上面那个 hook 可以统一入口

Component 用不了 hook 哦

Copy link
Member Author

Choose a reason for hiding this comment

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

好，我改回去了

validateTrigger,
getValueFromEvent,
normalize,
valuePropName,
valuePropName = 'value',
getValueProps,
fieldContext,
} = this.props;
Expand All @@ -581,7 +576,7 @@ class Field extends React.Component<InternalFieldProps, FieldState> implements F
const mergedGetValueProps = getValueProps || ((val: StoreValue) => ({ [valuePropName]: val }));

// eslint-disable-next-line @typescript-eslint/no-explicit-any
const originTriggerFunc: any = childProps[trigger];
const originTriggerFunc = childProps[trigger];
li-jia-nan marked this conversation as resolved.
Show resolved Hide resolved

const valueProps = name !== undefined ? mergedGetValueProps(value) : {};

Expand Down
Loading