Conversation
kmagiera
approved these changes
Feb 16, 2022
Member
kmagiera
left a comment
kmagiera
left a comment
There was a problem hiding this comment.
Looks good 👍 Left a couple of comments. Would be good to test jest environment with these changes as well
Member
Author
|
I can confirm that |
kmagiera
reviewed
Feb 18, 2022
| public class ReanimatedJSIModulePackage implements JSIModulePackage { | ||
|
|
||
| @Override | ||
| public List<JSIModuleSpec> getJSIModules( |
Member
There was a problem hiding this comment.
One thing that I think of now is that we can actually make the transition less of a pain and keep this class with empty implementation + some warning message. This way people won't get compile errors after upgrading. We can show a yellow box for them to update MainApplication.java
Member
Author
There was a problem hiding this comment.
I added @Deprecation annotation and Log.w(). LogBox doesn't have a native interface so we'd need to set some global variable or emit an event which isn't ideal.
piaskowyk
approved these changes
Feb 21, 2022
|
Is this supposed to work? I cleaned all Gradle caches and tried to reinstall multiple times, but it looks like Reanimated it not built, or not found |
3 tasks
piaskowyk
pushed a commit
that referenced
this pull request
Apr 1, 2022
## Description with #3005, we don't need the expo adapter to install JSI bindings. by removing the adapter, this pr will also fix #3084 and fix #3021 ## Changes remove expo adapter ## Test code and steps to reproduce 1. `$ createNPMPackage.sh` 2. create an expo sdk44 project, `yarn add file:/path/to/react-native-reanimated-2.5.0.tgz` and test launching. 2. create an react-native 0.67 project, `yarn add file:/path/to/react-native-reanimated-2.5.0.tgz` and test launching. ## 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
3 tasks
|
@renchap i am getting the same issue. did you solve it? |
|
@ugurdalkiran I did solve it, but I dont remember how. Probably either:
|
1 task
tomekzaw
added a commit
that referenced
this pull request
Jun 7, 2023
## 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](https://github.com/software-mansion/react-native-reanimated/blob/main/ios/LayoutReanimation/REAUIManager.mm)) * `RCTEventDispatcher` → `REAEventDispatcher` to intercept events in the native code while still on the UI thread ([here](https://github.com/software-mansion/react-native-reanimated/blob/663ee74925cafa4a1a802f4722466dc02cb1760f/ios/REAEventDispatcher.m#L9-L10)) 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`](https://github.com/software-mansion/react-native-reanimated/blob/663ee74925cafa4a1a802f4722466dc02cb1760f/ios/native/UIResponder%2BReanimated.mm#L19-L30)), 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 - [x] Restore support for 0.71 and below with #ifdefs ## Test plan <!-- Provide a minimal but complete code snippet that can be used to test out this change along with instructions how to run it and a description of the expected behavior. -->
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
5 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description
This patch removes setup instructions for Android. iOS is in progress (there's some complexity with bridge init lifecycle - Reanimated replaces native RN modules).
Changes
ReanimatedJSIModulePackagetoReanimatedModuleby utilizing@ReactMethod(isBlockingSynchronousMethod = true)annotation.Test code and steps to reproduce
I tested it on the example app but maybe it's worth building the package and checking on the freshly generated app.