-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix error on complex example #560
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
Conversation
Catch already sets error to error.message. So in render it should just show state.error.
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. Latest deployment of this branch, based on commit e170f74:
|
Codecov Report
@@ Coverage Diff @@
## master #560 +/- ##
=====================================
Coverage 100% 100%
=====================================
Files 3 3
Lines 95 95
Branches 15 15
=====================================
Hits 95 95 Continue to review full report at Codecov.
|
@@ -248,7 +248,7 @@ function Login() { | |||
</div> | |||
<button type="submit">Submit{state.loading ? '...' : null}</button> | |||
</form> | |||
{state.error ? <div role="alert">{state.error.message}</div> : null} | |||
{state.error ? <div role="alert">{state.error}</div> : null} |
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.
Hi, I have a question about the above syntax.
Is it some convenience to handle show/hidden a component like in README.md?
{ isTrue ? <Component /> : null }
than (shorter a way)
{ isTrue && <Component /> }
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 could write a blog post about this for the number of times I talk about it 😅
Basically I've been bitten by this before:
{contacts.length && <RenderContactList contacts={contacts} />}
If contacts.length
is 0
then you'll render 0
. No good. Shipped that to prod at PayPal. So I never do that and instead I always go with the more explicit version:
{contacts.length ? <RenderContactList contacts={contacts} /> : null}
But you do whatever you like :)
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 this!
@all-contributors please add @tteke for docs |
I've put up a pull request to add @tteke! 🎉 |
Thanks |
🎉 This PR is included in version 9.4.1 🎉 The release is available on:
Your semantic-release bot 📦🚀 |
What: Fixing an error on readme
Why: In the complex example catch already sets error to
error.message
. So in render, it should just showstate.error
.How: Render function updated to just show
state.error
instead ofstate.error.message
.Checklist:
docs site
Yes. Either that or catch shoud update error on the state as
error: error
instead oferror: error.message