-
Notifications
You must be signed in to change notification settings - Fork 707
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
Update useScaffoldEventHistory hook #827
Conversation
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.
Works well and the code looks good to me, I only left a small comment.
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.
Good job @rin-st!!
Tested it nicely and only found a problem when you pass enabled: false
.
- In the old version you would get 1
Error: Hook disabled
everytime it fires the hook (watch: true
) - In the new version, it keeps spamming the error in the console.
Rest worked great! 🙌
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.
Fix working great! Thanks for the quick fix! 🙌
Thanks! Merging this |
Description
readEvents
touseCallback
, so it's clear which dependencies it hasfromBlock
is required prop, removed if's likeif (fromBlock)
readEvents();
andThe one without
if (!deployedContractLoading)
was buggy - on load it threw errors to the console4. Changed default state of events to
[]
, so we don't guess is it array or undefinedAdditional Information