react-native-reanimated icon indicating copy to clipboard operation
react-native-reanimated copied to clipboard

Warn users to not use LayoutAnimation

Open hirbod opened this issue 2 years ago • 6 comments

in its current state, until the rewrite has been released. This issue is too impactful and it should be warned until its resolved.

Description

Changes

Test code and steps to reproduce

Checklist

  • [ ] Included code example that can be used to test this change
  • [ ] Updated TS types
  • [ ] Added TS types tests
  • [ ] Added unit / integration tests
  • [ ] Updated documentation
  • [ ] Ensured that CI passes

hirbod avatar May 06 '22 19:05 hirbod

The problem with deallocation of objects occurs only for absolutely positioned components such as modal. Right, It is an important issue but IMO not as important to discourage the use of Layout Animation in every case. Especially since we almost fixed it.

piaskowyk avatar May 12 '22 14:05 piaskowyk

I disagree on this one, since a single use is enough to crash your app. And it does also interfere with other libs like react-native-svg on android (massive duplication of nodes). I think a note about the pitfalls should be set. I've encountered plenty of users been stuck on "magical issues" that have been caused by this. I did not add this to trash talk anybody nor throw a bad light, you know I love what you guys are doing. I just think that as long as this issues are persistent and not fixed (also remember fixed versions of Expo), there has to be a warning.

hirbod avatar May 17 '22 14:05 hirbod

Agreed with @hirbod , a single usage of this (even with the patch in place) can cause plenty of issues across an application. Especially since we almost fixed it is not true in practice, been running this in prod for awhile with plenty of weird behaviour and crashes both iOS and Android side (with a simple implementation of the feature).

I had to implement custom native fixes myself to patch some of them, many developers who have not spent lots of time debugging these issues would have no clue what is happening. I love what the SM team is doing as well - you guys save us countless hours of work with these libraries. Yet not even warning users about this seems slightly careless

LeviWilliams avatar May 17 '22 19:05 LeviWilliams

This is such a simple yet important MR. I love SW as well, but this is such an insidious issue, and until it's fixed, it's borderline unethical to knowingly leave it without any warning to users. I've been following the GitHub issues associated with this for months now, as I assumed such a substantial issue would be addressed with some urgency, but there hasn't been any movement.

This is fine. You guys are doing great work for the community, and your priorities are yours alone. But adding a warning seems like the responsible thing to do at this point.

bfricka avatar Jul 21 '22 18:07 bfricka

Wow thank you for catching this. I had no idea this was going on.

shamilovtim avatar Jul 23 '22 03:07 shamilovtim

Has the issue been fixed yet? If so can someone link to the pr?

RayKay91 avatar Sep 08 '22 13:09 RayKay91

The rewrite is out in reanimated v3 rc.6 - this PR is obsolete now.

hirbod avatar Nov 18 '22 10:11 hirbod

What about everyone who is on V1 and V2. Shouldn't we warn them?

shamilovtim avatar Nov 29 '22 14:11 shamilovtim

The rewrite is going to be backported to v2 - so it's obsolete.

hirbod avatar Nov 30 '22 20:11 hirbod