React-transition-group: Understanding Transition exiting

[juandopazo]

Hi,

I'm looking at Transition in master and I'm trying to understand a design choice made in it because it affects the way my app works.

Consider the following steps:

• Render a TransitionGroup with one Transition item
• The Transition item is in ENTERED state
• After a user action, remove the Transition item
• The Transition element is going to start exiting. Exiting happens in the following stages:
  
  
  
  • First the Transition element is re-rendered
  
  
  • It's componentWillReceiveProps method takes the new state (EXITING) and stores it in an instance prop
  
  
  • In componentWillUpdate, it sets its state with the EXITING state
  
  
  • This causes a second re-render
  
  

The first re-render has the same status, so nothing should happen. Then the second re-render happens and the animation starts. This looks good in isolation. However, in the context of an application with a store, that first render might be dangerous. I could go into details about why I think it's dangerous, but I don't want to write a gigantic wall of text.

What I'd like to understand is why the switch to the EXITING state happens in componentWillUpdate instead of componentWIllReceiveProps, causing just one re-render instead of two, so I can make a decision about how to address my problem (fork ReactTransitionGroup, contribute a change or something along those lines).

Thanks!

[jquense]

Hey there, I'm not sure if I'm misunderstanding what you're describing here but Transition doesn't do any work in componentWillUpdate ?

[juandopazo]

You're right. It's componentDidUpdate.

[jquense]

Yes, it's very intentionally doing state updates on DidUpdate for a few reasons. Prop changes need to flush to the element before state changes otherwise it will swallow animations in certain cases. For instance if you add change the className prop on the element and toggle in you usually need the class you've changed to make it to the DOM before the state change.

I'm not sure i understand why the "extra" render is a problem. If your components are structured such that they are assuming numbers of re-renders, something is wrong with the component design. We aren't given an guarantees about when react will re-render something, and the React model is to not care, renders should be idempotent.

[juandopazo]

It's a timing thing. The problem I have is not because of the re-render, but the re-render prevents a possible solution to the problem.

Let's say we have a store which contains a normalized list of movies:

{
  movies: ['a', 'b'],
  moviesById: {
    a: {
      name: 'Jurassic Park'
    },
    b: {
      name: 'Godzilla'
    }
}

We have two components, which get data from the store directly:

class MovieList extends Component {
    render() {
        return (
            <TransitionGroup>
                {this.props.movies.map(id =>
                    <Transition>{status => <Movie id={id} status={status} />}</Transition>)}
            </TransitionGroup>
        );
    }
}

class Movie extends Component {
    render() {
        return <span>{this.props.name}</span>;
    }
}

(You can assume react-redux or a similar mechanism)

Let's say we want to remove an item from the list and have it animate out when removed. Then if we remove both the entry in the movies array and in the moviesById object, then while the item animates out, the Movie component lost access to the data and re-renders with no name (or it could potentially even throw depending on how you wrote your code).

To address this, I was considering taking a snapshot of the DOM (cloneElement or something like that) and animate that when switching to the EXITING state of Transition. However, with the extra ENTERED re-render, by the time EXITING happens, it's too late to take the snapshot.

[juandopazo]

I'll close this since you have a ton of issues and figure out whether to open a PR with an option around this or if there's an alternative.

[Andarist]

@juandopazo Have you figured out any clean solution for this problem? Ideally we could just freeze react's subtree so any setState within it would be a noop, but dont have this power right now.

[silvenon]

The example is not fully complete, so I'm not sure where exactly you're mapping the id to moviesById. Could you fill in those gaps in this demo?

[Andarist]

Here you go - https://codesandbox.io/s/pjlrzkqjvj

Problem occurs when a transitioned child receives an update (ie from redux store).

[silvenon]

Oh, Movie itself is connected to the Redux store. I know that libraries should aim to allow for maximum flexibility, but this error really makes sense to me, it's expected that the component would crash if it's trying to fetch non-existent data. I suggest passing that data to Movie directly instead:

<Transition key={id} timeout={500}>
  {state => <Movie movie={moviesById[id]} />}
</Transition>

[Andarist]

I know the error is completely understandable, I just wish for some clean solution for this. Ideally Movie shouldn't be really aware that it might be gone for a brief moment and it shouldn't use any additional guards or caching.

Problem is also that connecting list + items is generally more performant in redux according to general belief, some benchmarks (including mine own on our real-world app), so refactoring this to pass movie object to this component is a definite no go for us.

That being said ofc I've worked around it, just looking around for alternative approaches

[silvenon]

I understand. And you're right, Movie shouldn't have to be aware of this, so I'm changing my opinion and reopening this to encourage further discussion.

[Andarist]

Well, after thinking about it for quite some time I came to a conclusion that there is no way to workaround this with current APIs (in general, not specifically ones from react-transition-group).

The cleanest solution I can think of is to open an RFC about "freezing" React's subtree, ain't sure yet what ideal API this would have and what kind of API would be performant for this, but gonna think about it some more. Problem is that arbitrary component at any depth level should get frozen within the subtree and I'm not sure how this could be queried by a component in efficient way.

[jquense]

Does something like react-static-container work here? That usually what I go with when I need to block updates,but it doesn't freeze the whole subtree I'm not sure that's possible neatly...

[Andarist]

Unfortunately react-static-container wouldn't work here as it can "freeze" only a single child with dynamic props, can't do much for nested components that receives updates.

I suspect this issue might get more and more relevant with people using new context API more and more.

[juandopazo]

I haven't come back to this problem yet. In our case actually removing something from the store happens rarely, so this only affects some edge cases. For now we're rendering blank when this happens (we don't render the crashing subtree). Maybe we can get a React dev to give an opinion. @sebmarkbage?

[Andarist]

Leaving it here for others. I've finally solved my issue with:

const createCacheableSelector = (dependencyResolver, selector) => {
  let cached

  return state => {
    const dependency = dependencyResolver(state)

    if (dependency === undefined) {
      return cached
    }

    const result = selector(state, dependency)
    cached = result
    return result
  }
}

// ...

export default connect((_state, { id }) => {
  return createCacheableSelector(
    state => getItem(id),
    (state, item) => ({
      item,
      other: selectOther(state)
    }),
  )
})(Item)

This still very much sucks, because my connect has to know that it's wrapped in smth like react-transition-group, but at least I don't have to handle absence of that item in my connected component (in my case fortunately it won't fire any dispatch action when unmounting)