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

Make it clearer that string refs are considered legacy #6692

Merged
merged 1 commit into from
May 25, 2016

Conversation

acdlite
Copy link
Collaborator

@acdlite acdlite commented May 3, 2016

Anecdotally, it's my impression that most people aren't aware that string refs are considered legacy. The docs mention it, but it's easy to miss. As an example, I brought this up on Twitter and @ryanflorence (whom no one can accuse of being a React newbie) noted that he hadn't bothered to look into callback refs yet.

This PR places the legacy warning in a stylized "Note" box, like similar warnings throughout the docs.

screen shot 2016-05-03 at 2 42 21 pm

It might also be nice to link to a resource (perhaps a blog post?) that explains the disadvantages of string refs, but the only one I'm aware of is this old GitHub issue.

>
> Although string refs are not deprecated, they are considered legacy, and will likely be deprecated at some point in the future. Callback refs are preferred.

React also supports using a string (instead of a callback) as a ref prop on any component.
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just noticed: while I'm in here, should "ref prop" be changed to "ref attribute," since it's not really a prop?

Copy link
Contributor

Choose a reason for hiding this comment

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

We generally use the term prop to refer to anything that is passed into an element (eg. using jsx). I think it's probably fine as-is.

@gaearon
Copy link
Collaborator

gaearon commented May 25, 2016

LGTM

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

Successfully merging this pull request may close these issues.

4 participants