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
Re-use fetchAccountIds on all necessary places #139
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
hasRight?: boolean; | ||
isError?: boolean; | ||
isSuccess?: boolean; | ||
$hasRight?: boolean; |
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.
is having dollar sign is necessary here?
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 was getting errors/warnings on the console telling me that hasRight, isSuccess... are being passed to the DOM and doesn't exist there.
Then I checked the docs of styled components and looks like they also do it there: https://styled-components.com/docs/basics#adapting-based-on-props
But if you know a better way to do that we can try it too :)
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.
Based on the look, this component is being used on /signin
endpoint right? I don't see the error/warning on console.log. Would you mind redirecting me where you found those errors/warnings?
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.
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.
eh right, I was able to replicate it. I normally uses this repo along with near-discovery and warnings were happening within iframe..
In terms of actual problem, we actually had two approaches,
- using full lower case, eg,
hasright
- using $ in front eg.
$hasRight
Since styled component also recommends 2, I think it's LGTM.
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.
LGTM
No description provided.