-
Notifications
You must be signed in to change notification settings - Fork 47.2k
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
Comments
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, 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. |
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. |
Technically we could have refs on these objects expose a placeholder object that can only be passed to However, I guess what is really going on is that I'm trying to constrain and discourage the use of 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. |
Thanks for the quick responses folks :)
This certainly I agree with.
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? |
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 |
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. |
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. |
Edited: moved my comment to a new issue #4972. |
One place where this comes up is when using containers, like with Relay. Relay containers try to attach 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. |
I think that a helpful error message when a |
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. |
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? |
@vlinder Yes, that's a common strategy we recommend. You don't need a DOM node in between. |
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. |
@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. |
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". |
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: |
@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). |
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. |
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 |
@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:
Also this is amazing:
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. |
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 |
Thanks @mnpenner, your solution worked for me. I've mentioned you in a StackOverflow and credited you with the solution. 👍 |
@mnpenner If preserving original html structure matters, wrapping a 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. |
React 16.3 introduces const UsesRefX = ({ innerRef, children }) => (
<button ref={innerRef}>
{children}
</button>
);
const UsesRef = React.forwardRef((props, ref) => (
<UsesRefX innerRef={ref} {...props} />
));
const Usage = () => (
<UsesRef ref={alert} />
); |
If you need |
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 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. |
@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.
So how can we find the DOM node of a functional children? |
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.
The text was updated successfully, but these errors were encountered: