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

Refs don't work on Stateless Components #4936

Closed
jquense opened this issue Sep 22, 2015 · 36 comments
Closed

Refs don't work on Stateless Components #4936

jquense opened this issue Sep 22, 2015 · 36 comments

Comments

@jquense
Copy link
Contributor

jquense commented Sep 22, 2015

I realize to some extent this is an intended behavior? Stateless components having no public instance, but I couldn't find much in way of conversation around why this behavior was chosen.

As of right now there is no good way for a parent component to even know if a ref is going to work on a component. What is the rational behind not allowing DOM access to a stateless component? It seems like if there is no instance to expose, the proper behavior would be to return the DOM node (or whatever the underlying thing is) directly like DOM components do now.

The current behavior is surprising and seems inverted. A component is never going to reasonably be able to know if a parent needs access to its DOM node. And a Parent has no way of knowing which components are stateless (and why should it?) to know to wrap it. Right now we need to wait until something silently doesn't work. At the very least a stateless component with a ref should throw, though that would be a last resort concession in my mind.

@sebmarkbage
Copy link
Collaborator

A component is never going to reasonably be able to know if a parent needs access to its DOM node.

That's exactly the point. A component should be able to define that its internal structure is opaque and outer component shouldn't mess around with its DOM nodes. In fact, it may chose to render null, multiple DOM nodes, nested nodes, change tags, render a Custom Element.

Therefore, React.findDOMNode(ref) in general is considered a bad idea since it breaks the encapsulation of the component.

There is another use case for refs of course. You can use them to call imperative methods on a class instance. However, that doesn't make sense on plain functions since they have no imperative methods associated with them.

@jimfb
Copy link
Contributor

jimfb commented Sep 22, 2015

Haha, internally at Facebook we've had some long discussions about this, but it's true that we should find a way to highlight the discussions externally. I really like your idea of having refs on stateless components throw/warn. Just created an issue for that.

You are correct, the root of the issue is that stateless components don't have instances (for performance reasons, we may never instantiate the component). There is no instance for us to return, because no instance was created.

As @sebmarkbage mentioned, findDOMNode breaks encapsulation and really is an escape hatch, so you need to be super careful with it anyway. If you need to get a DOM node, you can always safely wrap a component (stateful or stateless) in a composite component, and then you can attach refs to the composite component.

@sebmarkbage
Copy link
Collaborator

Technically we could have refs on these objects expose a placeholder object that can only be passed to findDOMNode.

However, I guess what is really going on is that I'm trying to constrain and discourage the use of findDOMNode because of its flaws. We couldn't kill it immediately because it is so frequently used, but once we have a new surface like this, we shouldn't expose more uses of it.

I guess we should be clear and honest about that. In the stress of a 0.14 release we didn't message this very well.

@jquense
Copy link
Contributor Author

jquense commented Sep 22, 2015

Thanks for the quick responses folks :)

There is another use case for refs of course. You can use them to call imperative methods on a class instance. However, that doesn't make sense on plain functions since they have no imperative methods associated with them

This certainly I agree with.

As @sebmarkbage mentioned, findDOMNode breaks encapsulation and really is an escape hatch, so you need to be super careful with it anyway.

I think this is exactly my point, its an escape hatch, but a necessary one. React DOM is sitting on top of a layer that it can't be completely coerced into a declarative API 100% of the time. I agree that ideally something like findDOMNode is an annoying reality but it is necessary.

I respect wanting to only push ppl towards "Good Ideas" but that is assuming that react can reasonably cover everything, which i think we can agree that it is not there (yet!). As of right now lots of component API's depend on access to the underlying DOM node do stuff like read offset's, absolutely position things, focus management, etc, etc. To which react offers no real good story for other then make ever single component that may need to be measured implement some sort of interface for this and how would you even do that? Or wrap in a stateful component, which seems like the sort of thing react should jsut do for me no?

@jimfb
Copy link
Contributor

jimfb commented Sep 22, 2015

To which react offers no real good story for other then make ever single component that may need to be measured implement some sort of interface for this and how would you even do that?

The story is that the parent component can wrap whatever component they're trying to measure/position in a composite component, to which a ref can be attached. The wrapper can just return the child and get the dom node by calling findDOMNode(this). And the wrapper can even expose functions/abstractions (like getMeasurements()) to isolate the parent from touching actual DOM nodes, or expose the actual DOM nodes. Ultimately everything is still completely within the control of the parent/owner, even if some third party implemented a component as stateless.

@sebmarkbage
Copy link
Collaborator

Yea, that's not a great story though.

The full story is: We had to finally release 0.14 which was long over due and doing so we chose the most restrictive API because it is easier to go from restrictive -> loose than the other way around. It seems like a good idea but we can reevaluate if this is a huge problem.

I'd like to evaluate the use cases though. Parent context allow us to do some new ways of communication that can help this.

@jquense
Copy link
Contributor Author

jquense commented Sep 22, 2015

because it is easier to go from restrictive -> loose than the other way around.

makes sense, and I'll be happy to post more use cases as and if they arise. Thanks for the thoughtful responses from both of ya'll.

@jfairbank
Copy link
Contributor

Edited: moved my comment to a new issue #4972.

@taion
Copy link

taion commented Nov 17, 2015

One place where this comes up is when using containers, like with Relay. Relay containers try to attach ref="component" to the wrapped component, which means that I can't use them with stateless components.

This is annoying because most of the time, the ref is irrelevant and doesn't allow doing anything, and it seems like it'd be nice sometimes to be able to use a stateless component there - but right now I can't.

@fabiomcosta
Copy link
Contributor

I think that a helpful error message when a ref prop is set on a stateless component would be a good way to fix this issue.

@jimfb
Copy link
Contributor

jimfb commented Jan 21, 2016

@fabiomcosta #4943

@alansouzati
Copy link
Contributor

hi @sebmarkbage, I have a use case that might help here.

I'm working on Grommet which is a UX framework on top of React. We decided to move some of our components to stateless functions, following the guidelines defined in 0.14 release notes.

So, most of the things are working very well for us except for the fact that we started to receive issues from existing clients that had a ref applied in our components that got converted to stateless functions. The use cases for them were focus management and positioning after resize.

We could document which components support refs and which don't, but this is not really a good solution in my opinion. I feel we are exposing the internals of our components which is not a good idea.

As a result, we are thinking on adding stateless functions only to the internal components. In our scenario, internal components are the ones used only inside our core and not exposed to users.

@vlinder
Copy link

vlinder commented Feb 4, 2016

We also would like to have refs on stateless components for taking measurements of wrapped components in HOCs. Is it possible to first wrap with another HOC and make a ref to that one or do we need a real DOM node in between?

@jimfb
Copy link
Contributor

jimfb commented Feb 4, 2016

@vlinder Yes, that's a common strategy we recommend. You don't need a DOM node in between.

@taion
Copy link

taion commented Feb 4, 2016

Can React itself do this? It really seems less than ideal to not be able to treat components as just these opaque things, and instead have to think about whether or not they're functional.

@jimfb
Copy link
Contributor

jimfb commented Feb 4, 2016

@taion It has been discussed (#4936 (comment)). It means you are creating instances, which means you loose out on the benefits of making them stateless functions in the first place. Attaching refs is really more of an escape hatch anyway, and should be avoided to the extent possible. People use refs more often than they probably should, largely because refs give them access to an imperative API, which aligns with the way they've historically been trained to write programs. But if you get into the functional midset when writing React code, your life will be much easier. Anyway, that's a whole other discussion.

@taion
Copy link

taion commented Feb 4, 2016

I agree that refs should be avoided in general, but as @jquense has mentioned, they're often unavoidable for doing DOM manipulation. In many cases for e.g. displays overlays, I really do need to get the DOM node corresponding to a React element, and I'd like to not burden users of my libraries with having to think about whether their components are stateless or not.

I think it's precisely because refs are a bit dirty that it should be okay to attach suboptimal behavior like creating instances to elements that have refs attached to them – the philosophy being that "if you use refs as a feature, you may incur performance costs".

@jquense
Copy link
Contributor Author

jquense commented Feb 4, 2016

As it stands now though Stateless components do actually have instances...I assume your concern there is more of a "lets not block future optimizations"? Even still, "refs" in this conversation is really more of a stand-in for the "root DOM node they render", which is probably always going to known to React, at least internally, right?

Right now refs are the only mechanism for reliable access to a child's DOM node, aside from annoying: findDOMNode(this).children[2].firstChild sort of stuff in the parent. Which si why I think folks want to see something first class here, even acknowledging that it would be a first class escape hatch.

@jimfb
Copy link
Contributor

jimfb commented Feb 4, 2016

@taion I think you are missing the point / missing what @vlinder said. You don't need to burden your users because you can wrap the user's component in your own stateful component, and attach the ref to your stateful component. https://gist.github.com/jimfb/32b587ee6177665fb4cf

@jquense Yes, future optimizations. No, it can't be a DOM node reference, because the referenced dom node could change when the stateless component rerenders, so it would need to be a handle to the component which could be updated by React as part of a rerender (aka: an instance).

@sebmarkbage
Copy link
Collaborator

If we in the future allow multiple components to be returned from render, findDOMNode won't work. Likewise it might return null if a component chooses. Drilling through it using findDOMNode is really the thing we recommend against.

However, due to the artifacts of CSS it can be a convenient way to handle layout. Likewise focus can be useful. However, we're currently rethinking layout and focus and we should be able to handle that using React concepts alone in the future.

@gaearon
Copy link
Collaborator

gaearon commented Feb 4, 2016

Right now refs are the only mechanism for reliable access to a child's DOM node, aside from annoying: findDOMNode(this).children[2].firstChild sort of stuff in the parent.

Note that you can pass refs in props if you need this kind of access.

function ComponentWithInput({ inputRef }) {
  return <div><input ref={inputRef} /></div>
}

function Parent() {
  return <ComponentWithInput inputRef={input => input && input.scrollIntoView()} /> // or whatever
}

It’s breaking encapsulation but it’s pretty much an explicit equivalent of findDOMNode(this).children[2].firstChild which is breaking encapsulation in a worse way.

@jquense
Copy link
Contributor Author

jquense commented Feb 4, 2016

@gaearon does that work? Stateless components can also not set refs, or maybe that's just string refs?

In any case it doesn't quite solve the issue which is that child components would need to coordinate with parent ones to provide specific data. As discussed that's a big win for encapsulation, but doesn't work for components like this that need to measure arbitrary children.

Which is all to say that nothing is currently impossible with the current api, but what taion said:

its less than ideal to not be able to treat components as just these opaque things, and instead have to think about whether or not they're functional

Also this is amazing:

...and focus and we should be able to handle that using React concepts alone in the future

I very seriously would do anything to get a focus model that isn't the DOM one. I think I could demonstrate that an uncomfortably large percentage of bugs/frustration/hacks in our ui libraries are focus related.

@mnpenner
Copy link
Contributor

mnpenner commented Jan 6, 2017

I needed to get the bounding rect of an arbitrary (potentially stateless) component from a HOC. The solution I came up with is:

render() {
    return (
        <span ref={n => {this.inputWrap = n;}}>
            <WrappedComponent {...attrs}/>
        </span>
    );
}

someOtherFunc() {
    if(this.inputWrap && this.inputWrap.firstChild) {
        let inputRect = this.inputWrap.firstChild.getBoundingClientRect();
    }
}

i.e., wrap your arbitrary component with a <span>. If you try to get the bounding rect of that <span> it won't work correctly, but you can use that <span> to get its firstChild which will be the DOM node of your wrapped component, unless it renders null in which case firstChild will also be null.

@A7DC
Copy link

A7DC commented Aug 6, 2017

Thanks @mnpenner, your solution worked for me. I've mentioned you in a StackOverflow and credited you with the solution. 👍

@chbdetta
Copy link

chbdetta commented Mar 23, 2018

@mnpenner If preserving original html structure matters, wrapping a span around the children is not desired.

I figured another uglier way to do this:

let uniq_number = 0;
const prefix = 'ugly';

// .... inside a component
constructor() {
  this.id = uniq_number;
  uniq_number += 1;
}

fnThatUseDomNode() {
  const dom = document.querySelector(`.${prefix}-${this.id}`);
}

render() {
  return cloneElement(this.props.children, {
     className: `${prefix}-${this.id}`,
  });
}

But this won't work on non-document environments.

I wish they give stateless component refs, it will be really useful when imperative APIs (like resizeObserver) needs to be invoked on arbitrary children.

@shepmaster
Copy link

React 16.3 introduces React.forwardRef. You can use this to transfer a ref to a prop of the stateless component, which can then use it as appropriate.

const UsesRefX = ({ innerRef, children }) => (
  <button ref={innerRef}>
    {children}
  </button>
);

const UsesRef = React.forwardRef((props, ref) => (
  <UsesRefX innerRef={ref} {...props} />
));

const Usage = () => (
  <UsesRef ref={alert} />
);

@rangoo94
Copy link

If you need refs for (not only) stateless components before React 16.3, I prepared minimal external package react-instantiable-stateless which handles that.

@esr360
Copy link

esr360 commented Jul 9, 2018

For my use case I need to call a function on the DOM element created by the stateless component when the DOM has loaded, which I would normally put within componentDidMount. I've ended up doing this, which works perfectly but just seems a bit hacky...

const Carousel = ({ slides, init, ...props }) => {
    let carousel;

    window.addEventListener('load', () => {
        init(ReactDOM.findDOMNode(carousel));
    }, true);

    return (
        <Module ref={node => carousel = node} {...props}>
            {slides.map((slide, index) => (
                <Component name='slide' key={index}>{ slide }</Component>
            ))}
        </Module>
    );
}

EDIT:

I have since come to learn that the above is also possible by simply doing:

const Carousel = ({ slides, init, ...props }) => (
    <Module ref={node => init(ReactDOM.findDOMNode(node))} {...props}>
        {slides.map((slide, index) => (
            <Component name='slide' key={index}>{ slide }</Component>
        ))}
    </Module>
);

Frankly, I've only arrived at this by throwing a ton of proverbial shit at a wall and seeing what sticks. I don't understand why this works, and I'm still suspecting that it is not the recommended way to achieve my desired outcome.

@PierBover
Copy link

@chbdetta 's (ugly but necessary) solution didn't work for me. For some reason the CSS class is not passed down to the functional children.

render () {
	const child = React.Children.only(this.props.children);
	const cloned = React.cloneElement(child, {
		className: prefix + this.id
	});
	return cloned;
}

So how can we find the DOM node of a functional children?

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