Skip to content
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.

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

Added note about renderComponent() requirement #1799

Merged
merged 1 commit into from
Jul 11, 2014

Conversation

julen
Copy link
Contributor

@julen julen commented Jul 9, 2014

No description provided.

@sophiebits
Copy link
Collaborator

This isn't true, but it replaces the contents of the node, which may be valuable to state.

@julen
Copy link
Contributor Author

julen commented Jul 9, 2014

Someone mentioned this to me on IRC, thought it would be valuable for the docs. Feel free to close this if invalid, otherwise I'm happy to reword it.

@zpao
Copy link
Member

zpao commented Jul 10, 2014

What @spicyj said. Let's reword it to be truthful. Also, let's use our other formatting we have for other notes (see it in action - http://facebook.github.io/react/docs/top-level-api.html#react.unmountcomponentatnode)

@julen
Copy link
Contributor Author

julen commented Jul 10, 2014

Reworded and adapted the formatting.

> Note:
>
> `React.renderComponent()` currently replaces the contents of the node,
> which might be valuable to state. This restriction will go away in the
Copy link
Member

Choose a reason for hiding this comment

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

Looks better, thanks! Just a couple small things.

Let's drop the "which might be valuable to state".

And then "will go away" -> "may go away" (I'm hopeful but don't want to make promises we can't keep)

@julen
Copy link
Contributor Author

julen commented Jul 11, 2014

Updated.

@syranide
Copy link
Contributor

@zpao My comment on IRC was with regards to react-future toElement which I don't see why we wouldn't support in the future, failing that for some obscure reason, React.renderComponentAtIndex should be near trivial to implement. So React.renderComponent will always clear the node it mounts into, that's simply the nature of that API call, but I'm sure there will be other API calls that don't.

@sophiebits
Copy link
Collaborator

Yup, we plan to have a toElement: #1711.

sophiebits added a commit that referenced this pull request Jul 11, 2014
Added note about `renderComponent()` requirement
@sophiebits sophiebits merged commit 694a952 into facebook:master Jul 11, 2014
@sophiebits
Copy link
Collaborator

Thanks!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants