Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Bridgeless with runtimeExecutor #5734

Merged
merged 28 commits into from
Apr 5, 2024
Merged

feat: Bridgeless with runtimeExecutor #5734

merged 28 commits into from
Apr 5, 2024

Conversation

WoLewicki
Copy link
Member

@WoLewicki WoLewicki commented Feb 27, 2024 •
edited
Loading

PR bringing 0.74/bridgeless mode support to the library.

@WoLewicki WoLewicki marked this pull request as ready for review March 1, 2024 12:44
@tomekzaw tomekzaw self-requested a review March 1, 2024 13:51
@tomekzaw tomekzaw mentioned this pull request Mar 4, 2024
Closed
tjzel
tjzel reviewed Mar 4, 2024
View reviewed changes
src/reanimated2/hook/useAnimatedRef.ts Outdated Show resolved Hide resolved
src/reanimated2/fabricUtils.ts Outdated Show resolved Hide resolved
src/reanimated2/shareables.ts Outdated Show resolved Hide resolved
tjzel
tjzel reviewed Mar 4, 2024
View reviewed changes
FabricExample/App.tsx Outdated Show resolved Hide resolved
@WoLewicki WoLewicki force-pushed the @wolewicki/bridgeless-mode-with-runtime-executor branch from c09e572 to e5a24cf Compare March 11, 2024 16:16
@tjzel tjzel mentioned this pull request Mar 18, 2024
Merged
tjzel
tjzel requested changes Mar 20, 2024
View reviewed changes
Copy link
Collaborator

@tjzel tjzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm adding some more review changes just to not lose those notions, but now I will proceed to split this PR into 0.74 support part and bridgeless part.

Common/cpp/ReanimatedRuntime/WorkletRuntimeDecorator.cpp Outdated Show resolved Hide resolved
Common/cpp/Tools/JSScheduler.cpp Outdated Show resolved Hide resolved
apple/native/NativeProxy.h Outdated Show resolved Hide resolved
apple/native/NativeProxy.mm Outdated Show resolved Hide resolved
apple/native/NativeProxy.mm Outdated Show resolved Hide resolved
src/reanimated2/globals.d.ts Outdated Show resolved Hide resolved
android/src/main/java/com/swmansion/reanimated/NodesManager.java Outdated Show resolved Hide resolved
android/src/main/java/com/swmansion/reanimated/ReanimatedModule.java Outdated Show resolved Hide resolved
android/src/main/java/com/swmansion/reanimated/ReanimatedPackage.java Outdated Show resolved Hide resolved
src/reanimated2/fabricUtils.ts Outdated Show resolved Hide resolved
@tjzel @WoLewicki
Fixes for Reanimated with Bridgeless (#5807)
d28993e 
<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

Fix some stuff in @WoLewicki's PR #5734. 

## 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. -->

---------

Co-authored-by: Wojciech Lewicki <wojciech.lewicki@swmansion.com>
This was referenced Mar 20, 2024
Merged
Merged
Merged
github-merge-queue bot pushed a commit that referenced this pull request Mar 27, 2024
@tjzel
Make /app good for for 0.74 (#5830)
915ff35 
## Summary

This PR is a part of ongoing effort to bring Reanimated to React Native
0.74 with bridgeless mode. It consists of following Pull Requests, which
are split for review convenience - but shouldn't be considered separate
entities. They have to be merged in the following order:

- #5830
- #5815
- #5834
- #5734 

## Test plan

:shipit:
github-merge-queue bot pushed a commit that referenced this pull request Mar 27, 2024
@tjzel @WoLewicki @piaskowyk
Add support for React Native 0.74 (#5815)
9485509 
## Summary

This PR is a part of ongoing effort to bring Reanimated to React Native
0.74 with bridgeless mode. It consists of following Pull Requests, which
are split for review convenience - but shouldn't be considered separate
entities. They have to be merged in the following order:

- #5830
- #5815
- #5834
- #5734 

## Test plan

:shipit:

---------

Co-authored-by: Wojciech Lewicki <wojciech.lewicki@swmansion.com>
Co-authored-by: Krzysztof Piaskowy <krzysztof.piaskowy@swmansion.com>
github-merge-queue bot pushed a commit that referenced this pull request Mar 27, 2024
@tjzel @WoLewicki @piaskowyk
Bump examples to 0.74 (#5834)
500f256 
## Summary

This PR is a part of ongoing effort to bring Reanimated to React Native
0.74 with bridgeless mode. It consists of following Pull Requests, which
are split for review convenience - but shouldn't be considered separate
entities. They have to be merged in the following order:

- #5830
- #5815
- #5834
- #5734 

## Test plan

:shipit:

---------

Co-authored-by: Wojciech Lewicki <wojciech.lewicki@swmansion.com>
Co-authored-by: Krzysztof Piaskowy <krzysztof.piaskowy@swmansion.com>
tjzel
tjzel reviewed Apr 2, 2024
View reviewed changes
...NativeVersionPatch/ReanimatedUIManager/latest/com/swmansion/reanimated/ReanimatedModule.java Outdated Show resolved Hide resolved
...NativeVersionPatch/ReanimatedUIManager/latest/com/swmansion/reanimated/ReanimatedModule.java Outdated Show resolved Hide resolved
apple/REAModule.mm Outdated Show resolved Hide resolved
apple/REAModule.mm Outdated Show resolved Hide resolved
apple/REAModule.mm Outdated Show resolved Hide resolved
WoLewicki
WoLewicki commented Apr 3, 2024
View reviewed changes
Copy link
Member Author

@WoLewicki WoLewicki left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I left some more comments. I see you did some more refactoring of the code. It could be done in the follow-up PR, but if you are confident that it works correctly then we can go with it imo 🚀

FabricExample/android/app/src/main/java/com/fabricexample/MainApplication.kt Show resolved Hide resolved
Common/cpp/NativeModules/NativeReanimatedModule.cpp Show resolved Hide resolved
Common/cpp/Tools/JSScheduler.h Show resolved Hide resolved
tomekzaw
tomekzaw approved these changes Apr 5, 2024
View reviewed changes
Copy link
Member

@tomekzaw tomekzaw left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not a big fan of these changes – some of them introduce code duplication and make things unnecessarily complex. We need to push forward with bridgeless mode support so let's merge it and work on it iteratively.

Common/cpp/Tools/JSScheduler.cpp Outdated Show resolved Hide resolved
Common/cpp/Tools/JSScheduler.cpp Show resolved Hide resolved
android/build.gradle Show resolved Hide resolved
tjzel
tjzel approved these changes Apr 5, 2024
View reviewed changes
Copy link
Collaborator

@tjzel tjzel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

:shipit:

@tjzel tjzel enabled auto-merge April 5, 2024 10:34
@tjzel tjzel added this pull request to the merge queue Apr 5, 2024
Merged via the queue into main with commit 2eb2882 Apr 5, 2024
14 of 19 checks passed
@tjzel tjzel deleted the @wolewicki/bridgeless-mode-with-runtime-executor branch April 5, 2024 10:39
@phucle1010 phucle1010 mentioned this pull request May 24, 2024
Open
@arenault-pass arenault-pass mentioned this pull request May 24, 2024
Closed
@phucle1010 phucle1010 mentioned this pull request Jun 24, 2024
Open
@tjzel tjzel mentioned this pull request Dec 3, 2024
Merged
github-merge-queue bot pushed a commit that referenced this pull request Dec 9, 2024
@tjzel
refactor: restore JSCallInvoker (#6783)
4f5690d 
<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

Turns out that majority of work with regards to using `RuntimeExecutor`
done in
- #5734
can be reverted since `jsCallInvoker` is staying in the API.

Thanks to that we can significantly cleanup the code which is helpful
for Worklets.


## Test plan

I tested the changes on 0.74, 0.75 and 0.76 for Android/iOS Paper/Fabric
and all worked well.
tjzel added a commit that referenced this pull request Dec 13, 2024
@tjzel
refactor: restore JSCallInvoker (#6783)
e3888cd 
<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

Turns out that majority of work with regards to using `RuntimeExecutor`
done in
- #5734
can be reverted since `jsCallInvoker` is staying in the API.

Thanks to that we can significantly cleanup the code which is helpful
for Worklets.


## Test plan

I tested the changes on 0.74, 0.75 and 0.76 for Android/iOS Paper/Fabric
and all worked well.
tjzel added a commit that referenced this pull request Dec 13, 2024
@tjzel
refactor: restore JSCallInvoker (#6783)
bf937e7 
<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

Turns out that majority of work with regards to using `RuntimeExecutor`
done in
- #5734
can be reverted since `jsCallInvoker` is staying in the API.

Thanks to that we can significantly cleanup the code which is helpful
for Worklets.


## Test plan

I tested the changes on 0.74, 0.75 and 0.76 for Android/iOS Paper/Fabric
and all worked well.
tjzel added a commit that referenced this pull request Dec 13, 2024
@tjzel
refactor: restore JSCallInvoker (#6783)
5530522 
<!-- Thanks for submitting a pull request! We appreciate you spending
the time to work on these changes. Please follow the template so that
the reviewers can easily understand what the code changes affect. -->

## Summary

Turns out that majority of work with regards to using `RuntimeExecutor`
done in
- #5734
can be reverted since `jsCallInvoker` is staying in the API.

Thanks to that we can significantly cleanup the code which is helpful
for Worklets.


## Test plan

I tested the changes on 0.74, 0.75 and 0.76 for Android/iOS Paper/Fabric
and all worked well.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lukmccall lukmccall lukmccall left review comments

@tjzel tjzel tjzel approved these changes

@piaskowyk piaskowyk piaskowyk approved these changes

@tomekzaw tomekzaw tomekzaw approved these changes

@kmagiera kmagiera Awaiting requested review from kmagiera

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@WoLewicki @lukmccall @tomekzaw @piaskowyk @tjzel