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

Better support for internal use of forwardRef #16619

Closed
MilosRasic opened this issue Aug 30, 2019 · 3 comments
Closed

Better support for internal use of forwardRef #16619

MilosRasic opened this issue Aug 30, 2019 · 3 comments

Comments

@MilosRasic
Copy link

MilosRasic commented Aug 30, 2019

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

Not sure. The core of the issue is React.forwardRef passing null as ref argument to the wrapped component if the parent component doesn't pass a ref. undefined would be better in this case, but there might be a reason to pass null that I'm not aware of, so I'm not comfortable calling this a bug.

What is the current behavior?

Given:

export default React.forwardRef((props, ref) => <MyComponent innerRef={ref} {...props} />);

MyComponent will receive null as ref argument when a parent component doesn't pass a ref. Leaving aside that undefined better represent the fact that something was not passed than null, this also makes it impossible to use the forwarded ref internally in MyComponent because it will be null and we cannot reassign the prop.

I've been able to work around this using the following solution (excuse my classyness please, shouldn't matter much for the example):

class MyComponent extends React.Component {
	el = React.createRef();

	componentDidMount() {
		if (!this.el.current) {
			this.el = this.props.innerRef;
		}

		if (!this.el.current) return;

		// bind event handlers to this.el
	}

	render() {
		const childRef = this.props.innerRef || this.el;

		return <button ref={childRef}>Look at my button</button>;
	}
}

While the above works, it's not the nicest piece of code and there's code in two places in order to support adapting to the case when parent doesn't pass a ref.

What is the expected behavior?

Not only is undefined a less surprising value for ref argument when no ref is passed by the parent, but I think it would also allow us to use defaultProps to make a new ref the default when nothing is passed in. My initial attempt was something like:

class MyComponent extends React.Component {
	static defaultProps = {
		innerRef: React.createRef(),
	};

	componentDidMount() {
		if (!this.innerRef.current) return;

		// bind event handlers to this.innerRef.current
	}

	render() {
		return <button ref={innerRef}>Look at my button</button>;
	}
}

but it didn't work because innerRef is null.

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

React 16.9.0
Chrome 76.0.3809.132 on Linux
Didn't try with older versions of React or in other browsers.

@nortonwong
Copy link

In this case, you would probably not want to use a defaultProp at all. If you render multiple <MyComponent /> they will all read and write the same innerRef.

@MilosRasic
Copy link
Author

ouch, right, didn't think of that

If no one has a better idea might as well close the issue.

@nortonwong
Copy link

Each <MyComponent /> will need to create its own ref as well as handle props.innerRef. You might want to take a look at the discussion in #13029, where people have examples of multiple refs.

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

No branches or pull requests

3 participants