-
-
Notifications
You must be signed in to change notification settings - Fork 5.2k
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
Avoid "no source" warning when theres a finalSource in useInput #10153
Avoid "no source" warning when theres a finalSource in useInput #10153
Conversation
if ( | ||
!source && | ||
!finalSource && | ||
props.label == null && | ||
process.env.NODE_ENV === 'development' | ||
) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I agree we should change this condition, thanks for your report!
Actually I wonder if we should go even further. AFAICT this check was mainly added to make sure we won't call useController with an empty name
(and also, to make TS happy).
So we should maybe replace it altogether by:
if (
!finalName &&
process.env.NODE_ENV === 'development'
) { /* ... */ }
@fzaninotto wdyt?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You're right, an input with no source but with a name should not raise any warning!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slax57 @fzaninotto thanks for your feedback! Are you guys going to work on this? I saw the changes last week in useInput but I'm still getting the warnings which is making my application very slow when editing/creating
Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@GuilhermeCarra the changes you refer to are unrelated (you can have a look at the originating PR description to understand the context).
Regarding this PR, if you apply the changes I requested above we will be happy to merge it. Thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@slax57 thanks! I've made the change 🙌
Thanks! |
Problem
Currently
useInput
is generating erroneous warnings when usingArrayInput
withTextInput
without source. The source is provided byuseWrappedSource
Solution
Change conditional check to finalSource
How To Test
Create an ArrayInput as indicated
Additional Checks
master
for a bugfix, ornext
for a featureAlso, please make sure to read the contributing guidelines.