-
Notifications
You must be signed in to change notification settings - Fork 15
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
Remove children
from props of ErrorMessage when using component
#87
Remove children
from props of ErrorMessage when using component
#87
Conversation
exports[`ErrorMessage should render correctly with flat errors and as with component and render 1`] = ` | ||
<DocumentFragment> | ||
flat | ||
</DocumentFragment> |
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.
This looks incorrect. According to the README, the as
should take precedence, but that seems to only apply to elements provided in as
, not components.
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.
@lanvs oh... you are right!
We need to fix this.
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.
Do you need me to try to work on it in this PR, or can that maybe be a separate effort?
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.
Let's merge this once 👍🏻
If you are willing to help, we would like you to create another PR 🙏🏻
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'll see what I can do, but TBH I'm stretched a bit thin at the moment.
thanks for the PR 👍 |
Hi there, any word on this? @kotarella1110 are you able to take a look? 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.
Thanks for your PR! 🎉
exports[`ErrorMessage should render correctly with flat errors and as with component and render 1`] = ` | ||
<DocumentFragment> | ||
flat | ||
</DocumentFragment> |
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.
@lanvs oh... you are right!
We need to fix this.
Thanks @IanVS and @kotarella1110 |
Thanks both of you. I've created #88 to track the |
I tried using
ErrorMessage
in my own project with anas
component that takeschildren
as a prop. Because of the way the typescript is set up, it forced me to also add children to theErrorMessage
, even though it ends up getting replaced with the actual error message.The change here removes children from the props when using a component in
as
.I also added a pair of tests for components, since those were missing. I think one snapshot is revealing that the behavior is different from what's specified in the README, but I didn't tackle that change here, it can be done in another PR if it is indeed a bug.