-
Notifications
You must be signed in to change notification settings - Fork 24.3k
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.
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
fix: make it possible to add multiple mountingOverrideDelegates #44927
fix: make it possible to add multiple mountingOverrideDelegates #44927
Conversation
I am aware that the name of the method should be probably changed from |
packages/react-native/ReactCommon/react/renderer/mounting/MountingCoordinator.cpp
Outdated
Show resolved
Hide resolved
Base commit: 8b53d41 |
@@ -203,7 +206,7 @@ ShadowTreeRevision MountingCoordinator::getBaseRevision() const { | |||
void MountingCoordinator::setMountingOverrideDelegate( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
shouldn't this be renamed to 'addMountingOverrideDelegate'?
@sammy-SC has imported this pull request. If you are a Meta employee, you can view this diff on Phabricator. |
looks good. I'm importing the diff. If you manage to do the rename, that's great. Otherwise, it can be done in subsequent diff. Thank you |
As mentioned in #44927 (comment), not changing the name would make it work for different RN versions, but since we don't care too much about it on new arch right now, I'll change it 😅 |
@sammy-SC changed from |
This pull request was successfully merged by @WoLewicki in 358fe46. When will my fix make it into a release? | How to file a pick request? |
Summary:
PR changing the single mountingOverrideDelegate to a vector of those, so other listeners can operate on the transaction. Used by
react-native-screens
in software-mansion/react-native-screens#2134 andreact-native-reanimated
in software-mansion/react-native-reanimated#6055.Till now, only one listener could be added there, meaning that e.g.
Layout Animations
fromreact-native
,Layout Animations
fromreact-native-reanimated
and listening forScreen
removal inreact-native-screens
could not operate at the same time.Changelog:
[GENERAL] [FIXED] - Add option for multiple
mountingOverrideDelegates
Test Plan:
The code of
LayoutAnimations
insidereact-native
should work the same since it will add just one listener then. For other cases, different libraries can read/mutate transactions.