Emotion: Unknown prop on DOM element

[eXtreaL]

• emotion version: 6.0.3
• react version: 15.6.1

Relevant code:

const Avatar = styled('img')`
  width: 96px;
  height: 96px;
  border-radius: ${props =>
    props.rounded ? '50%' : 0};
`

Example above is from https://emotion.sh/.

What you did:
Tried to render the example from above using

<Avatar rounded />

What happened:
I got the following error in my console: Unknown prop rounded on tag. Remove this prop from the element.

Problem description:
I'm not sure whether this is a problem due to my configuration/implementation or if this is a problem in the library. I assumed this should work without any errors, since i copied the exact example from https://emotion.sh/. However when scanning emotion's source code i found that styled just blindly merges/spreads all the props onto the DOM element.

Suggested solution:
Whitelist the allowed props for each DOM element, which is the way styled-components is doing it at the moment.

[ai]

My temporary solution:

const Avatar = styled('img')`
  width: 96px;
  height: 96px;
`

const RoundAvatar = styled(Avatar)`
  border-radius: 50%;
`

props.rounded ? <RoundAvatar> : <Avatar>

[divyagnan]

I don't know if this is a 'good' solution but I've been using the following and getting by without any errors:

const e = React.createElement;

/* manually pull off the props that you will be using 
 * and then forward everything else to the createElement call
 */
const Button = styled(
  ({ backgroundColor, children, ...props }) => e("button", props, children)
)`
  background-color: ${props => props.backgroundColor};
`;

<Button backgroundColor="green">No Error!</Button>

edit

The following helper function can help if the above is a little tedious:

const e = React.createElement;

/**
 * filter helper function 
 * @param {*} tag - they type of component you want, div, button, etc
 * @param {string[]} whitelist - the props that you don't want to forward
 */
const filter = (tag, whitelist) => ({ children, ...props }) => {
  // this probably isn't that safe, but it works as a hack
  whitelist.map(i => delete props[i]);
  return e(tag, props, children);
};

// use by passing in filter to the styled function
const Button = styled(filter("button", ["backgroundColor", "color"]))`
  background-color: ${props => props.backgroundColor};
  color: ${props => props.color}
`;

You could also have the tag be curried if you didn't want to pass that it (or if you make a lot of the same type of component)

// modified helper
const filter = tag => whitelist => ({ children, ...props }) => {
  whitelist.map(i => delete props[i]);
  return e(tag, props, children);
};

// create the buttonFilter function
const buttonFilter = filter("button");

// use the button filter function - now you only have to pass in the array of filtered props
const Button = styled(buttonFilter(["backgroundColor", "color"]))`
  background-color: ${props => props.backgroundColor};
  color: ${props => props.color}
`;

[tkh44]

great stuff @divyagnan

FWIW you can use recompose's mapProps

https://github.com/acdlite/recompose/blob/master/docs/API.md#mapprops

const omitProps = keys => mapProps(props => omit(keys, props))

styled(omitProps(['color'])(Button))`
  color: blue;
`

[eXtreaL]

That's actually a pretty good solution, @divyagnan. However i would like to see this solved from the library instead. Currently i'm just directly using the css() function in my components instead of using styled(), passing the relevant props to css() and composing a style object (resulting in classnames) myself.

[tkh44]

@eXtreaL That is actually how I prefer to use emotion. You can also use the css prop on any element that supports className.

<div css=`color:blue;`/>

is compiled to

<div className={css`color:blue;`} />

which is compiled down to (in extractStatic mode)

<div className={'css-13vd44'} />

[ai]

@tkh44 any plans to fix it in core?

[thangngoc89]

@ai from slack, styled will have anp new arg:

Styled("div", { omitProps: ["foo"] })

[ai]

@thangngoc89 you mean?

const Link = styled.css`
  color: ${props => props.foo ? 'black' : 'red'};
`

<Styled foo={true} omitProps={['foo']}>

[thangngoc89]

@ai Sorry I was on my phone. It's like this

const Link = styled("div", { omitProps: ["foo"] })`...`

<Link foo="bar" />

[tkh44]

@ai I've been writing more and more emotion code and thinking on how I want to handle this in the core. Right now I'm using this setup.

// base.js
import React from 'react'
import styled from 'emotion/react'
import { omit } from 'emotion/lib/utils'
import { space, width, fontSize, color } from 'styled-system'

import theme from './theme'

const defaultExcludedProps = [
  'm',
  'mt',
  'mr',
  'mb',
  'ml',
  'mx',
  'my',
  'p',
  'pt',
  'pr',
  'pb',
  'pl',
  'px',
  'py',
  'fontSize'
]

export default (tag, omitProps = []) => styled(props =>
  React.createElement(tag, omit(props, defaultExcludedProps.concat(omitProps)))
)`
  ${space};
  ${width};
  ${fontSize};
  ${color};
  color: ${theme.colors.gray[8]};
  font-family: 'Oxygen', sans-serif;
  box-sizing: border-box;
`

and usage

import React from 'react'
import styled, { css } from 'emotion/react'
import base from '../base'
import theme from '../theme'

export default styled(base('input', ['omit', 'these', 'props']))`
  position: relative;
  display: block;
  width: 100%;
  border-radius: 0;
  background: ${theme.colors.gray[0]};
  font-weight: bold;
  -webkit-appearance: none;
  border: none;
  border-bottom: 1px solid ${theme.colors.green[5]};
  outline: none;
`

I've come to really like this pattern but I'm not sure how to make this a solution that works for everyone.

[souporserious]

I've been using glamor jsxstyle recently with some custom options that allow me to set global custom style props to strip out things like marginVertical, elevation, etc.. similar to what you have above. Maybe you could have something similar. Some way to set global props or something. Could it be a babel option? Or maybe you create your own emotion? Super naive example:

import createEmotionFactory from 'emotion/create'
export default createEmotionFactory(['mv', 'mh', ...])

..later

import styled from './my-styled'
export default styled(base('input'))`
  position: relative;
  display: block;
  width: 100%;
`

[ai]

@tkh44 Wow, it was fast! I like Emotion community :).

Also, we may need docs for it since it is not clear.

[thangngoc89]

@ai you don't need to do anything on your side, just upgrade to the latest emotion beta.

[ai]

@thangngoc89 uh, so you clean all non-DOM props automatically? Awesome!

[thangngoc89]

Oh yeah. Thanks @mitchellhamilton for that

[tomduncalf]

Sounds like this should be fixed looking at the above, but with emotion 9.0.1 I still get an error e.g. with a div:

const Container = styled<{ backgroundColor: string }, 'div'>('div')(props => ({
  backgroundColor: props.backgroundColor,
}));

<Container backgroundColor={colors[0]}>

Warning: React does not recognize the `backgroundColor` prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase `backgroundcolor` instead. If you accidentally passed it from a parent component, remove it from the DOM element.

Any advice? I am using this with react-static if that could make any difference...

[tkh44]

@tomduncalf I can't reproduce what you are describing here https://codesandbox.io/s/r552m5nvjo

[tomduncalf]

Interesting @tkh44, my next step was going to be a simple repro case. I guess something somewhere in my setup is breaking it. I'll look into it more when I have time :) Thanks!

[tomduncalf]

Sorry, I'd got a bit confused, don't write code late at night ;)

The issue is that the react-router Link component is passing through all the props it receives. I'm now manually instantiating it to filter the props, like:

const HomeItem = styled<IProps, 'Link'>((props: IProps) => (
  <Link to={props.to} className={props.className}>
    {props.children}
  </Link>
))((props: IProps) => ({
 ...

[bernharduw]

I also came across that issue, in my case with react-bootstrap's ListGroupItem, which also passes through all the props.

What do you think about an official omitProps option to styled?

const ListItem = styled(ListGroupItem, { omitProps: ['color'] })`
  color: ${props => props.color || '#000'};
`;

My current workaround doesn't feel right nor readable:

const ListItem = styled(({ color, ...props }) => <ListGroupItem {...props} />)`
  color: ${props => props.color || '#000'};
`;

I could take a stab on a PR if you like that omitProps proposal.

[davej]

@bernharduw @mitchellhamilton: ^

Sorry for the rude ping. The post above has 6 thumbs up. Is it something that you would consider?

Finding this issue a lot with components in the Ant Design component library.

[Andarist]

This feature already is in emotion - check out shouldForwardProp

[davej]

Perfect thank you!
For anybody stumbling across this issue, take a look at #655.

[DylanVann]

With the new syntax I end up getting babel errors when compiling:

styled(SomeComponent, { shouldForwardProps: func })

I get: "Second argument to a styled call is not an object, it's going to be removed"

It doesn't emit an error when using a built in element, e.g.

styled('a', { shouldForwardProps: func })

[Andarist]

That's weird. @DylanVann could u prepare a sample repo with the issue reproduced?

[bitttttten]

I am getting this error on a component:

Warning: React does not recognize the `isOpen` prop on a DOM element. If you intentionally want it to appear in the DOM as a custom attribute, spell it as lowercase `isopen` instead. If you accidentally passed it from a parent component, remove it from the DOM element.
    in div
    in Box
    in StyledFlex (created by InteractiveMediaObject)

My component looks like:

import styled from 'react-emotion'
import { Flex, Box } from 'grid-styled/emotion'

const StyledFlex = styled(Flex)`
    background: ${({ variant, theme }) => theme.colours[variant || 'lightest']};
    position: relative;
    padding-left: ${({ isOpen }) => (isOpen ? '10rem' : 0)};
    transition: padding-left 0.2s ease;
`

and it's used like:

        return (
            <StyledFlex
                variant={variant}
                isOpen={this.isOpen}
                justifyContent="center"
                alignItems="center"
            >
                <ActionBox onClick={onAction}>{actionText}</ActionBox>
                <ImageBox
                    image={image}
                    onClick={this.toggle}
                />
            </StyledFlex>
        )

I omitted a lot of code for beverity. Why is isOpen being passed down but none of the others?

[tgallacher]

I'm not familiar with grid-styled/emotion, but looking at the code I would guess that variant, justifyContent and alignItems are recognised props for their Flex API, and therefore they will take care of ensuring these don't propagate to the underlying DOM.

However, isOpen is something I assume is what you have added and is thus not recognised by the grid-styled/emotion Flex component. Therefore, styled (from react-emotion) will pass everything through it doesn't recognise -- as it doesn't know if you are wrapping a base DOM node, or a custom component, and therefore can't know when or what to prevent propagating through.

However, you can manually tell styled which props is should propagate and which it shouldn't, although the functionality doesn't seem to be well documented yet: See https://github.com/emotion-js/emotion/issues/655 for more info.

To fix the warning here, I think the following should do the job:

import styled from 'react-emotion'
import isPropValid from '@emotion/is-prop-valid'
import { Flex, Box } from 'grid-styled/emotion'

const StyledFlex = styled(Flex, { shouldForwardProp: isPropValid })`
    background: ${({ variant, theme }) => theme.colours[variant || 'lightest']};
    position: relative;
    padding-left: ${({ isOpen }) => (isOpen ? '10rem' : 0)};
    transition: padding-left 0.2s ease;
`

[Andarist]

And this (shouldForwardProp) could be added directly in grid-styled/emotion to get rid of such problems altogether.

It seems though that this could be fixed on emotion side, Im not sure though if we can do a 9.x release at the moment, cc @mitchellhamilton (the fix would be to reuse tag.__emotion_forwardProp for isReal components)

[bitttttten]

Is there an update on this yet? Couldn't shouldForwardProp be inherited from what is being passed into styled(x)?

It avoids making the below as verbose. It would be nice of StyledLink inherited shouldForwardProp from Link.

const StyledLink = styled(Link, {
    shouldForwardProp: isPropValid,
})<{ aspectRatio: number }>(({ theme, aspectRatio }) => ({
    display: `block`,
    position: `relative`,
    border: `1px solid ${theme.colours.primary}`,
    paddingBottom: `${100 / aspectRatio}%`,
}))