Skip to content

Do not pass innerRef to the component - #310

Merged
chiefjester merged 4 commits into
styled-components:masterfrom
mkhazov:do_not_pass_innerRef
Dec 14, 2016
Merged

Do not pass innerRef to the component#310
chiefjester merged 4 commits into
styled-components:masterfrom
mkhazov:do_not_pass_innerRef

Conversation

@mkhazov

@mkhazov mkhazov commented Dec 12, 2016

Copy link
Copy Markdown
Contributor

Avoid
Warning: Unknown prop `innerRef` on <textarea> tag. Remove this prop from the element. For details, see https://fb.me/react-unknown-prop

@mxstbr-bot

mxstbr-bot commented Dec 12, 2016

Copy link
Copy Markdown
2 Warnings
⚠️ changes to StyledComponent.js might be semver major changes
⚠️ changes to StyledNativeComponent.js might be semver major changes

Generated by 🚫 danger

@@ -84,7 +84,7 @@ export default (ComponentStyle: Function) => {
const propsForElement = {}
/* Don't pass through non HTML tags through to HTML elements */
Object.keys(this.props)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What if we use Object.keys(rest), then on this line:

https://github.com/mkhazov/styled-components/blob/00e941cc4242f60f8233b3719fec30fe378ed806/src/models/StyledComponent.js#L81

We do this const { className, children, innerRef, ...rest } = this.props

No need for adding innerRef in filter, /cc @mxstbr

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like that!

@chiefjester chiefjester left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm 👍

@chiefjester
chiefjester merged commit fe22372 into styled-components:master Dec 14, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants