reagent-project / reagent

A minimalistic ClojureScript interface to React.js
http://reagent-project.github.io/
MIT License
4.75k stars 415 forks source link

Possible removal of `findDOMNode()` from React in a future version #251

Open ducky427 opened 8 years ago

ducky427 commented 8 years ago

There are rumblings in the React world of deprecating findDOMNode() which is equivalent to Reagent's reagent.core/dom-node.

@gaearon also tweeted about it here.

We may want to start planning for this in a future release.

Frozenlock commented 8 years ago

This is making me uneasy... I'm a heavy user of dom-node. I like being able to get my hands on the dom element.

gaearon commented 8 years ago

To be clear React still provides a way to get the node: with callback refs. I’m not sure how that would translate to your API but https://github.com/yannickcr/eslint-plugin-react/issues/678 has some React examples for better alternative patterns.

ducky427 commented 8 years ago

@Frozenlock, its actually pretty straight forward to get the actual dom element as @gaearon mentions.

I'll post a very simple example of this in reagent.

ducky427 commented 8 years ago

This example illustrates how to avoid using React's findDOMNode:

(defn hello-component
  []
  (let [dom-node (atom nil)]
    (reagent/create-class
     {:component-did-mount (fn [x] (js/console.log @dom-node))
      :reagent-render (fn [x] [:h2 {:ref (fn [y] (reset! dom-node y))} "hello"])})))

Note: dom-node is a clojurescript atom and not a reagent atom.

Repo with this code is here.

Edit: I've removed :component-will-unmount (fn [] (reset! dom-node nil)) after comment by @Frozenlock.

bhauman commented 8 years ago

Wow that is easy AND straight forward...

gaearon commented 8 years ago

Frozenlock commented 8 years ago

Looks simple enough.

I wasn't aware of the :ref field. If it doesn't have too many gotchas it could be the way going forward. Is it necessary to have it at the top level in a component?

ducky427 commented 8 years ago

@Frozenlock,

Regarding ref:

React supports a special attribute that you can attach to any component. The ref attribute can be a callback function, and this callback will be executed immediately after the component is mounted. The referenced component will be passed in as a parameter, and the callback function may use the component immediately, or save the reference for future use (or both).

More here.

So basically, you can place any function in ref. In my example, I created a function which just captures the referenced component in an atom. That atom didn't needn't be where it is.

Hope this makes sense.

Frozenlock commented 8 years ago

It does. If I understand correctly, it's a much simpler :component-did-mount.

Also, it appears that the unmounting function in the example is superfluous:

Note that when the referenced component is unmounted and whenever the ref changes, the old ref will be called with null as an argument. This prevents memory leaks in the case that the instance is stored, as in the second example.

ducky427 commented 8 years ago

@Frozenlock, that's true. I didn't realise that. I'll fix my example

bskimball commented 8 years ago

Is there an alternative for ReactDOM.findDOMNode (this.element).getClientBoundingRect ()?

gaearon commented 8 years ago

What is this.element?

everdimension commented 7 years ago

I think ref={n => this.el = n} is not an alternative to findDOMNode.

Let's say I want to create a <HashAnchor id="something" /> component which would accept any element (or component) as its child. Upon mount it would get the underlying dom node, calculate it's offset and if it's id matches the location.hash then it mount scroll it's child into browser's view.

I would use it like this:

<HashAnchor id="something">
  <AnyComponent />
</HashAnchor>

I can't use ref here because I can't know for sure that its child is going to be a DOM node; it might be a component. That's where findDOMNode comes in handy: it will find the underlying DOM node no matter what the child is.

I think this is exactly the case where "findDOMNode escape hatch" use is justified.

One still could argue that findDOMNode is an antipattern even in this case, because ideally we would want to add an id attribute to the underlying dom node in order to conform to the browser's native anchor behavior.

So we can just wrap the child with a <div id="something" />:

I would use it like this:

<HashAnchor id="something">
  <div id="something">
    <AnyComponent />
  </div>
</HashAnchor>
bskimball commented 7 years ago

this.element is a ref to a dom node. I was using findDomNode to get the bounding coordinates of a dom node.

everdimension commented 7 years ago

@bskimball if this.element is already a dom node then all you need is this.element.getClientBoundingRect() You don't need .findDOMNode

You need findDOMNode only when all you have is a reference to a component instance.

bskimball commented 7 years ago

I'm sorry I misrepresented this.element it is a component instance.

Deraen commented 7 years ago

@everdimension AFAIK Most React components support ref so you should be able to use <AnyComponent ref="ref-fn"/> (or [AnyComponent {:ref ref-fn}]).

everdimension commented 7 years ago

@Deraen not "most", but "stateful" components support refs, but the ref callback function doesn't give you a dom node; it gives you a component instance.

Deraen commented 6 years ago

Tagging as wontfix, as this doesn't seem to be needed in near future. The function is still available and not deprecated in React 16.

If we at some point change Reagent to generate functional components, we would need to revisit this as findDOMNode doesn't work for those. But that would be a lot work anyway.

Deraen commented 4 years ago

Related #490