On chrome mobile after 2 animations with many leaves then react-flip-move gets stuck in state where many nodes hang about that should have been removed.

[insidewhy]

Using a recent version of mobile chrome go to kchomp.co and touch the worldnews link at the top followed by the news link as quickly as possible.

From here if you click the worldnews link again you should see that a bunch of the nodes related to news remain in the DOM. I verified with remote debugging that these nodes are no longer in props.children. From here clicking further links in the header leads to more and more HTML elements being left around. It seems that react-flip-move has gotten into a state where it can no longer remove needed elements.

I have been unable to replicate the issue on desktop chrome, it seems specific to mobile chrome. Using the latest react-flip-move (2.7.3).

[insidewhy]

I downgraded the react-flip-move version on kchomp.co to 2.6.5 and no longer see the issue, I'm thinking it might have been introduced with the 2.7 series.

[insidewhy]

I've tried some other versions: The bug manifests in 2.6.6 but animates differently after the stuck case. Version 2.7.1 manifests the bug more violently. 2.7.3 isn't as bad as 2.7.1 but seems slightly worse than 2.6.6 when it comes to this.

So I assume it was broken in 2.6.6, then got worse and was partially fixed between 2.7.1 and 2.7.3. Eventually I'll stick kchomp on 2.6.6 but I'll leave it on 2.7.3 for now so you can see the issue.

[joshwcomeau]

Hi @ohjames,

Right. Interrupts have always been a little tricky, especially when an item that was in the middle of exiting is re-entered.

I can't promise I'll get to fixing it soon, I have a lot of other things vying for my attention. For now, I'd keep using 2.6.5. I'm gonna try and roll a series of small bug fixes into a version at some point in the next month or two, will include this one.

[insidewhy]

Thanks for the info!

I went ahead and wrote my own react component that is very similar to react-flip-move but uses the "Web Animations" APIs. I wouldn't have been able to do it without your great article and the links you provided on the technique. Thanks! It's easier to cancel animations and react to events with that API, which made working around this problem much easier. It also makes scheduling easier, the webanimations are designed to start on the next frame so no requestAnimationFrame stuff is needed. Downside is that IE and Safari don't support web animatons yet. There's a polyfill to support them but I imagine it'll be much slower and may need some specialised code. I noticed performance was a little bit worse for my library than yours also. I managed to shave off some time by only animating nodes that are scrolled into view (or about to be).

[joshwcomeau]

Awesome, glad to hear it :) FlipMove's initial version was written with Web Animations API, but yeah, performance was noticeably worse, at least in the tests I did. I also didn't want to bloat the library with polyfills.

You're right, though; initially this library didn't have enter/leave animations, so the tradeoff wasn't so significant. At this point, it may be worth the tradeoff, since it would greatly simplify the code.

Anyway, glad to hear you found a solution that works :) is the code for it open-source? Would be curious to see how you dealt with cancellations.

[insidewhy]

I dumped the code I'm using here for now: https://github.com/ohjames/react-flip-webanimate I'll add some docs and make it more generic someday soon, maybe.

[jrmyio]

Using 2.8.0 I replicated the issue on desktop chrome. Elements that should have been removed get stuck in the DOM and start animating again each time another item is removed.

[joshwcomeau]

@ConneXNL cool, thanks for the heads-up. I'm pretty busy with work but I'll try and find some time to investigate. Is it specifically when adding/removing items quickly (eg. interrupting a leave animation)?

[jrmyio]

@joshwcomeau I just cloned the repo and playing with it although i never looked at the internals of FlipMove.

But what I found is that when quickly adding multiple children only the last "transitionend" (https://github.com/joshwcomeau/react-flip-move/blob/master/src/FlipMove.js#L340) callback is fired. The other "transitionend" listeners don't seem to be triggered (they do remain on the dom elements) because the transition/styles instantly update to their final position when a new item is added.

As a result https://github.com/joshwcomeau/react-flip-move/blob/master/src/FlipMove.js#L343-L348 is only called once (for the last animation). this.remainingAnimations will never become 0 again and thus the cleanup https://github.com/joshwcomeau/react-flip-move/blob/master/src/FlipMove.js#L350-L376 is no longer called, resulting in the this.state.children getting out of sync, which results in items staying in the DOM which causes all sorts of wierd flickering animations.

I don't know what part is unintentional, but I can imagine that when adding items quickly you want them all to be animated instead of having the non-last items to be instantly flip to their end style.

[insidewhy]

I found something similar, the transitionend event doesn't fire reliably in chrome or Firefox. I'm not sure if it can be fixed, too many race conditions to avoid. I'm now of the opinion that this can only be reliably solved when using Web Animation objects. Best you can do with this library as it stands is throttle your updates.

[jrmyio]

@ohjames I am not 100% sure if it fires unreliably on a browser level. In my last comment I explained why this is happening. The transition never ends when FlipMove re-renders with new items as the transition doesn't actually finish.

[joshwcomeau]

@ConneXNL and @ohjames thanks for digging into it.

@ConneXNL I'm not certain I follow; When adding items quickly, the style is updated, but the transition isn't cut off. The other items should still finish their transition (and thus trigger the callback). Maybe the problem is that it actually triggers 2 different transition events, and only accounts for 1 of them?

A future version of this project might indeed use Web Animations. The browser support at the moment is awful though, and the polyfill looks pretty massive.

In the meantime, a dirty fix could be to use a setTimeout to clear, instead of relying on transitionend. I had closed that PR because it felt like treating the symptom rather than the cause, but I'm not sure I see a cleaner solution.

[insidewhy]

Accepting that PR is probably the pragmatic thing to do for the short term.

[jrmyio]

@joshwcomeau did I make clear that by adding multiple items I meant adding them 1 by 1 instead of multiple at once.

So I trigger multiple renders of the flipmove component.

[jrmyio]

I am using this story to test the problem: https://gist.github.com/ConneXNL/8797d1e6a6d60927374e3968aab7c892

Just click the add button quickly a couple times. Notice how only the last item is animated?

Now click the remove button.

As you see the links get out of sync. The problem is: https://github.com/joshwcomeau/react-flip-move/issues/120#issuecomment-278045046

A naughty fix to at least let the FlipMove continue normally is to add:

this.remainingAnimations = 0;

at https://github.com/joshwcomeau/react-flip-move/blob/master/src/FlipMove.js#L106

I haven't gone over most of the other code of FlipMove, were quick re-renders of taken into account/tested?

[joshwcomeau]

@ConneXNL yeah, I understood. I'm still confused about the 'why', but I understand the 'what' :)

Thanks for the reproduction. I'm super busy this week but I'll dedicate some time this weekend to debugging, and if I can't come up with a solution I'll use the setTimeout solution for now.

[jrmyio]

I edited my last comment with a potential temporary fix.

[joshwcomeau]

Hm, so the problem with that fix is that I don't think it would fire off any of the user-supplied callbacks.

And no, in my own projects I've never needed to handle quick adding/removing. It's also a massive pain to write tests for this module, so the basic stuff is tested but none of the edge-cases.

[jrmyio]

@joshwcomeau thanks for your replies. I fiddled some more into the problem and found the cause of the problems.

My example from above: https://github.com/joshwcomeau/react-flip-move/blob/master/src/FlipMove.js#L106 shows how items stay in the DOM because of transitionend not being called. Notice how I am using the "fade" for the animations:

                                    enterAnimation="fade"
                                    leaveAnimation="fade"

However, it appears that when using an the default transition (elevator) my example does not break! All the transitionend are fired normally.

And this is where it gets troublesome. It seems to be related to whether the item is "moving on the screen" whether the transitionend is actually fired.

Only when we are using Elevator (fade, accordionHorizontal, accordionVertical all break)

                                    enterAnimation="elevator"
                                    leaveAnimation="elevator"

the items that are being added 1 by 1 all continue their move but all jump to a opacity(1) / scale(1)-size-wise. The reason why we see the items move with the 'elevator' animation is because of the scale transition (atleast thats the case in my example). The scale animation apparently somewhat continues (although bugged) and at least fires the transitionend event.

This means that the CSS Transitions (at least in my chrome version) are falsely animating the transition when the component is being re-rendered. On top of that, the transitionend is fired only if something moves on the screen.

To be 100% sure of my theory I tweaked the animations with a little hack and gave them all a transform: translate(0.001px,0)from which they start. This should not be visible on the screen but it tricks Chrome into thinking the items are being moved.

 export const enterPresets = {
   elevator: {
     from: { transform: 'translate(0.001px,0) scale(0)', opacity: 0 },
     to: { transform: '', opacity: '' },
   },
   fade: {
     from: { transform: 'translate(0.001px,0)', opacity: 0 },
     to: { transform: '', opacity: '' },
   },
   accordionVertical: {
     from: { transform: 'translate(0.001px,0) scaleY(0)', transformOrigin: 'center top' },
     to: { transform: '', transformOrigin: 'center top' },
   },
   accordionHorizontal: {
     from: { transform: 'translate(0.001px,0) scaleX(0)', transformOrigin: 'left center' },
     to: { transform: '', transformOrigin: 'left center' },
   },
   none: false,
 };

 export const leavePresets = {
   elevator: {
     from: { transform: 'translate(0.001px,0) scale(1)', opacity: 1 },
     to: { transform: 'scale(0)', opacity: 0 },
   },
   fade: {
     from: { transform: 'translate(0.001px,0)', opacity: 1 },
     to: { opacity: 0 },
   },
   accordionVertical: {
     from: { transform: 'translate(0.001px,0) scaleY(1)', transformOrigin: 'center top' },
     to: { transform: 'scaleY(0)', transformOrigin: 'center top' },
   },
   accordionHorizontal: {
     from: { transform: 'translate(0.001px,0) scaleX(1)', transformOrigin: 'left center' },
     to: { transform: 'scaleX(0)', transformOrigin: 'left center' },
   },
   none: false,
 };

The result? All the transitionend events are fired and no items get stuck in the DOM because of this. All in all this reminds me of the sad IE6 days :(.

The question is where to go from here. I think my hack could atleast solve FlipMove from breaking completely when adding/removing items quickly but it's still sad that the transitions aren't fully visible when the component gets re-rendered. I wonder what exactly FlipMove/React is doing with the DOM whenever a render is called. To me it seems like the transitions shouldn't break if the DOM of the transition item isn't touched at all.

[joshwcomeau]

Huh! That's super weird.

Thanks for digging into this. I'd be surprised if this was a React issue, it sounds like a Chrome issue. I'm gonna check to see if others have reported this issue, see if I can create a minimal recreation without react in a JSBin.

As a quick fix, I'll likely update the presets to do this weird hack, and throw a warning in the docs.

[jrmyio]

Did some more browser testing.

FireFox works completely, it even continues the actual transitions of the opacity/scaling etc without hacks.

Edge has the same problem as Chrome, but it doesn't want to be tricked. The translate(0.001px,0) has no effect whereas the actual the moving because of the scale does result in a transitionend.

There might be other ways to trick Edge though...

[jrmyio]

Ok, digged some more to see why we are even touching entering/leaving transitions because if we don't touch them they should just animate/call their transitionend.

Commenting the following lines seems to let FlipMove for me animate, and trigger its events in Chrome, Firefox and Edge without any problem without any other changes/hacks:

https://github.com/joshwcomeau/react-flip-move/blob/master/src/FlipMove.js#L229-L236

What are those lines for exactly?

[joshwcomeau]

Right, so the issue is for shuffling, unrelated to enter/leave.

If I recall correctly, let's say we have an item moving from 0px to 50px, and halfway through we trigger another shuffle. Without that, the item being shuffled will "jump" to 50px and begin transitioning from there.

If you remove that line and shuffle quickly, is it smooth? Possible those lines have been made redundant by other changes.

[jrmyio]

Tested in storybook, the lines are not made redundant by other changes. When I spam "Shuffle Items" on Enter/Leave Animations - composite > default (elevator preset) the shuffling shows unintended behavior when I uncomment those lines.

[jrmyio]

A problem might be that whenever FlipMove is rendered items that might actually still be "entering" (that don't have "entering" set to false yet) have their entering value removed on the next render.

Whereas they are then picked up by https://github.com/joshwcomeau/react-flip-move/blob/master/src/FlipMove.js#L229-L236 .

I find this all very hard to wrap my head around but I added the concept of 'entered' which is achieved when the animation hook is called and for the initial items. This so far doesn't seem to break the shuffling, and makes the transitions for adding items 1 by 1 work again.

https://github.com/ConneXNL/react-flip-move/commit/5f44a09bbd51ad8edcad6e4d706bf440e9b25534

Sadly tabs screwed up the commit but you can take a look at this solution and whether it makes sense.

[jrmyio]

I take back that we need 'entered' as a prop. I think we need to get rid of 'leaving' and 'entering' as currently it seems to be possible that element has both props set to true. Having multiple status props whereas an element can only have 1 active creates room for errors.

I suggest we create a 'status' instead that can either have 'leaving', 'entering' or 'entered' as its value.

[insidewhy]

Impossible to see what changed in that commit, you should get your editor/IDE's editorconfig plugin so it'll pick up indentation/style settings on a project wide basis.

[jrmyio]

@ohjames I know, but you can ignore that commit for now as just adding entered doesn't cut it.

I am currently in the progress in overhauling the "cild status" (aka leaving/entering) and seperating this in a seperate class that keeps track of the objects. So far it's looking good but I am still testing if I broke something else.

[joshwcomeau]

Yeah, good call on simplifying entering/leaving to a single prop.

Looking forward to seeing your solution @ConneXNL!

[jrmyio]

@joshwcomeau Things were coming along nicely until I hit the following CSS Transition limitation: http://jsfiddle.net/connexnl/qq6y4aww/2/

This basically happends when you are shuffling items and React calls appendChild() to move/readd an item. It basically cancels the ongoing transition (which looks bad) and the transitionend is never called as a result (even worse).

And here is how Flexbox could potentially work around the issue http://jsfiddle.net/connexnl/h783k0gv/ when the items would keep the same order in the DOM but have their order css property updated. I know this is kinda hacky :P

Another less hacky solutions would be too delay all shuffling until the very end (the moment the hook is called currently). All in all pretty hard to implement.

[joshwcomeau]

@ConneXNL riiight. Yeah.

I wanna avoid that flexbox solution, as yeah it's pretty hacky. Especially since the module currently works for most usecases without the hacks.

The timeout solution might be the way to go, then. Rather than relying on transitionEnd, we just register a timeout for durationms.

The tricky thing is if the duration prop changes, but that seems like an uncommon usecase.

I'd also want to check what the cost of having several simultaneous setTimeout calls running is. I don't expect it to be high, but I'd be curious.

[jrmyio]

@joshwcomeau I will give this thing a couple more hours. So far it still looks like transitionend is only not called when an item is re-added and the animation breaks anyway (so not that buggy as far as transitionend goes).

I almost have something fully working (i hope) where I catch this on https://github.com/joshwcomeau/react-flip-move/blob/master/src/FlipMove.js#L229-L236 . The transition would break anyway so we might aswell remove the transitionend handler here and call it manually.

I will get back to you on this with all the changes I made before the weekend.

Edit: I am now experimenting with getComputedStyle() with some success in order to re-start an animation if items were moved/shuffled during entering.

[jrmyio]

After 3 days non stop: https://github.com/ConneXNL/react-flip-move/commit/5a208e018e688e51ef7b1958487616e5361f2600

I am not proud of it. There are so many edge cases to cover when you want FlipMove to work with high frequency changes. This has resulted in (too) many changes.

The bottom line:

1) Moved all the information of childeren regarding their status to a seperate class: ChildStatusTracker. The tracker currently has methods for checking and setting all states. If preferred one could also use just use getStatus/setStatus or even remove this class in favor of a simple object in the FlipMove react component itself.

In any case, a single child can only have a single status. Here is a list of all the status types:

    // Items that are idle are not entering or leaving... (they might be moving though!)
    // Whenever entering/leaving finish the item will get this status.
    // Items that were already there before the first render will start as idle.
    IDLE: 'IDLE',

    // The child is getting ready to have its entering transition being added...
    QUEUED_TO_ENTER: 'QUEUED_TO_ENTER',

    // The child is currently in the middle of an entering transition...
    ENTERING: 'ENTERING',

    // The child is about to enter without animation...
    ENTER_WITHOUT_ANIMATION: 'ENTER_WITHOUT_ANIMATION',

    // The child is about to (re)enter without animation while it was still leaving...
    TOGGLED_TO_ENTERING_WITHOUT_ANIMATION: 'TOGGLED_TO_ENTERING_WITHOUT_ANIMATION',

    // The child is now scheduled to (re)enter while it was still leaving...
    TOGGLED_TO_ENTERING: 'TOGGLED_TO_ENTERING',

    // The child is getting ready to have its leaving transition being added...
    QUEUED_TO_LEAVE: 'QUEUED_TO_LEAVE',

    // The child is currently in the middle of a leaving transition...
    LEAVING: 'LEAVING',

    // The child is now scheduled to leave while it was still entering...
    TOGGLED_TO_LEAVING: 'TOGGLED_TO_LEAVING',

    // The item has finished its leaving transition and is waiting for the cleanup...
    LEFT: 'LEFT'

2) It refreshes starting/leaving animations when items have shuffled because when elements move transitions are interrupted and thus transitionend is not called. This seems to be standard for css transitions: http://jsfiddle.net/connexnl/qq6y4aww/2/.

3) If for some reason some 'transitionend' calls are still not called they will be picked up by a single setTimeout call; as a final fallback. I can only replicate this when I hit add/remove/shuffle for 5 minutes long. In other words I don't expect people to have this setTimeout ever called.

4) The transtionend handlers are now tracked in an object so that we can manually remove the eventListeners.

5) transitionEndHandler now wait for the transform property to complete as sometimes opacity is incorrectly triggered before a transform is completed.

6) Many more small changes.

I am aware the code got a lot messier but as it turned out a lot has to change in order for this library to work under stress. It will up to the maintainers whether they want to choose and pick certain parts of my re-work or go with it all the way.

[insidewhy]

@ConneXNL If you can handle all the cases my WebAnimation version of react-flip-move covers with transitionEnd then I'm super impressed. I spent about 20 hours trying to get it to work, constantly finding some new corner case it failed on.

[jrmyio]

@ohjames You can clone my repo and see if your use-cases /tests can break it. I focused for the most part on letting FlipMove not break during high frequency changes. There are still animations that can show wierd behavior when an item is (re)entering, leaving and moving in rapid speed.

[jrmyio]

Seems like my last commit wans't complete. I added the missing componentWillUpdate now otherwise the 'refreshing' of entering/leaving transitions wasn't working. https://github.com/ConneXNL/react-flip-move/commit/92832eba92c19f818b7a91276e088d774618f7a7 for the commit.

Edit: I am now able to replicate some instances where setTimeout is needed. I think these are such edge cases that for now we can rely on setTimeout to do a cleanup. However, it did contain a bug that prevented the cleanup from doing its work, this is now fixed in https://github.com/ConneXNL/react-flip-move/commit/1820a694525b2a654a15ffe142bf21c92fb142a9

[scamden]

was there ever a resolution here?

i'm running into this problem and it's resulting in users clicking a filtered option they didn't intend to since the opacity : 0 element is overlaid directly on the intended option.

[insidewhy]

@scamden nope I ended up writing my own version using WebAnimations. You can try that if you want.

[scamden]

@ohjames would love to. the repo said it's not ready for general use tho.. are you saying it may be ready?

[insidewhy]

@scamden I've been using it in a production site for several months now. It only supports vertical animation for now. And the browser needs WebAnimations or a polyfil.

[scamden]

ok thanks i'll give it a shot

[joshwcomeau]

Yeah, I haven't been able to give this issue the attention it deserves. Sorry about that!

Totally open if anyone wants to take a crack at solving it :)

[scamden]

Given the amount of time spend documented in these comments I'm honestly scared to, seems like a very tough issue On Wed, May 17, 2017 at 4:16 AM Joshua Comeau notifications@github.com wrote:

Yeah, I haven't been able to give this issue the attention it deserves. Sorry about that!

Totally open if anyone wants to take a crack at solving it :)

— You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub https://github.com/joshwcomeau/react-flip-move/issues/120#issuecomment-302059953, or mute the thread https://github.com/notifications/unsubscribe-auth/ABE5mMjKE5thDTVXlUWvzTq8FIKUcc36ks5r6tcjgaJpZM4LVz9c .

[insidewhy]

I say we port this library to WebAnimations or improve my version that already uses WebAnimations. IMO this just can't be done without them, at least not with so many hacks that the code becomes readable. I don't care which people do but that's some good advice there from someone who has spent more than 30 hours on this issue across 3 projects now. Aurelia, a fairly popular framework, still suffers from ridiculous bugs when using the CSS animation library and they have many more developers working on it.

[joshwcomeau]

Yeah, so the future is 100% WebAnimations API. It still has essentially no browser support, though, and from what I remember the polyfill is both larger than FlipMove itself, and isn't as performant. I could be mistaken, but I remember when I tried it, it was noticeably choppier than the current implementation. For an animation library, that's a big deal, lol.

I imagine once the browsers implement it, the performance should be fine; it's probably the polyfill itself that has the perf issues?

In the meantime, I'd encourage anyone who has this issue to give @ohjames' solution a try. Once the browser support situation changes (Chrome/Firefox/Safari), I think we should revisit this!

[insidewhy]

WebAnimations are only slightly slower in Chrome. After making the library only animate nodes that are scrolled into view it performs more than adequately in Firefox/Chrome. Safari though... Safari sucks at everything. grumbles

[trevorwhealy]

Throwing another vote in the "please support rapid addition/deletion" pile.

I tried to use @ohjames above module, but it did not integrate with the existing react-flip-move so I imagine there may be API differences, (not to mention it's not on the npm registry anyway).

We would love to use react-flip-move in production on a number of cool new animations, but it would need to support rapid typing. I'm in favor of embracing the webAnimation polyfill, even if it makes the package much larger.

But we're just one use case, and if rapid addition/deletion is not beneficial for the majority of use cases, then perhaps it is better left as is until there is wider browser support.

[insidewhy]

The API of my library is different but much more flexible due to the programmatic nature of WebAnimations.

[Hypnosphi]

There is a chance it could be solved if we would store styles in state and pass it to react children instead of applying directly. In that case the styles would be preserved during rerender. But this would mean a breaking change as it would require all children to accept style prop.

[cusspvz]

@Hypnosphi we can also remap children and refill styles.

EDIT: Example of children cloning/mapping

import React, { Children, cloneElement } from 'react'

// somewhere inside a render method...
    const elementStyle = { /* ... */ }
    const children = Children.map(
      this.props.children,
      ( element ) => {
        if ( ! element ) {
          return element
        }

        return cloneElement( element, { style: elementStyle })
      }
    )
//...