Skip to content

Update reference-react.md #237

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open

Update reference-react.md #237

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 1 addition & 3 deletions content/docs/reference-react.md
View file
Original file line number Diff line number Diff line change
@@ -103,16 +103,14 @@ React.cloneElement(
)
```

Clone and return a new React element using `element` as the starting point. The resulting element will have the original element's props with the new props merged in shallowly. New children will replace existing children. `key` and `ref` from the original element will be preserved.
Clone and return a new React element using `element` as the starting point. The resulting element will have the original element's props with the new props merged in shallowly. New children, if passed as `...children` or otherwise `props.children`, will replace existing children. `key` and `ref` from the original element will be preserved unless `props` contains `key` or `ref` (which will overwrite the `key` or `ref` in the cloned element, respectively).
Copy link
Contributor

Choose a reason for hiding this comment

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

The new wording is informative but it reads a little confusingly to me. How do you feel about just slightly tweaking what was there before?

Clone and return a new React element using element as the starting point. The resulting element will have the original element's props with the new props merged in shallowly. New children will replace existing children. key and ref from the original element will be preserved unless they are explicitly overridden.

Copy link
Contributor Author

@jedwards1211 jedwards1211 Nov 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

Hmmm, how can we be precise without being confusing? "explicitly overridden" doesn't make it clear that they can be overridden by mixing key or ref in with the other props passed to cloneElement. To me that's not obvious unless explicitly stated because key and ref are like pseudoprops. I guess the code example does make that clear, if one thinks through what it means.

And as far as children goes, "new children will replace existing children" doesn't clarify that existing children can be preserved -- it seems to imply that if ...children and props.children are not provided, then the cloned element will have no children.

The fact that children can be overridden via props.children instead of ...children is also not obvious at all. I'm trying to fully and correctly document all behavior of cloneElement, which really isn't that trivial...


`React.cloneElement()` is almost equivalent to:

```js
<element.type {...element.props} {...props}>{children}</element.type>
```

However, it also preserves `ref`s. This means that if you get a child with a `ref` on it, you won't accidentally steal it from your ancestor. You will get the same `ref` attached to your new element.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is a useful sentence. Maybe we could just tweak the wording slightly?

Except that it also preserves refs by default. This means that if you get a child with a ref on it, you won't accidentally steal it from your ancestor. You will get the same ref attached to your new element.

Copy link
Contributor Author

@jedwards1211 jedwards1211 Nov 2, 2017 •
edited
Loading

Choose a reason for hiding this comment

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

I think this sentence is very unclear though. Is "child with a ref on it" referring to a child of the cloned element or the cloned element itself? In any case, it's basically just restating what the sentence before the code example says.

Maybe the clearest thing to say would be "cloneElement preserves the refs of all descendants of the element being cloned".

Copy link
Contributor

Choose a reason for hiding this comment

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

You could replace, "child with a ref on it" with something more like, "if you clone an element that has a ref".

Copy link
Contributor Author

Choose a reason for hiding this comment

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

but up above the code example it already says "key and ref from the original element will be preserved."


This API was introduced as a replacement of the deprecated `React.addons.cloneWithProps()`.

* * *