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
Add innerRef support to withTheme HOC #710
Conversation
src/hoc/withTheme.js
Outdated
innerRef={innerRef} | ||
ref={ref} | ||
/> | ||
) |
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.
Personally, I'd make that render method more like:
const { innerRef } = this.props
const refs = isStyledComponent(Component) ? { innerRef } : { ref: innerRef }
return (
<Component {...{theme, ...this.props, ...refs}}/>
)
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 trying to avoid the double-merge as a micro-optimisation, but it's definitely cleaner. Should I change it?
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.
We could also extract a variable, like _isStyledComponent
and write:
<Component
theme={theme}
{...this.props}
innerRef={_isStyledComponent ? innerRef : undefined}
ref={_isStyledComponent ? undefined : innerRef}/>
9d5e9d9
to
aec674e
Compare
Generated by 🚫 dangerJS |
No description provided.