joshwcomeau / react-flip-move

Effortless animation between DOM changes (eg. list reordering) using the FLIP technique.
http://joshwcomeau.github.io/react-flip-move/examples
MIT License
4.09k stars 258 forks source link

Use DOM children instead of findDOMNode #148

Open everdimension opened 7 years ago

everdimension commented 7 years ago

Hi! I saw this issue (https://github.com/joshwcomeau/react-flip-move/issues/89) but it looked to me like the discussion was going in the wrong direction.

I don't see why you need refs at all when you have node.children.

So based on your current code you could access all of the child elements in your <FlipMove /> component like this:

const childNodes = this.parentData.domNode.children;

And we basically get stateless functional components support for free (without asking users for extra stuff).

What's more, with this approach you could also handle non-flat item structures. Say we have a good old bootstrap layout:

<div class="row">
  <div class="col-md-6 item"></div>
  <div class="col-md-6 item"></div>
</div>
<div class="row">
  <div class="col-md-6 item"></div>
</div>

I gave each item an .item class. We could pass a selector prop to the <FlipMove /> component and get all of the child elements like this:

const childNodes = this.parendData.domNode.querySelectorAll(this.props.itemsSelector);

Obviously your code right now is based on the assumption that you use findDOMNode and expect a flat list of children items, so there could be some edge cases with my suggestion, but nothing unsolvable as far as I can tell.

everdimension commented 7 years ago

I'd be happy to hear your thoughts on this and perhaps start a PR if this change is welcome.

joshwcomeau commented 7 years ago

Ok, I have some thoughts!

(It's also Friday evening and my brain's a little fuzzy, so apologies in advance if this doesn't make total sense.)

So the reason it's done using findDOMNode is to try and maintain React's abstraction over the DOM. I'm not sure if it'd be feasible from a perf standpoint to read from the DOM during each render (AKA right before each animation begins). We'd have to test that.

Also, I think we'd still be limited to flat structures, because we use props.children for a bunch of stuff (including tracking enter/leave animations, interruptions, ...). We could theoretically track that stuff with regular objects instead of piggybacking on react elements - indeed, that's probably a better idea - but we still need the react elements to be passed to onStart/onFinish handlers. Unless we could work backwards from the DOM node to find the React element? I'm not sure that's possible, or if I like the sound of it, though.

(I also don't really like the idea of using CSS selectors; it complicates things for people who use CSS-in-JS or who obfuscate class names in production, and it generally feels like an antipattern to me in React. I'm cool with doing stuff like that from within Flip Move, because it's self-contained, but I wouldn't want to advocate using selectors. Although I also wouldn't want philosophy to get in the way of productivity).

I'm gonna think about this more over the next couple days, when my brain isn't so fried. It's an interesting idea though! If you want to do some experimentation, I think it could be worth exploring. I really like the idea of being able to use SFCs, as well as removing findDOMNode.

Hypnosphi commented 7 years ago

Unless we could work backwards from the DOM node to find the React element?

There is a way, but it's not a public API and may change at any time, so we shouldn't use it