Summary:
This moves all deps on the checked in fbreact/specs files to use the generated output (during build time) instead.
TLDR:
`react_native_target("java/com/facebook/fbreact/specs:FBReactNativeSpec")` ==> `react_native_root_target("Libraries:generated_java_modules-FBReactNativeSpec")`
Changelog: [Internal]
Reviewed By: RSNara
Differential Revision: D24520293
fbshipit-source-id: 3fb34f172f1ab89b7198dfb4fccf605fbc32d660
Summary:
When switching between non-Fabric and Fabric screens, I believe that `initializeEventListenerForUIManagerType` is not always being called on the NativeAnimatedNodesManager if
`NativeAnimatedModule.initializeLifecycleEventListenersForViewTag` is being called before the NativeAnimatedNodesManager ivar has been set. This should occur very infrequently, but logs
indicate that it /does/ happen in some marginal cases. Protecting against these cases should be trivial, by using the `getNodesManager` method which is responsible for returning a nodes manager or creating a new one.
The existing uses of the `NativeAnimatedNodesManager` ivar also occur on different threads and we were not protecting against this, so I'm changing it to an atomic. It's very likely that
the inconsistency issues in the past were caused not by ordering errors, but thread races.
Changelog: [Internal]
Reviewed By: fkgozali
Differential Revision: D24267118
fbshipit-source-id: 68abbff7ef3d0b2ecc9aa9977165663ad9447ab8
Summary:
Previously this was crashing only in debug, but that's too noisy and isn't giving us any value for now.
Changelog: [Internal]
Differential Revision: D23338800
fbshipit-source-id: bf1535cdda231ccf30af6d00509eec1499a552a1
Summary:
In the past I tried a few heuristics to guess when a batch of Animated Operations were ready, and none of these were super reliable. But it turns out we can safely allow JS to manage that explicitly.
Non-Fabric still uses the old behavior which seems fine.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D23010844
fbshipit-source-id: 4c688d3a61460118557a4971e549ec7457f3eb8f
Summary:
After D22801173 (https://github.com/facebook/react-native/commit/9e6ba9ddb8608d4e3a4a80d0138600130b766d4c) has landed, the native mechanisms in NativeAnimated to delay queued items from immediate execution are no longer necessary.
Fabric and non-Fabric animations both look smooth after deleting this code.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22807906
fbshipit-source-id: 9241fff84376f6aa9a35049cc40dfd6561effaa1
Summary:
This exception will be more disruptive during development than necessary.
This can be upgraded again if there's a reasonable root-cause, but right now this error isn't actionable enough, is too common, and the repercussions aren't obvious.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22775734
fbshipit-source-id: 2cd9449f5b78025f7a230fbbd5f2e87bce183d04
Summary:
Due to subtle differences in lifecycle on the native side, as well as in JS, Fabric constructs partial graphs more frequently than non-Fabric RN did.
We still crash if we detect a cycle, which we check for more explicitly now; and we still always crash in non-Fabric. But if we detect a partial graph in Fabric,
we warn instead of crashing. We also print the state of the graph before crashing/warning, to assist in debugging in production.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D22752291
fbshipit-source-id: f452892678fbe7b5a49f93644d39d3b6ae5bda75
Summary:
Apparently it's possible for `!isEmpty()` to be true and `peek` to be non-null, but for `poll()` to be false. It doesn't really make sense to me, and I don't have a repro, but that's what logs are showing.
I suspect a teardown race condition or /maybe/ a Fabric/non-Fabric handoff race condition. Neither of those make a ton of sense, though.
The mitigation is fairly straightforward: we are just much more cautious with how we treat the queue and assume everything could be null.
This impacts Fabric and non-Fabric equally.
Changelog: [Internal]
Reviewed By: motiz88
Differential Revision: D22593924
fbshipit-source-id: 7748121951a64941fa6da2bd25ebf070be6dc89c
Summary:
There are potential race conditions in the old implementation that could result in operations being executed out-of-order when the user transitions between Fabric/non-Fabric screens.
Specifically, if the non-Fabric path queues up a batch of operations to execute on the next UI tick, it is possible that Fabric lifecycle events fire /before/ that next UI tick and synchronously execute a batch of animation operations on the UI thread, before their predecessors have executed.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22564633
fbshipit-source-id: 91c24547d5a682e61fc0c433302667330349a5f1
Summary:
See videos. In some cases when an `Animated.timing` animation was triggered, there are not necessarily any corresponding Fabric commits. This causes the animations to be queued up but never execute... until there's some Fabric commit at a distant point in the future.
We can't immediately flush all animation operations (see inline comments) but we also shouldn't wait forever. Thus, we wait 2 frames and then flush.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22490650
fbshipit-source-id: 1669bfed00f2a92b50f9558fc7ccaf71dc636980
Summary:
Expose `resolveCustomDirectEventName` from UIManager interface for Bridgeless Mode+NativeAnimatedModule.
We cannot remove the interface from the Non-Fabric UIManagerModule yet, as it would break downstream open-source dependencies, so I just marked it deprecated.
Note that this still doesn't totally fix issues with Bridgeless mode: generally I have to navigate to a Bridgeless surface, exit it, and then navigate back, and only on the 2nd navigation does everything seem to work properly...
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22483508
fbshipit-source-id: 685126e7e51aa5d0fd60ad5d4ecc44e8c6c3029d
Summary:
The previous assumption was that any animations would result in `addAnimatedEventToView` being called. That's not true in all cases, so sometimes we never subscribed to Fabric lifecycle events, preventing animations from working on some screens and for Venice.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22483509
fbshipit-source-id: c97576675902b4b9e1d4e659c7c1e24c5fe92946
Summary:
After animation has been finished using Native driver there is no final value passed from the native to JS side. This causes a bug from https://github.com/facebook/react-native/issues/28114.
This PR solves this problem in the same way as `react-native-reanimated` library. When detaching it is calling native side to get the last value from Animated node and stores it on the JS side.
Preserving animated value even if animation was using `useNativeDriver: true`
Fixes https://github.com/facebook/react-native/issues/28114
## Changelog
<!-- Help reviewers and the release process by writing your own changelog entry. For an example, see:
https://github.com/facebook/react-native/wiki/Changelog
-->
[Internal] [Fixed] - Save native Animated node value on JS side in detach phase
Pull Request resolved: https://github.com/facebook/react-native/pull/28841
Test Plan: Unit tests for added getValue method passed. Green CI
Reviewed By: mdvacca
Differential Revision: D22211499
Pulled By: JoshuaGross
fbshipit-source-id: 9a3a98a9f9a8536fe2c8764f667cdabe1f6ba82a
Summary:
Searching for details and maybe a fix for T68843308 crashing in disconnectFromView, "Attempting to disconnect view that has not been connected with the given animated node".
May be related to recent refactoring but it's not clear. Change logic slightly and add more diagnostic information.
Changelog: [Internal]
Reviewed By: shergin
Differential Revision: D22153179
fbshipit-source-id: b95dbaf01ae8bca154c61442898b0f9d3aebb4de
Summary:
If UIManager disappears, it's likely due to (1) teardown due to memory pressure, (2) teardown due to crash, (3) normal teardown.
In all of those cases, I would just want NativeAnimatedModule to stop executing and fail silently ASAP.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D22079209
fbshipit-source-id: 21650abdfdb119a6f4abccd6962d0c09f7c7c6cd
Summary:
Switch between "Fabric" and "Non-Fabric" modes based on which types of native Views are being attached to animations. Don't allow non-Fabric to drive Fabric animations and vice-versa.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21985411
fbshipit-source-id: fb9bef1e38375b384430b4e0275e7b6d62eda7a4
Summary:
This is the second part of a rewrite of D15390384, which allows Animated timing to be driven by Paper or Fabric.
The intuition is: we don't care which one drives the animation. We will expect one or both of them to issue a callback that operations are about to be executed, and the first one wins. The blocks will only execute once, the second time will be a noop.
I don't think there's a 100% safe way of reimplementing Native Animated Module for Fabric/Venice (without a new API and implementing in C++) since it's inherently disconnected from the commit process and the tree. This gets us slightly closer to visual functionality, though.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21698192
fbshipit-source-id: c11d3cebd12cfc8acf4b63c87ccbe62cdbd8b672
Summary:
This is (part of) a rewrite of D15390384.
This implements the lifecycle interface only for Fabric to signal to NativeAnimatedModule when preOperations are about to run / operations are about to be dispatched.
We will likely want to remove this mechanism entirely and rewrite NativeAnimatedModule in C++ for Fabric/Venice, but for now, I'm not sure of a better solution to unblock.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D21698193
fbshipit-source-id: a13d445073911fd63d896202a7a1bfbe1167038a
Summary:
This diff reduces exposure of UIManagerModule in the NativeAnimatedNodesManager class, this is necessary to enable NativeDriverAnimations in Venice
changelog: [Internal][Android] Internal change to enable native driver animations in RN bridgless mode
Reviewed By: ejanzer
Differential Revision: D21317629
fbshipit-source-id: 81cd4ade296de4757acefe566e1466154d6b4e4b
Summary:
This is the codemod portion of the parent diff.
I modified all call-sites to `ReactContext.getNativeModule` to do a null check on the returned NativeModule.
Changelog:
[Android][Fixed] - Check if NativeModules returned from CatalystInstanceImpl.getNativeModule are null before using them.
Reviewed By: JoshuaGross
Differential Revision: D21272028
fbshipit-source-id: 6bd16c6bf30605f2dfdf4c481352063712965342
Summary:
Fixed some typos in the comment.
## Changelog
[Internal] [Fixed] - Fixed typo in the comments
Pull Request resolved: https://github.com/facebook/react-native/pull/28269
Test Plan: Changes are only made in the comments, so test is not necessary.
Reviewed By: cpojer
Differential Revision: D20342637
Pulled By: shergin
fbshipit-source-id: f6e7dd538ee54c43e1570c35e1f8c4502054e328
Summary: Right now, animations don't work on Android in bridgeless mode because NativeAnimatedModule directly uses the UIManagerModule for various things. Previously, I added a bridgeless check to the module's `initialize()` method to avoid adding a listener on the UIManager; in this diff, I'm moving that check so that we also skip adding the lifecycle listener on the context in bridgeless mode, too. I'll remove this check once we come up with a replacement for the UIManagerModule API.
Reviewed By: JoshuaGross
Differential Revision: D19964171
fbshipit-source-id: 7c461f535e370b0e607c28905c505239cce0e157
Summary:
NativeAnimatedModule registers itself as a listener on UIManagerModule, which doesn't exist in bridgeless mode. We now have an API on ReactContext to detect if we're in bridgeless mode, so let's just bail out when that's the case (for now).
In the future, we'll need a replacement for this API on FabricUIManager (or somewhere).
Changelog: [Internal]
Reviewed By: PeteTheHeat, mdvacca
Differential Revision: D19185762
fbshipit-source-id: 1cf53304ab58a5b985c8f4806544da51f09e8ba5
Summary:
All these NativeModules are now: (1) type-safe, (2) TurboModule-compatible.
**Note:** We still need to update `{Catalyst,Work,Fb4a}TurboModuleManagerDelegate` to understand these TurboModules. I'll most likely write up that diff and stack this one on top of it.
Changelog:
[Android][Added] - Make NativeModules TurboModule-compatible
Reviewed By: mdvacca
Differential Revision: D18888735
fbshipit-source-id: 34df64dc70e3f3a0a0303c049861205f9d3fd2ed
Summary:
## Step 1
I'm going to make every Java NativeModule type-safe and TurboModule-compatible. The first step is to make sure that every type-unsafe NativeModule has a dependency on its spec's codegen target.
## Input
Module -> owner(Module): P123320255
Module -> name(Module): P123320256
Module -> owner(spec(name(Module))): P123320257
### Excluded NativeModules
NativeModules without Specs: P123320644
Java only NativeModules: P123320645
Changelog:
[Internal] - Add buck dependencies for NativeModule specs
Reviewed By: mdvacca
Differential Revision: D18781629
fbshipit-source-id: 89f39017b8224355d9d7b43bf6c162b0957760ee
Summary:
This diff re-throw and logs exceptions in the animated module of RN Android
Changelog: internal
Reviewed By: JoshuaGross
Differential Revision: D18694124
fbshipit-source-id: bb4cb56dce99f09c56b0bc62733e8264f2df5a3f
Summary:
There are some cases where restoring default values on component unmount is not desirable. For example in react-native-screens we want to keep the native view displayed after react has unmounted them. Restoring default values causes an issue there because it will change props controlled my native animated back to their default value instead of keeping whatever value they had been animated to.
Restoring default values is only needed for updates anyway, where removing a prop controlled by native animated need to be reset to its default value since react no longer tracks its value.
This splits restoring default values and disconnecting from views in 2 separate native methods, this way we can restore default values only on component update and not on unmount. This takes care of being backwards compatible for JS running with the older native code.
## Changelog
[General] [Fixed] - NativeAnimated - Don't restore default values when components unmount
Pull Request resolved: https://github.com/facebook/react-native/pull/26978
Test Plan:
- Tested in an app using react-native-screens to make sure native views that are kept after their underlying component has been unmount don't change. Also tested in RNTester animated example.
- Tested that new JS works with old native code
Reviewed By: mmmulani
Differential Revision: D18197735
Pulled By: JoshuaGross
fbshipit-source-id: 20fa0f31a3edf1bc57ccb03df9d1486aba83edc4
Summary:
Simplify the API of `getReactApplicationContextIfActiveOrWarn`. We don't need to pass so much information into this method to collect good SoftExceptions.
Changelog: [Internal]
Reviewed By: mdvacca
Differential Revision: D18134400
fbshipit-source-id: 0a250ab0252a44121f3339a31506a0a6c4c7cd35
Summary:
In D18032458 we introduce `getReactApplicationContextIfActiveOrWarn`. In this diff, modules that access a JS or Native module through ReactApplicationContext need to check if the CatalystInstance is still alive before continuing.
Changelog: [Internal]
Reviewed By: furdei
Differential Revision: D18032788
fbshipit-source-id: 5152783afd0b93b8ce0970fe4a509ea71396a54a
Summary:
We noticed a repro-able crash in Ride in T52804960 on Android Q due to NaN being passed into setCameraDistance
on View
see Oleg's related post: https://fb.workplace.com/groups/rn.support/permalink/2682537011794897/
It looks like a generic fix or wrapper around View setCameraDistance might be planned in T48580247
But in the meantime, it kind of maybe seems reasonable-ish to say, ~~if the value of an input node is NaN, don't use it in the math for this node?~~ if a one of the inputs for this node evaluates to NaN, update that input node first? But I'm not super familiar with the Animations library so maybe that's not a good idea, idk.
From what I can tell in our specific error, it's coming from an InterpolatedNode A based off an AdditionNode B which tried to add a ValueNode C + a InterpolatedNode D, but D had only just been created and not had it's first update, so it's value was NaN, and so when B runs it's update value of C + NaN means B's new values is also NaN, and A's subsequent update based on that now comes out to NaN. Atleast that's what it seems like based on Log statements.
Reviewed By: olegbl
Differential Revision: D16960177
fbshipit-source-id: 99c8ca35be4b5e99f7c21db6733ebd622ae39d07
Summary:
This diff migrates the usages Nullable and NonNull annotations to AndroidX instead of javax.
The purpose of this change is to bring consistency in the annotations used by the core of RN
Reviewed By: makovkastar
Differential Revision: D16054504
fbshipit-source-id: 21d888854da088d2a14615a90d4dc058e5286b91
Summary: After we ran google-java-format D16071725, some Javadocs which weren't properly written broke. This includes putting unordered and ordered lists not using <ul> and <ol>, putting code blocks and pseudo-graphics not using <pre>. I ran through all the changed classes and tried to fix the broken Javadocs.
Reviewed By: cpojer
Differential Revision: D16090087
fbshipit-source-id: f31971cbc0e367a04814ff90bbfb2192751d5e16
Summary:
This diff formats the Java class files inside xplat/js/react-native-github. Since google-java-format was enabled in D16071401 we want to codemode the existing code so that users don't have to deal with formatter lint noise at diff-time.
```arc f --paths-cmd 'hg files -I "**/*.java"'```
drop-conflicts
Reviewed By: cpojer
Differential Revision: D16071725
fbshipit-source-id: fc6e3852e45742c109f0c5ac4065d64201c74204
Summary:
Add example showing regression before this fix is applied.
https://github.com/facebook/react-native/pull/18187 Was found to introduce a regression in some internal facebook code-base end to end test which couldn't be shared. I was able to create a reproducible demo of a regression I found, and made a fix for it. Hopefully this will fix the internal test, such that the pr can stay merged.
## Changelog
[GENERAL] [Fixed] - Fix connection of animated nodes and scroll offset with useNativeDriver.
Pull Request resolved: https://github.com/facebook/react-native/pull/24177
Reviewed By: rickhanlonii
Differential Revision: D14845617
Pulled By: cpojer
fbshipit-source-id: 1f121dbe773b0cde2adf1ee5a8c3c0266034e50d
Summary:
This is already handled cleanly on the JS side of things in AnimatedInterpolation.js: https://github.com/facebook/react-native/blob/0ee5f68929610106ee6864baa04ea90be0fc5160/Libraries/Animated/src/nodes/AnimatedInterpolation.js#L133-L142
However, the native driver interpolation will try to divide by 0, produce NaN and then crash. This change just copies the logic from the JS version of the interpolation logic and adds it to the Java version.
Note that this bug only reproduces on Android Q. It seems that RenderNode::setCameraDistance now crashes when receiving NaN on Android Q.
Reviewed By: sahrens
Differential Revision: D15380844
fbshipit-source-id: cfa82d8f58574e1040a851aaa5b5af1e23c9daa8
Summary: This is removing packages and libraries from the repo. Any modified buck files simply change the redirect targets to something more appropriate (no logic actually changed)
Differential Revision: D14950721
fbshipit-source-id: 6c14f827b76ca1dbaf83dcb983930f362c6a27d4
Summary:
Allow interpolation of strings with useNativeDriver. This allows animating much more of react-native-svg. This PR adds support for native animation of lengths with units, path data, colors etc. Plus, fixing the redundantly created nodes and (and thus, previously incorrect) connection of native animated nodes, improving performance.
Docs will need to change, specifying that string interpolation works with the native driver as well.
[GENERAL] [Added] Add support for native driven string interpolation in Animated
[GENERAL] Fix exception: Expected node to be marked as "native"
[GENERAL] Fix connection of AnimatedNodes and creation of redundant AnimatedNodes
Pull Request resolved: https://github.com/facebook/react-native/pull/18187
Differential Revision: D14597147
Pulled By: cpojer
fbshipit-source-id: 82a948a95419236be7931a8cc4ff72f41e477e9c
Kevin Gozali
Joshua Gross
Oleg Lokhvitsky
TMaszko
David Vacca
Ramanpreet Nara
generatedunixname89002005287564
Jesse Katsumata
Emily Janzer
Janic Duplessis
Andres Suarez
Peter Laraia
Oleksandr Melnykov
Mikael Sand
Rick Ratmansky
Christoph Nakazawa
Max Komarychev