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

[RFC] Add a new React.createRef() API for ref objects #11555

Closed
wants to merge 7 commits into from

Conversation

trueadm
Copy link
Contributor

@trueadm trueadm commented Nov 14, 2017

This PR adds a new API into the react package and the relevant logic to support this in react-dom along with tests. The ideas behind this stem from this issue from @sebmarkbage: #10581.

Currently, we offer two ways of using refs in React – string refs and callback refs. In the future, we'd like to deprecate and remove string refs from React.

I'm proposing this PR as replacement for string refs that provide similar functionality to how they currently work but without many of the issues that string refs currently have (a list of them can be found at #1373).

The React.createRef() API will create an immutable object ref (where it's value is a mutable object referencing the actual ref). Accessing the ref value can be done via ref.value. An example of how this works is below:

class MyComponent extends React.Component {
  constructor() {
    super();
    this.divRef = React.createRef();
  }

  render() {
    return <div><input type="text" ref={this.divRef} /></div>;
  }

  componentDidMount() {
    this.divRef.value.focus();
  }
}

This alternative API shouldn't provide any big real wins over callback refs – other than being a nice convenience feature. There might be some small wins in performance – as a common pattern is to assign a ref value in a closure created in the render phase of a component – this avoids that (even more so when a ref property is assigned to a non-existent component instance property).

@@ -45,3 +45,8 @@ export type ReactPortal = {
// TODO: figure out the API for cross-renderer implementation.
implementation: any,
};

export type RefObject = {
contents: any,
Copy link
Contributor Author

@trueadm trueadm Nov 14, 2017

Choose a reason for hiding this comment

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

How can we better type this so it's not any?

Copy link
Contributor

Choose a reason for hiding this comment

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

It can be generic

export type RefObject<T> = {
  contents: T | null
};

@@ -45,3 +45,7 @@ export type ReactPortal = {
// TODO: figure out the API for cross-renderer implementation.
implementation: any,
};

export type RefObject = {|
value: any,
Copy link
Contributor

Choose a reason for hiding this comment

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

With input will be ref.value.value :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yep, I believe contents is also a property on some DOM nodes too. value makes more sense in the case of this immutable object having a mutable object representing the ref itself.

Copy link
Contributor

Choose a reason for hiding this comment

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

Might be a bit much, but what if it was element for a DOM element and component for a React component, instead of value?

@gaearon
Copy link
Collaborator

gaearon commented Nov 14, 2017

This alternative API shouldn't provide any big real wins over callback refs – other than being a nice convenience feature

How should people choose between them? Does it mean we need to deprecate callback refs in long term?

@trueadm
Copy link
Contributor Author

trueadm commented Nov 14, 2017

@gaearon I look at them as solving different problems and crossing over in what they offer too – much like how components in React do currently.

Someone would likely choose an object ref when they only care about binding the object reference to the component instance/state they're in. Then the ref can be used for caching, or being passed to another component/handler in a lifecycle event.

Callback refs are great when someone likes to have find grain control over the node at the point where it gets provides and removed (like a subscription) to react to it.

*
* This source code is licensed under the MIT license found in the
* LICENSE file in the root directory of this source tree.
*/
Copy link
Contributor

Choose a reason for hiding this comment

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

missing @flow annotation?

@jamiebuilds
Copy link

This alternative API shouldn't provide any big real wins over callback refs

One benefit would be more correct Flow types. People tend to type refs incorrectly because Flow doesn't enforce uninitialized class properties correctly:

class Foo { neverSet: boolean; }
let foo = new Foo();
(foo.neverSet: boolean); // works

@gaearon
Copy link
Collaborator

gaearon commented Jan 5, 2018

Closing PR as stale, but I filed #11973 to track it.

We should probably turn this into an RFC first since we now have a process for them.
This way we can get some feedback too.

@gaearon gaearon closed this Jan 5, 2018
@TrySound
Copy link
Contributor

@trueadm @gaearon Can this idea fit to one of near minors? Or you want RFC first?

@gaearon
Copy link
Collaborator

gaearon commented Jan 25, 2018

If you can start an RFC that would be great

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.

7 participants