Skip to content

Instantly share code, notes, and snippets.

@jimfb
Last active February 10, 2023 15:08
Show Gist options
  • Save jimfb/d99e0678e9da715ccf6454961ef04d1b to your computer and use it in GitHub Desktop.
Save jimfb/d99e0678e9da715ccf6454961ef04d1b to your computer and use it in GitHub Desktop.

The unknown-prop warning will fire if you attempt to render a DOM element with a prop that is not recognized by React as a legal DOM attribute/property. You should ensure that your DOM elements do not have spurious props floating around.

There are a couple of likely reasons this warning could be appearing:

  1. Are you using {...this.props} or cloneElement(element, this.props)? Your component is transferring its own props directly to a child element (eg. https://facebook.github.io/react/docs/transferring-props.html). When transferring props to a child component, you should ensure that you are not accidentally forwarding props that were intended to be interpreted by the parent component.

  2. You are using a non-standard DOM attribute on a native DOM node, perhaps to represent custom data. If you are trying to attach custom data to a standard DOM element, consider using a custom data attribute (https://developer.mozilla.org/en-US/docs/Web/Guide/HTML/Using_data_attributes).

  3. React does not yet recognize the attribute you specified. This will likely be fixed in a future version of React. However, React currently strips all unknown attributes, so specifying them in your React app will not cause them to be rendered.


To fix this, composite components should "consume" any prop that is intended for the composite component and not intended for the child component. Example:

Bad: Unexpected layout prop is forwarded to the div tag.

function MyDiv(props) {
  if (props.layout === 'horizontal') {
    // BAD! Because you know for sure "layout" is not a prop that <div> understands.
    return <div {...props} style={getHorizontalStyle()} />
  } else {
    // BAD! Because you know for sure "layout" is not a prop that <div> understands.
    return <div {...props} style={getVerticalStyle()} />
  }
}

Good: The spread operator can be used to pull variables off props, and put the remaining props into a variable.

function MyDiv(props) {
  const { layout, ...rest } = props
  if (layout === 'horizontal') {
    return <div {...rest} style={getHorizontalStyle()} />
  } else {
    return <div {...rest} style={getVerticalStyle()} />
  }
}

Good: You can also assign the props to a new object and delete the keys that you're using from the new object. Be sure not to delete the props from the original this.props object, since that object should be considered immutable.

function MyDiv(props) {

  const divProps = Object.assign({}, props);
  delete divProps.layout;
  
  if (props.layout === 'horizontal') {
    return <div {...divProps} style={getHorizontalStyle()} />
  } else {
    return <div {...divProps} style={getVerticalStyle()} />
  }
}
@gajus
Copy link

gajus commented Jul 1, 2016

Is there an environment variable that could be used to silence this error?

@STRML
Copy link

STRML commented Jul 1, 2016

It would be good if there were a way to turn this off for certain elements. For instance, a router might pass url params etc as props to its children. If a user decides to ignore them for some reason and simply provide a DOM element as a child, this will warn.

@markshust
Copy link

I agree with the above posters, this is a loud error message that effects some popular libraries such as material-ui. Perhaps an info message or etc instead?

@jimfb
Copy link
Author

jimfb commented Jul 1, 2016

@gajus There is no environment variable yet, but it has been discussed, and we will probably add such a thing in the future. In the meantime, one technique that people sometimes use is to override console.error such that it checks for whatever warning they want to blacklist, and forwards all other messages to the original console.error function. That might work for you.

@gaearon
Copy link

gaearon commented Jul 1, 2016

If a user decides to ignore them for some reason and simply provide a DOM element as a child, this will warn.

You can provide a utility that strips the router props.
Check out redux-form/redux-form#1249 (comment) for a similar discussion.

this is a loud error message that effects some popular libraries such as material-ui.

We plan to add warning levels but this will take time. These warnings, however, need to be addressed. They’re not there to make your life harder 😉 . Maybe you could hold off upgrading to 15.2.0 until those libraries fix the issue, or contribute to those libraries.

@STRML
Copy link

STRML commented Jul 2, 2016

You can provide a utility that strips the router props.

Given that React has been doing this in the past and wants to make it opt-in rather than (expensively) applying to all nodes, would it not make sense for React to expose the function, even if it's an addon lib?

@alitaheri
Copy link

this is a loud error message that effects some popular libraries such as material-ui.

We will soon release 0.15.2 which addresses all the warnings.

Besides, it's just noise, nothing breaks yet. I think this change is very necessary 👍 👍

@gaearon
Copy link

gaearon commented Jul 4, 2016

Given that React has been doing this in the past and wants to make it opt-in rather than (expensively) applying to all nodes, would it not make sense for React to expose the function, even if it's an addon lib?

Then nobody would fix the issue and sidestep it by forcing React to (1) keep maintaining the whitelist, (2) doing expensive checks for every single DOM component which defeats the whole purpose of the change in the first place.

We understand it's not fun to have console spammed with warnings. If it's not your code, file issues with the components you use or send PRs and roll back to using React 15.1.0 until they are ready. It will get better once popular libraries fix this. But we need an actual fix, not to plug a hole with a solution that is just as bad.

@gpbl
Copy link

gpbl commented Jul 4, 2016

Lib developers may find useful this small function to skip the static prop types (if they won't be removed from prod builds) when implementers need to pass down custom props (see test and usage):

Actually not an happy solution, see the comment below.

function getCustomProps(props, propTypes) {
  const customProps = {};
  Object.keys(props)
      .filter(propName => !propTypes.hasOwnProperty(propName))
      .forEach(propName => {
        customProps[propName] = props[propName];
      });
  return customProps;
}

@aamirafridi
Copy link

Thank you very much for the tip. I was using {...this.props} which was passing all props.
Just did something like..

import cloneDeep from 'lodash.clonedeep';
const Input = (props) => {
  const clonedProps = cloneDeep(props);
  delete clonedProps.classNames;
  delete clonedProps.error;
  return (
      <input
         id={props.id}
         {...clonedProps}
      />
   );
}

Not sure thats the best.

@pdeva
Copy link

pdeva commented Jul 4, 2016

completely breaks react-bootstrap
react-bootstrap/react-bootstrap#2020

@gaearon
Copy link

gaearon commented Jul 4, 2016

@gpbl This is more fragile (it’s very surprising if propTypes actually influence how your code behaves) and slow (you are calling Object.keys on every render). It’s better to just do it manually, or extract “input props” into a separate prop, e.g. inputProps.

@aamirafridi cloneDeep is a very bad solution here. It is very expensive. Just use shallow _.clone() instead if you’re using Lodash.

@tuckerconnelly
Copy link

tuckerconnelly commented Jul 4, 2016

Just solved this for my lib. Used ReactInjection to inject a custom DOM property:

import { DOMProperty } from 'react/lib/ReactInjection'

DOMProperty.injectDOMPropertyConfig({
  isCustomAttribute: attributeName => attributeName === 'css',
})

Found it here: facebook/react#140 (comment)

@ffxsam
Copy link

ffxsam commented Jul 4, 2016

@aamirafridi What about this instead:

const Input = (props) => {
  const {
    classNames,
    error,
    ...clonedProps,
  } = props;

  return (
      <input
         id={props.id}
         {...clonedProps}
      />
   );
}

This will result in a clonedProps object that does not contain classNames nor error.

@gpbl
Copy link

gpbl commented Jul 4, 2016

Thanks @gaearon for the feedback!

@gpbl
Copy link

gpbl commented Jul 4, 2016

@ffxsam I believe delete is still the best solution here, without creating unused variables.

@philwhln
Copy link

philwhln commented Jul 5, 2016

Seeing lots of warnings noise from Formsy-React christianalfoni/formsy-react#361 since upgrading to React 15.2.0

@zediah
Copy link

zediah commented Jul 5, 2016

While refactoring to remove the warnings introduced by this change, I've come across a situation where I don't quite know how to get around it. I'm passing props on the children of my element (in my case, an Animation element like ReactCSSTransition) to be used by the parent as it renders them. I want to be able to remove these props before rendering the child so I won't see any of the warnings about unknown props. I just can't work out a way to make this happen.

An example:

React.Children.Map(this.props.children, child => {
    // do something with child.props.foo
    // now I want to remove child.props.foo as the actual child object doesn't need it
   const clonedProps = _.clone(child.props);
   delete clonedProps.foo;
   return React.cloneElement(child, clonedProps);
});

In this case, because React.cloneElement does a shallow clone of props and 'foo' is still on the (immutable) props of the child - 'foo' will still be on the props of the cloned child. I also tried making foo = undefined and null, but the warning still picks this up (as it's only looking at keys).

I could potentially create the element via something like
<child.type {...clonedProps}>{child.children}</child.type>
but I don't believe this would would (nor is it a neat solution) as it'll strip refs and any other fields that might be introduced/I might not be aware of.

Anyone got a proper way to achieve this?

@dan-mckay
Copy link

@jimfb Is it safe to say that all unknown props that are currently firing warnings were previously being stripped away as unrecognised?

@gaearon
Copy link

gaearon commented Jul 5, 2016

@zediah

it'll strip refs and any other fields that might be introduced/I might not be aware of.

You can pass both key and ref that you read directly from the element, as part of the argument you to React.createElement. This won’t work for string refs (because they require owner which wouldn’t get copied), but it will work for callback refs (ref={inst => ...}).

However this sounds like an anti-pattern. If you’re trying to remove a prop post factum, it probably means it shouldn’t have been there in the first place.

Instead, you can change your API to be like this:

<Animation>
  <AnimationChild someArg={42}>
    <div />
  </AnimationChild>,
  <AnimationChild someArg={100}>
    <Whatever />
  </AnimationChild>
</Animation>

@gaearon
Copy link

gaearon commented Jul 5, 2016

@dmcaodha

Is it safe to say that all unknown props that are currently firing warnings were previously being stripped away as unrecognised?

Yes.

@zediah
Copy link

zediah commented Jul 6, 2016

@gaearon

Greatly appreciate the reply and the suggestion. You're probably right in that I've been using an anti-pattern, I figured my way looked 'neater' and I never thought of all the props having to be known by the child, rather that the child just cherry-picked the props given to it.

I think my example shows a separate problem where it's not possible to truly remove a prop when cloning an element. You can make it undefined, which is very similar - but this still throws the warning. Is it really still something to warn about if the value is undefined? (it could have a default value set, but then it wouldn't be undefined)

@tyv
Copy link

tyv commented Jul 7, 2016

Hope this will be helpful, spent some time migrating on 15 too
https://www.npmjs.com/package/pick-react-known-prop

@oztune
Copy link

oztune commented Jul 15, 2016

@gaearon

However this sounds like an anti-pattern. If you’re trying to remove a prop post factum, it probably means it shouldn’t have been there in the first place.

I think there are cases where it's not an anti-pattern. For example a library may choose to 'extend' the API of a native element with some custom attributes. So the user may create and element with invalid props which the library then strips out before sending the element down to react.

I understand some purists may not appreciate such a library, but I think there are cases where it's useful and React should try to cater to that.

@fetis
Copy link

fetis commented Jul 27, 2016

I just want to make a note, that syntax

const { layout, ...rest } = props

is not a part of ES6, but currently only Stage 2 proposal of ES7.

So, for real life usage there's only delete variant or you can use Lodash .omit(), .pick() functions.

@faceyspacey
Copy link

@zediah did you ever come up with a solution? Creating an element definitely isn't it, and it can be nice to not have to add additional components to the tree.

@prasanna1211
Copy link

Hy, is there a way where I can specifically cut down props which are not required for my html element? For example I have a parent Component which wraps which wraps standard HTML input component . When I pass props from wrapper I want to remove props in CustomInput which are not standard html props for input. Apart from checking every valid props is there any other way ?

@moparlakci
Copy link

extract your own props from it

const { isMenuOpen, ...props } = this.props

then pass props to your component (without this)

<Header {...props} />

@skavan
Copy link

skavan commented Mar 30, 2021

Are they any different ways to handle this problem in 2021? In my case, the props sent to a child component are dynamic and unknown at runtime. So I need a way of (a) stripping them out automagically or (b) suppressing the warning.
(a) would be better! Ideas?

@numpde
Copy link

numpde commented Feb 10, 2023

What's the "source of truth" for the known props?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment