From b21a9419236175a1d7f93189a7aa33e143536f9a Mon Sep 17 00:00:00 2001 From: Oleksandr Melnykov Date: Tue, 1 Oct 2019 03:54:41 -0700 Subject: [PATCH] Fix order of method execution in Fabric ViewManager Summary: Because of the changes made in Fabric, the order of the execution of some methods in ViewManager has changed. In Paper the following order was guaranteed: `createViewInstance -> addEventEmitters -> updateProperties -> onAfterUpdateTransaction`. But in Fabric, the order is the following: `createViewInstance -> updateProperties -> onAfterUpdateTransaction -> addEventEmitters`. This change can actually break some existing view managers, because they rely on the fact that `addEventEmitters` will be called before `onAfterUpdateTransaction`. Check ReactVideoManager: ``` ReactModule(name = ReactVideoManager.REACT_CLASS) public class ReactVideoManager extends SimpleViewManager implements VideoManagerInterface { ... Override protected void onAfterUpdateTransaction(ReactVideoPlayer view) { super.onAfterUpdateTransaction(view); view.commitChanges(); } Override protected void addEventEmitters( final ThemedReactContext reactContext, final ReactVideoPlayer view) { view.setStateChangedListener( new ReactVideoPlayer.PlayerStateChangedListener() { ... } ``` As you can see there is a state change listener registered in `addEventEmitters` and `view.commitChanges()` can actually cause a state change. It means that if `onAfterUpdateTransaction` is executed before `addEventEmitters`, the state change listener will be added after a state change has happened and the event will be missed. Reviewed By: JoshuaGross Differential Revision: D17600308 fbshipit-source-id: 044e09e0d64973c8237876311d37c057a1ba384e --- .../src/main/java/com/facebook/react/uimanager/ViewManager.java | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java index 547260dfc41..0fb5268621a 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java @@ -80,7 +80,6 @@ public abstract class ViewManager @Nullable StateWrapper stateWrapper, JSResponderHandler jsResponderHandler) { T view = createViewInstance(reactContext, props, stateWrapper); - addEventEmitters(reactContext, view); if (view instanceof ReactInterceptingViewGroup) { ((ReactInterceptingViewGroup) view).setOnInterceptTouchEventListener(jsResponderHandler); } @@ -137,6 +136,7 @@ public abstract class ViewManager @Nullable ReactStylesDiffMap initialProps, @Nullable StateWrapper stateWrapper) { T view = createViewInstance(reactContext); + addEventEmitters(reactContext, view); if (initialProps != null) { updateProperties(view, initialProps); }