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.

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

feat: add warning "cannot be a function" in getValueProps #642

Merged

Conversation

LongHaoo
Copy link
Contributor

[中文版模板 / Chinese template]

🤔 This is a ...

  • New feature
  • Bug fix
  • Site / documentation update
  • Demo update
  • Component style update
  • TypeScript definition update
  • Bundle size optimization
  • Performance optimization
  • Enhancement feature
  • Internationalization
  • Refactoring
  • Code style optimization
  • Test Case
  • Branch merge
  • Workflow
  • Other (about what?)

🔗 Related issue link

关联 PR ant-design/ant-design#46445

关联 issue ant-design/ant-design#46417

💡 Background and solution

根据 #46445 的改动为 getValueProps 的返回值加上一层 warning,判断返回值是否为 function 是 function 则给予提示

📝 Changelog

Language Changelog
🇺🇸 English Add a prompt for the error message returned by the Field component's getValueProps
🇨🇳 Chinese 为 Field 组件的 getValueProps 返回错误提示增加提示

☑️ Self-Check before Merge

⚠️ Please check all items below before requesting a reviewing. ⚠️

  • Doc is updated/provided or not needed
  • Demo is updated/provided or not needed
  • TypeScript definition is updated/provided or not needed
  • Changelog is provided or not needed

Copy link

vercel bot commented Dec 16, 2023

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
field-form ✅ Ready (Inspect) Visit Preview 💬 Add feedback Dec 19, 2023 10:07am

@@ -578,9 +578,16 @@ class Field extends React.Component<InternalFieldProps, FieldState> implements F
// eslint-disable-next-line @typescript-eslint/no-explicit-any
const originTriggerFunc: any = childProps[trigger];

const valueProps = mergedGetValueProps(value);

// warning when prop value is function
Copy link
Member

Choose a reason for hiding this comment

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

process 包一下,免得打包后还会检查。

src/Field.tsx Outdated

// warning when prop value is function
Object.keys(valueProps).forEach(key => {
warning(typeof valueProps[key] !== 'function', `cannot return prop value is function in getValueProps (prop name: "${key}")`)
Copy link
Member

Choose a reason for hiding this comment

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

这个描述比较怪异,它不是不能返回。而是不推荐通过 getValueProps 动态添加 function。

Copy link
Contributor Author

Choose a reason for hiding this comment

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

好的👌

src/Field.tsx Outdated
// warning when prop value is function
if (process.env.NODE_ENV !== 'production') {
Object.keys(valueProps).forEach(key => {
warning(typeof valueProps[key] !== 'function', `Not recommended return prop value is function in getValueProps (prop name: "${key}")`)
Copy link
Member

Choose a reason for hiding this comment

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

It's not recommended to generate dynamic function prop by getValueProps. Please pass it to child component directly.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

专业呀

@zombieJ zombieJ merged commit 07996ea into react-component:master Dec 19, 2023
7 checks passed
// warning when prop value is function
if (process.env.NODE_ENV !== 'production') {
Object.keys(valueProps).forEach(key => {
warning(typeof valueProps[key] !== 'function', `It's not recommended to generate dynamic function prop by \`getValueProps\`. Please pass it to child component directly (prop: ${key})`)
Copy link
Contributor

Choose a reason for hiding this comment

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

一直不知道 warning 第一个参数是 true 打印,还是 false 打印

Copy link
Contributor Author

Choose a reason for hiding this comment

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

false hh 我也是看下面代码学的,下面代码也有用 warning 的

Copy link
Contributor

Choose a reason for hiding this comment

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

所以我倾向

if(xx){
   warning(false,'xx')
}

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants