Emotion: Don't pass innerRef to DOM component

[thangngoc89]

I have a StyledInput and use innerRef to get the input element. But React will display this annoying warning.

Warning: Unknown propinnerRefon <input> tag. Remove this prop from the element. For details, see https://fb.me/react-unknown-prop

We should remove the innerRef after pass it to ref. Or better, filter unknown props like styled-component, but this comes with a cost of bigger runtime

[souporserious]

I ran into this issue with one of my other libraries. I ended up with a forwardRef prop, which wasn't as elegant as I'd hoped for. This was due to the fact that I wanted to be able to compose multiple style components together and still access the element's node. There's probably a better way, but I thought I'd share my experience just in case the same issue comes up.

[thangngoc89]

@tkh44 What is your opinion on this?

Treat innerRef as a special case or do what styled-components does?

Ref https://github.com/styled-components/styled-components/blob/322b289982a395f41f5632f78a0b227b6c28f407/src/utils/validAttr.js

[souporserious]

I feel like this needs to be its own package 😅 I always find that I need to filter attributes and make sure I pass valid props through. I've used this specialAssign function from react-aria-menubutton in the past.

[tkh44]

I'm going to filter it. I'm just not sure how yet.

I think we are going to remove the css prop as it currently is being used and use it as a namespace. Then we would filter out css.

[tkh44]

If you want to put in a pr that uses the white list like here https://github.com/styled-components/styled-components/blob/96afe68b3b44be740d3158976be498f4f92f7605/src/utils/validAttr.js we could see what that does to the bundle size. I'm not sure we want to include that many bytes though.

[thangngoc89]

@tkh44 we can use the list directly from ReactDOM if we can require it.
https://unpkg.com/[email protected]/lib/HTMLDOMPropertyConfig.js

[tkh44]

That pulls in a chain of dependencies I think. Whichever is smaller.

[thangngoc89]

@tkh44 if you use emotion/react, you already have ReactDOM.

[tkh44]

Yeah. Is there a reason why glamorous and styled-components use their own list?

[thangngoc89]

Well, React discourage aceesing private API like that

[k15a]

React removes this list with React 16 as far as I know and will start passing any props to the elements without filtering.

[tkh44]

closed in #106

[green-arrow]

@tkh44 - not sure if this is the same issue or belongs in a separate issue, but I still have the Unknown prop ... on tag error when using styled and any non-DOM attribute.

Small example:

const SvgWrapper = styled.div`
  width: ${props => props.iconWidth || 'auto'}
`

This results in Unknown prop 'iconWidth' on <div> tag.

[thangngoc89]

@green-arrow It's the same issue. styled passes all props to the wrapped component.

[green-arrow]

Is there any plan or open issue to resolve this? Seems like the only real options is to do what styled-components does with a self-maintained whitelist.

[thangngoc89]

@green-arrow I believe that @tkh44 is working on this. Since emotion is focus on library size, adding a whitelist mean double the size of emotion.

[andrewsuzuki]

Temporary solution, a simple HOC that removes specified props:

import omit from 'lodash/omit' // or just reduce a new map using removeList

const removeProps = (component, removeList) => (props) =>
  React.createElement(component, omit(props, removeList))

const MyDiv = styled(removeProps('div', ['iconWidth']))`
  width: ${props => props.iconWidth || 'auto'};
`

// Or you can be clever and use the keys from your prop types for the removeList (assuming it doesn't contain any prop types for "native" attributes)
// Example:

const myDivPropTypes = {
  iconWidth: PropTypes.number,
}

const MyDiv = styled(removeProps('div', Object.keys(myDivPropTypes)))`
  width: ${props => props.iconWidth || 'auto'};
`

MyDiv.propTypes = myDivPropTypes