search

chenglou / react-tween-state

React animation.
Other
1.74k stars 70 forks source link

Move away from component.isMounted() #49

Closed swhite24 closed 8 years ago

swhite24 commented 9 years ago

Moves away from component.isMounted in order to be compatible with ES6 classes using react-mixin.

As a side note, isMounted() may be completely deprecated in the future anyway.

Also, I went ahead and checked in built files. I wasn't sure of your contribution guidelines, but in the event you'd rather I not do that, let me know and I'll revert.

Thanks!

chenglou commented 8 years ago

Hey! Sorry for the delay. This PR has diverged (entirely my fault) and while I was pushing in some upgrade I updated the code to remove isMounted() too.

In general I prefer explicitly canceling the rAF callback rather than putting a flag inside willUnmount (though in this case we need the flag too anyway, because of https://github.com/chenglou/react-tween-state/blob/c4301073c1c8423439dcc945552163205d682882/index.js#L144). The rationale is, just because we're leaking memory for only 16ms doesn't mean it's "correct". We're still leaking in theory and that's not the way to go.

Also yeah, no need to build the lib.

Sorry again for the delay and for closing this (btw, check out https://github.com/chenglou/react-motion).