Unify iOS initialization / Add support for 0.72.0-rc.5 by tomekzaw · Pull Request #4523 · software-mansion/react-native-reanimated

tomekzaw · 2023-06-02T08:50:52Z

Summary

This PR bumps react-native to 0.72.0-rc.5 and slightly changes initialization path on iOS (Paper) due to recent changes in the framework (facebook/react-native#37523).

Fixes #4521. Continues #3005.

On iOS, Reanimated needs to overwrite two React internal modules:

RCTUIManager → REAUIManager to intercept manageChildren calls in order to observe React tree changes (here)
RCTEventDispatcher → REAEventDispatcher to intercept events in the native code while still on the UI thread (here)

The rest of the initialization (injecting JSI bindings) can be safely done in installTurboModule method, as we already do on Android and iOS/Fabric.

Previously, this was done using categories (see UIResponder+Reanimated.mm), in particular by overwriting jsExecutorFactoryForBridge: method which is called during initialization. However, since 0.72.0-rc.4, this method is already implemented in RCTAppDelegate so the trick does no longer work, making the app fail with the following error "[Reanimated] The native part of Reanimated doesn't seem to be initialized".

In this PR, I've used a category to overwrite another method extraModulesForBridge which swizzles the implementation of jsExecutorFactoryForBridge method and runs REAInitializer before the original call.

As suggested by @kmagiera, alternatively we could just swizzle the methods of RCTUIManager and RCTEventDispatcher.

TODO

Restore support for 0.71 and below with #ifdefs

Test plan

MicroDroid · 2023-06-02T16:30:05Z

Any ETA for when this PR will land in a release?

tomekzaw · 2023-06-02T16:31:44Z

@MicroDroid Probably next week, before 0.72 stable is released.

MicroDroid · 2023-06-02T16:32:36Z

If I modify my package.json to pull the package straight from this pull request, would the package work?

I have no idea how pods work, not sure if something needs to be published somewhere

tomekzaw · 2023-06-02T16:48:35Z

@MicroDroid Yeah, you can install Reanimated from a particular commit, here's the command:

yarn add software-mansion/react-native-reanimated#a6b0515c0d0d554dc584d3cda6dba4ce3bba7bce

MicroDroid · 2023-06-02T16:50:26Z

ok cool, I see you made a bunch of commits just minutes ago, let me know when you think I should try upgrade

It's kinda blocking my work hence I am going fast at trying it out

MicroDroid · 2023-06-04T01:26:25Z

@tomekzaw

MicroDroid · 2023-06-04T01:39:39Z

I noticed that you, here, added React-RCTAppDelegate as a dependency under some conditions, so I rm -rf ./ios/Pods/, rm ./ios/Podfile.lock (I know I shouldn't), and did a Clean Build Folder in Xcode, and yet the issue persists.

I do not use Fabric, and I am using RN 0.72.0-rc.5 (confirmed by yarn why)

tomekzaw · 2023-06-04T06:53:34Z

@MicroDroid cd ios && rm -rf Pods Podfile.lock build && pod install usually works for me.

MicroDroid · 2023-06-04T07:04:22Z

Tried exactly that now, but still no go.

I don't feel like it's a caching issue... not sure what it could be though.

MicroDroid · 2023-06-04T07:07:04Z

I echo $RCT_NEW_ARCH_ENABLED and the result was , as expected.

MicroDroid · 2023-06-04T07:14:18Z

More interestingly, I went ahead and:

Added random characters in RNReanimated.podspec, and pod install errored, which means pod install is picking it up
I removed the if statement wrapping the missing dependency entirely
pod install
rm -rf build/
⌘B

... and still the same error

MicroDroid · 2023-06-04T07:20:03Z

This builds:

tomekzaw · 2023-06-04T13:04:39Z

@MicroDroid What happens if you run cd ios && find . -name "*RCTAppDelegate*"?

MicroDroid · 2023-06-04T13:39:18Z

./Pods/Pods.xcodeproj/xcuserdata/<user>.xcuserdatad/xcschemes/React-RCTAppDelegate.xcscheme
./Pods/Target Support Files/React-RCTAppDelegate
./Pods/Target Support Files/React-RCTAppDelegate/React-RCTAppDelegate-dummy.m
./Pods/Target Support Files/React-RCTAppDelegate/React-RCTAppDelegate.debug.xcconfig
./Pods/Target Support Files/React-RCTAppDelegate/React-RCTAppDelegate-umbrella.h
./Pods/Target Support Files/React-RCTAppDelegate/React-RCTAppDelegate-prefix.pch
./Pods/Target Support Files/React-RCTAppDelegate/React-RCTAppDelegate-Info.plist
./Pods/Target Support Files/React-RCTAppDelegate/React-RCTAppDelegate.modulemap
./Pods/Target Support Files/React-RCTAppDelegate/React-RCTAppDelegate.release.xcconfig
./Pods/Local Podspecs/React-RCTAppDelegate.podspec.json

tomekzaw · 2023-06-05T10:41:08Z

@MicroDroid Indeed, there's no RCTAppDelegate.h in your output. But it works for me locally and on our CI as well.

MicroDroid · 2023-06-05T11:14:57Z

Not sure honestly, I did pod install like a billion times, no clue what's happening

kmagiera

No super happy we are adding more version specific code. Let's get this one in but we should seriously consider either unifying initialization logic (it appears like we still use the same jsExecutorFactorySth method, so it looks like we should be able to do this across older versions too), or dropping support for older RN releases

ios/native/RCTAppDelegate+Reanimated.mm

tido64 · 2023-06-05T14:29:35Z

ios/native/RCTAppDelegate+Reanimated.mm

+    Class cls = [self class];
+    Method originalMethod = class_getInstanceMethod(cls, @selector(jsExecutorFactoryForBridge:));
+    Method swizzledMethod = class_getInstanceMethod(cls, @selector(swizzled_jsExecutorFactoryForBridge:));
+    method_exchangeImplementations(originalMethod, swizzledMethod);


Will this still work if the instance doesn't implement jsExecutorFactoryForBridge:?

I wonder if you should use something like this: https://github.com/microsoft/react-native-test-app/blob/2e5d0e64535230ed07a4ecf398387057b34b7121/ios/ReactTestApp/React%2BCompatibility.m#L7

I think you should also use a different prefix for the method name to avoid potential name clashes, e.g. reanimated_jsExecutorFactoryForBridge.

Hey @tido64, thanks for the review! I've renamed the prefix in d018b2c as you suggested 😄

As for the case when jsExecutorFactoryForBridge: is not implemented, indeed we don't handle it properly now but it looks like since 0.72.0-rc.4 this is always implemented in RCTAppDelegate, at least in greenfield apps. We could add a conditional on didAddMethod as you proposed but I'm not sure what would happen in the line where we call [self reanimated_jsExecutorFactoryForBridge:bridge] then. I need some more time to learn and think about it.

Thanks for the code pointer to RTASwizzleSelector, would you mind if we copy it to Reanimated codebase as a utility function? We also need to swizzle some methods of ViewControllers for Shared Element Transitions.

Finally, I'm curious to know what's the reason to use +initialize here, I found one article that recommends to perform swizzling always in +load method (link).

Thanks for the code pointer to RTASwizzleSelector, would you mind if we copy it to Reanimated codebase as a utility function? We also need to swizzle some methods of ViewControllers for Shared Element Transitions.

Feel free to copy it 👍

Finally, I'm curious to know what's the reason to use +initialize here, I found one article that recommends to perform swizzling always in +load method (link).

The reason behind using +initialize is because we only want to swizzle if the class is actually used. In our case, race condition isn't really a concern since React doesn't initialize modules on multiple threads (at least not currently). We also use dispatch_once here so its atomicity is ensured.

Feel free to copy it 👍

Thanks!

The reason behind using +initialize is because we only want to swizzle if the class is actually used. In our case, race condition isn't really a concern since React doesn't initialize modules on multiple threads (at least not currently). We also use dispatch_once here so its atomicity is ensured.

That makes perfect sense, thanks!

Will this still work if the instance doesn't implement jsExecutorFactoryForBridge:?

@tido64 I'm still thinking about this question. Our aim is to automatically initialize Reanimated for greenfield apps; if some custom app doesn't use RCTAppDelegate, it needs to perform initialization on its own. It looks like since 0.72.0-rc.4 jsExecutorFactoryForBridge: is already implemented in RCTAppDelegate (link). This means that we can assume that didAddMethod returns false which reduces to the code that we currently have in this PR, or am I missing something?

As far as I can tell, Reanimated only needs to swizzle jsExecutorFactoryForBridge: on Paper. Is that correct? RCTAppDelegate is only available from 0.70. In this version, jsExecutorFactoryForBridge: is only implemented iff New Architecture is enabled. I don't know how far back you intend to support, but if you follow React Native's support window, this change will:

Break 0.69 because RCTAppDelegate is missing

Break 0.70 because jsExecutorFactoryForBridge: is not implemented.

As far as I can tell, Reanimated only needs to swizzle jsExecutorFactoryForBridge: on Paper. Is that correct?

Yes, that's correct. This PR introduces swizzling in RCTAppDelegate only for React Native 0.72+:

react-native-reanimated/ios/native/RCTAppDelegate+Reanimated.mm

Line 1 in d018b2c

#if REACT_NATIVE_MINOR_VERSION >= 72 && !defined(RCT_NEW_ARCH_ENABLED) && !defined(DONT_AUTOINSTALL_REANIMATED)

For 0.71 and older we will still use the old way which is to implement jsExecutorFactoryForBridge: method for UIResponder:

react-native-reanimated/ios/native/UIResponder+Reanimated.mm

Line 1 in d018b2c

#if REACT_NATIVE_MINOR_VERSION <= 71 && !defined(RCT_NEW_ARCH_ENABLED) && !defined(DONT_AUTOINSTALL_REANIMATED)

Ah, I missed this part. Then it looks like you should be good for now.

hiagolfMB · 2023-06-08T21:27:40Z

Is it already available on npm?

tomekzaw · 2023-06-09T05:25:15Z

Yes, you can install nightly build with yarn add react-native-reanimated@nightly.

tomekzaw · 2023-06-09T09:56:40Z

@MicroDroid Here's the issue for the problem that you've mentioned: #4553

## Summary Recently, we began rewriting the iOS initialization process. Tomek moved the injection of JSI bindings to `installTurboModule` (check out #4523) and then used swizzling to eliminate the `REAEventDispatcher` (see #4576). More context here: expo/expo#23057 (comment) This PR removed UIManager overriding by swizzling the `_manageChildren` and `uiBlockWithLayoutUpdateForRootView` methods. These changes allowed us to get rid of `RCTAppDelegate+Reanimated`, `UIResponder+Reanimated`, and `REAInitializer`. For now, we're gonna leave `REAInitializer` as deprecated for a few more releases to maintain backward compatibility. But just a heads up, we'll be removing it in the future. ## Test plan - I tested building on different react native versions by CI https://github.com/piaskowyk/reanimated-rn-version-tester/actions/runs/5375166823 - Tested Layout Animations on Example app form main branch.

tomekzaw added 4 commits May 31, 2023 15:36

Bump react-native to 0.72.0-rc.4

eb248e2

Bump react-native to 0.72.0-rc.5

f215f99

Fix initialization by swizzling jsExecutorFactoryForBridge

73a8aca

Unify installTurboModule for Paper and Fabric

bcdeed6

tomekzaw mentioned this pull request Jun 2, 2023

iOS: Version 2.17.0 fails to build on React Native 0.72.0+ #4522

Closed

tomekzaw added 4 commits June 2, 2023 12:45

Update Podfile.lock

424cf72

Swizzle RCTAppDelegate instead

061475b

Restore backward compatibility

a780cba

Add #ifdef in REAInitializer

a6b0515

Remove unnecessary import

c0e26e8

kmagiera approved these changes Jun 5, 2023

View reviewed changes

ios/native/RCTAppDelegate+Reanimated.mm Show resolved Hide resolved

tomekzaw mentioned this pull request Jun 5, 2023

Inject _WORKLET to RN runtime #4511

Merged

tido64 reviewed Jun 5, 2023

View reviewed changes

Rename swizzling prefix

d018b2c

piaskowyk approved these changes Jun 6, 2023

View reviewed changes

Merge remote-tracking branch 'origin/main' into @tomekzaw/72rc5

19e1800

tomekzaw added this pull request to the merge queue Jun 7, 2023

Merged via the queue into main with commit a8206f3 Jun 7, 2023

tomekzaw deleted the @tomekzaw/72rc5 branch June 7, 2023 06:14

tomekzaw mentioned this pull request Jun 23, 2023

[dev-launcher] fix compatibility issue with reanimated expo/expo#23057

Merged

3 tasks

piaskowyk mentioned this pull request Jun 26, 2023

Swizzle RCTUIManager #4619

Merged

Uh oh!

Comments

Conversation

tomekzaw commented Jun 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

TODO

Test plan

Uh oh!

MicroDroid commented Jun 2, 2023

Uh oh!

tomekzaw commented Jun 2, 2023

Uh oh!

MicroDroid commented Jun 2, 2023

Uh oh!

tomekzaw commented Jun 2, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MicroDroid commented Jun 2, 2023

Uh oh!

MicroDroid commented Jun 4, 2023

Uh oh!

MicroDroid commented Jun 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

tomekzaw commented Jun 4, 2023

Uh oh!

MicroDroid commented Jun 4, 2023

Uh oh!

MicroDroid commented Jun 4, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

MicroDroid commented Jun 4, 2023

Uh oh!

MicroDroid commented Jun 4, 2023

Uh oh!

tomekzaw commented Jun 4, 2023

Uh oh!

MicroDroid commented Jun 4, 2023

Uh oh!

tomekzaw commented Jun 5, 2023

Uh oh!

MicroDroid commented Jun 5, 2023

Uh oh!

kmagiera left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

tido64 Jun 5, 2023

Choose a reason for hiding this comment

Uh oh!

tomekzaw Jun 6, 2023 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

tido64 Jun 6, 2023

Choose a reason for hiding this comment

Uh oh!

tomekzaw Jun 6, 2023

Choose a reason for hiding this comment

Uh oh!

tomekzaw Jun 6, 2023

Choose a reason for hiding this comment

Uh oh!

tido64 Jun 6, 2023

Choose a reason for hiding this comment

Uh oh!

tomekzaw Jun 6, 2023

Choose a reason for hiding this comment

Uh oh!

tido64 Jun 6, 2023

Choose a reason for hiding this comment

Uh oh!

hiagolfMB commented Jun 8, 2023

Uh oh!

tomekzaw commented Jun 9, 2023

Uh oh!

tomekzaw commented Jun 9, 2023

Uh oh!

Reviewers

tomekzaw commented Jun 2, 2023 •

edited

Loading

tomekzaw commented Jun 2, 2023 •

edited

Loading

MicroDroid commented Jun 4, 2023 •

edited

Loading

MicroDroid commented Jun 4, 2023 •

edited

Loading

tomekzaw Jun 6, 2023 •

edited

Loading