facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
229.26k stars 46.95k forks source link

Refs don't work on Stateless Components #4936

Closed jquense closed 8 years ago

jquense commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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 commented 9 years ago

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

taion commented 9 years ago

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 commented 8 years ago

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 commented 8 years ago

@fabiomcosta https://github.com/facebook/react/pull/4943

alansouzati commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

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

taion commented 8 years ago

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 commented 8 years ago

@taion It has been discussed (https://github.com/facebook/react/issues/4936#issuecomment-142384665). 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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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 commented 8 years ago

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 commented 8 years ago

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 commented 8 years ago

@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.

varungupta85 commented 8 years ago

I ended up on this issue when I was trying to create a stateless component to show a form in my react-native app which contains some TextInput fields. When user focuses on any TextInput field, a keyboard is shown due to which we need to scroll the view up so that the TextInput box is not hidden behind the keyboard for which I need to find the node handle for the TextInput field using React.findNodeHandle by passing in the ref for the TextInput field. But since functional components can't have refs, I won't be able to use functional components in this case and ended up creating traditional react components which don't have any state.

peter-mouland commented 8 years ago

Sorry for commenting on an old issue, but I want to add my problem / half-solution.

our problem, like others here, is that our reusable components might be statefull or stateless.

In the below example Field is stateless. we want to know if it has text so that we can add a new class.


  onBlur() {
    this.setState({ hasEmailText: !!this.refs[inputName].value })
  }

  render() {
    return (
       <Field ref={inputName} type="email" onBlur={this.onBlur} 
                  { ...bem('input', { 'has-text' : hasEmailText }) }
       />
   )
 }
}

I was confused because i wrongly assumed refs were recommended way of doing this above old-skool events. That was until i found this page ... https://facebook.github.io/react/docs/forms.html

so i now have :

  onChange(event){
    this.setState({ hasEmailText : !!event.target.value })
  }

  render() {
    return (
       <Field type="email" onChange={this.onChange} 
                  { ...bem('input', { 'has-text' : hasEmailText }) }
       />
   )
 }
}

the remaining problem is how to find the input value on-load.

so i guess we are back to recommending a standard composite component for all components?

BLamy commented 8 years ago

Is there something wrong with this?

const CustomTags = () => {
  let input;

  const didClickButton = () => {
    console.log(input.value);
  };

  return (
    <div>
      <input ref={c => (input = c)} />
      <button onClick={didClickButton}>foo</button>
    </div>
  );
};
sophiebits commented 8 years ago

@BLamy No, that should work – though in most cases you will end up wanting state in your component so it doesn't come up too much.

sophiebits commented 8 years ago

I'm going to close this issue out now because I don't think we have any plans to change this. If anyone has more usage questions or needs code help, please ask on Stack Overflow.

bchenSyd commented 8 years ago

Can someone correct me if my understanding is wrong?

a ref, which essentially, a backing instance, can only be returned from a stateful component or a DOM, but not a stateless component, because there is no instance created for a stateless component.

http://www.webpackbin.com/4kcExnGXM

jimfb commented 8 years ago

@bochen2014 Conceptually, yes, that's correct.

mnpenner commented 7 years ago

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 commented 7 years ago

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

chbdetta commented 6 years ago

@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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

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 commented 6 years ago

@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?