React: React.cloneElement cannot remove existing props

[tokland]

Do you want to request a feature or report a bug?

A feature.

What is the current behavior?

When an element is cloned with React.cloneElement, it's possible to add new props or modify existing ones, but not to remove existing props.

Relevant code.

Example of how it works right now:

const element = React.createElement("a", {href: "http://github.com"});
const newElement = React.cloneElement(element, {href: undefined});
console.log(newElement.props); // {href: undefined}

What is the desired behavior?

It would be great to add some way to remove props (passing undefined as value?):

const element = React.createElement("a", {href: "http://github.com"});
const newElement = React.cloneElement(element, {href: undefined});
console.log(newElement.props); // {}

I guess I could use directly React.createElement but, AFAIK, I'll have also to worry about special attributes like key and ref. I'd rather not mess with internals.

Which versions of React, and which browser / OS are affected by this issue? Did this work in previous versions of React?

I think it has worked this way in all React versions.

What's your use case?

I am applying a map transformation of elements recursively and I need to remove some virtual props before passing the real elements for React to render. Console shows React does not recognize the [unknownProp] prop on a DOM element for those props, I'd want to avoid that.

[jmz7v]

You can achieve this using destructuring or a function.

Example:

To only use label, key, onClick, and ref for the following object you could remove className using this:

const demo = {
  key: 'a1',
  ref: 'ref1',
  className: 'button button--primary',
  label: 'Click me',
  onClick: () => { console.log('button clicked') }
}

const { className, ...demo2 } = demo

// demo2 has the value
{
  key: 'a1',
  ref: 'ref1', // className is removed from demo2
  label: 'Click me',
  onClick: () => { console.log('button clicked') }
}

However, you'll be left with an unused var if you do this

You could also create a removeProps function that only returns the values you are interested in.

function removeProps ({key, ref, label, onClick}) {
  return ({key, ref, label, onClick})
}

const demo = {
  key: 'a1',
  ref: 'ref1',
  className: 'button button--primary',
  label: 'Click me',
  onClick: () => { console.log('button clicked') }
}

const demo2 = removeProps(demo)

// Now demo2 doesn't have the className prop and you can pass it to the cloneElement function
{
  key: 'a1',
  ref: 'ref1', // className is removed from demo2
  label: 'Click me',
  onClick: () => { console.log('button clicked') }
}

[NE-SmallTown]

I guess I could use directly React.createElement but, AFAIK, I'll have also to worry about special attributes like key and ref. I'd rather not mess with internals.

@jmz7v I think you maybe misunderstand what @tokland says, the 'props' doesn't have key and ref (https://github.com/facebook/react/blob/master/packages/react/src/ReactElement.js#L171-L258), he is talking about the createElement which allow us to define the element(so we control the props, don't need to set something to undefined to override/remove something) rather than clone it, but this needs us to provide the ref and key but cloneElement doesn't.

I agree @tokland 's suggestion, I think we just need add some judgement in https://github.com/facebook/react/blob/master/packages/react/src/ReactElement.js#L338-L342 , @gaearon , right, Dan?

[softberry]

in my point of view; props should not be removed (even on cloned element). Other than that, although it seems logic for href property, undefined may also be a valid value and has it own use cases for props. It's also difficult (or even impossible) to follow clone of cloned elements and their removed props for ReactJS users, so backward incompatible.

[sophiebits]

We try to treat undefined the same as absent basically everywhere in React. Perhaps we should change the code that warns React does not recognize the unknownProp prop on a DOM element instead so that it is fine to leave an undefined in.

[stephan-noel]

Is there any update on this issue, I'm still having this problem as well. Further, an almost year-old PR that references this issue ( https://github.com/facebook/react/pull/13387 ) doesn't seem to be taking the approach mentioned by @sophiebits and is trying to skip undefined props on cloneElement instead of not giving the DOM warning when it is undefined.

[gaearon]

@stephan-noel Can you please explain the use case in more detail? Why do you have host elements (like divs) with props that shouldn't belong on them? Why are they created in the first place with extra props? Can the problem be solved on that level instead?

[stephan-noel]

Hi @gaearon, sure thing.:

I have an Operator component with an API as follows:

    <Operator operationProp={operationModifier}>
      <SomeArbitraryComponent propForOperation={operationArgument1} />
      <SomeArbitraryComponent propForOperation={operationArgument2} />
      <AnotherArbitraryComponent propForOperation={operationArgument3} />
      <Button propForOperation={operationArgument4} />
    </Operator>

Why do you have host elements (like divs) with props that shouldn't belong on them? Why are they created in the first place with extra props?

Let's say we want Button and similar leaf components to take in other props without having to specify what they are and so we spread other props over the button inside of Button. Now Button has propForOperation in the actualDOM and I get the warning.

Attempted Solution

To remove this warning, I try to clone Button with cloneElement and set propForOperation to undefined, so this prop isn't propagated (no pun intended) to button. _propForOperation is set to undefined and does not appear in the actual DOM as expected, yet because propForOperation still exists, even if the value is undefined, I still get the warning I mentioned in #16169._

Can the problem be solved on that level instead?

propForOperation is needed for Operator to work. We don't want to extract out propForOperation in Button before spreading to <button> because Button is used by many components and shouldn't even know it exists. We want Operator to be used with other leaf components that also do prop spreading as well and don't want code extracting that specific prop to have to be added in all of them.

I suppose one solution is to not use spread at all, though 1) this is very tedious since there are many HTML attributes we would like to support in many leaf components and 2) the source code of these components that spread may not be under our control.

[joaopedrodcf]

Any new update on this issue? 👍

[stale[bot]]

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contribution.

[stale[bot]]

Closing this issue after a prolonged period of inactivity. If this issue is still present in the latest release, please create a new issue with up-to-date information. Thank you!

[gaearon]

https://github.com/facebook/react/pull/16169#issuecomment-607462102