From 63099c40e69f7439dace594bb95b5e87734b946c Mon Sep 17 00:00:00 2001 From: Lulu Wu Date: Tue, 12 May 2020 04:35:34 -0700 Subject: [PATCH] Migrate Android view managers to type-safe commands generated by JS codegen Summary: ## Changelog: [General] [Changed] - Migrate Android view managers to type-safe commands generated by JS codegen. Reviewed By: JoshuaGross, mdvacca Differential Revision: D21406461 fbshipit-source-id: 93584b240314254675a36a58c4d0c0880d6889fb --- .../facebook/react/config/ReactFeatureFlags.java | 7 +++++++ .../react/uimanager/BaseViewManagerDelegate.java | 3 +++ .../uimanager/NativeViewHierarchyManager.java | 9 ++++++++- .../react/uimanager/ViewManagerDelegate.java | 6 +++++- .../AndroidDrawerLayoutManagerDelegate.java | 7 ++++--- .../AndroidSwipeRefreshLayoutManagerDelegate.java | 5 +++-- .../viewmanagers/AndroidSwitchManagerDelegate.java | 5 +++-- .../AndroidViewPagerManagerDelegate.java | 7 ++++--- .../react/viewmanagers/SwitchManagerDelegate.java | 6 +++--- .../swiperefresh/SwipeRefreshLayoutManager.java | 2 +- .../react/views/switchview/ReactSwitchManager.java | 2 +- .../views/viewpager/ReactViewPagerManager.java | 4 ++-- .../components/GeneratePropsJavaDelegate.js | 5 +++-- .../GeneratePropsJavaDelegate-test.js.snap | 14 ++++++++------ 14 files changed, 55 insertions(+), 27 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java index c04a883d5fb..c3428836cde 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -38,6 +38,13 @@ public class ReactFeatureFlags { */ public static boolean useViewManagerDelegates = false; + /** + * Should this application use a {@link com.facebook.react.uimanager.ViewManagerDelegate} (if + * provided) to execute the view commands. If {@code false}, then {@code receiveCommand} method + * inside view manager will be called instead. + */ + public static boolean useViewManagerDelegatesForCommands = false; + /** * Should this application use Catalyst Teardown V2? This is an experiment to use a V2 of the * CatalystInstanceImpl `destroy` method. diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManagerDelegate.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManagerDelegate.java index b1c531f8f60..f430398bacf 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManagerDelegate.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/BaseViewManagerDelegate.java @@ -113,4 +113,7 @@ public abstract class BaseViewManagerDelegate the type of the view supported by this delegate */ public interface ViewManagerDelegate { void setProperty(T view, String propName, @Nullable Object value); + + void receiveCommand(T view, String commandName, ReadableArray args); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidDrawerLayoutManagerDelegate.java b/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidDrawerLayoutManagerDelegate.java index 5985c4ff1b9..aee5f3f9c94 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidDrawerLayoutManagerDelegate.java +++ b/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidDrawerLayoutManagerDelegate.java @@ -47,13 +47,14 @@ public class AndroidDrawerLayoutManagerDelegate viewManager, T view, String commandName, ReadableArray args) { + @Override + public void receiveCommand(T view, String commandName, ReadableArray args) { switch (commandName) { case "openDrawer": - viewManager.openDrawer(view); + mViewManager.openDrawer(view); break; case "closeDrawer": - viewManager.closeDrawer(view); + mViewManager.closeDrawer(view); break; } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidSwipeRefreshLayoutManagerDelegate.java b/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidSwipeRefreshLayoutManagerDelegate.java index 56a18f9aba0..e5fc3ac7a2c 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidSwipeRefreshLayoutManagerDelegate.java +++ b/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidSwipeRefreshLayoutManagerDelegate.java @@ -47,10 +47,11 @@ public class AndroidSwipeRefreshLayoutManagerDelegate viewManager, T view, String commandName, ReadableArray args) { + @Override + public void receiveCommand(T view, String commandName, ReadableArray args) { switch (commandName) { case "setNativeRefreshing": - viewManager.setNativeRefreshing(view, args.getBoolean(0)); + mViewManager.setNativeRefreshing(view, args.getBoolean(0)); break; } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidSwitchManagerDelegate.java b/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidSwitchManagerDelegate.java index 5152015a956..1b2f225d390 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidSwitchManagerDelegate.java +++ b/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidSwitchManagerDelegate.java @@ -56,10 +56,11 @@ public class AndroidSwitchManagerDelegate viewManager, T view, String commandName, ReadableArray args) { + @Override + public void receiveCommand(T view, String commandName, ReadableArray args) { switch (commandName) { case "setNativeValue": - viewManager.setNativeValue(view, args.getBoolean(0)); + mViewManager.setNativeValue(view, args.getBoolean(0)); break; } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidViewPagerManagerDelegate.java b/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidViewPagerManagerDelegate.java index 2f7b5d0853d..68c110b3241 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidViewPagerManagerDelegate.java +++ b/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/AndroidViewPagerManagerDelegate.java @@ -43,13 +43,14 @@ public class AndroidViewPagerManagerDelegate viewManager, T view, String commandName, ReadableArray args) { + @Override + public void receiveCommand(T view, String commandName, ReadableArray args) { switch (commandName) { case "setPage": - viewManager.setPage(view, args.getInt(0)); + mViewManager.setPage(view, args.getInt(0)); break; case "setPageWithoutAnimation": - viewManager.setPageWithoutAnimation(view, args.getInt(0)); + mViewManager.setPageWithoutAnimation(view, args.getInt(0)); break; } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/SwitchManagerDelegate.java b/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/SwitchManagerDelegate.java index 5e419f8bd3f..65d557ced19 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/SwitchManagerDelegate.java +++ b/ReactAndroid/src/main/java/com/facebook/react/viewmanagers/SwitchManagerDelegate.java @@ -15,7 +15,6 @@ import com.facebook.react.bridge.ColorPropConverter; import com.facebook.react.bridge.ReadableArray; import com.facebook.react.uimanager.BaseViewManagerDelegate; import com.facebook.react.uimanager.BaseViewManagerInterface; -import com.facebook.react.uimanager.LayoutShadowNode; public class SwitchManagerDelegate & SwitchManagerInterface> extends BaseViewManagerDelegate { public SwitchManagerDelegate(U viewManager) { @@ -53,10 +52,11 @@ public class SwitchManagerDelegate viewManager, T view, String commandName, ReadableArray args) { + @Override + public void receiveCommand(T view, String commandName, ReadableArray args) { switch (commandName) { case "setValue": - viewManager.setValue(view, args.getBoolean(0)); + mViewManager.setValue(view, args.getBoolean(0)); break; } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/swiperefresh/SwipeRefreshLayoutManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/swiperefresh/SwipeRefreshLayoutManager.java index 5d7a4343db6..c8462a138cc 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/swiperefresh/SwipeRefreshLayoutManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/swiperefresh/SwipeRefreshLayoutManager.java @@ -131,7 +131,7 @@ public class SwipeRefreshLayoutManager extends ViewGroupManager @Override public void setNativeValue(ReactSwitch view, boolean value) { - // TODO(T52835863): Implement when view commands start using delegates generated by JS. + setValueInternal(view, value); } @Override diff --git a/ReactAndroid/src/main/java/com/facebook/react/views/viewpager/ReactViewPagerManager.java b/ReactAndroid/src/main/java/com/facebook/react/views/viewpager/ReactViewPagerManager.java index 010cdfa2872..bf6af60998b 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/views/viewpager/ReactViewPagerManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/views/viewpager/ReactViewPagerManager.java @@ -160,12 +160,12 @@ public class ReactViewPagerManager extends ViewGroupManager @Override public void setPage(ReactViewPager view, int page) { - // TODO(T52835863): Implement when view commands start using delegates generated by JS. + view.setCurrentItemFromJs(page, true); } @Override public void setPageWithoutAnimation(ReactViewPager view, int page) { - // TODO(T52835863): Implement when view commands start using delegates generated by JS. + view.setCurrentItemFromJs(page, false); } @Override diff --git a/packages/react-native-codegen/src/generators/components/GeneratePropsJavaDelegate.js b/packages/react-native-codegen/src/generators/components/GeneratePropsJavaDelegate.js index 08c99a48791..dd0258a488a 100644 --- a/packages/react-native-codegen/src/generators/components/GeneratePropsJavaDelegate.js +++ b/packages/react-native-codegen/src/generators/components/GeneratePropsJavaDelegate.js @@ -55,7 +55,8 @@ const propSetterTemplate = ` `; const commandsTemplate = ` - public void receiveCommand(::_INTERFACE_CLASSNAME_:: viewManager, T view, String commandName, ReadableArray args) { + @Override + public void receiveCommand(T view, String commandName, ReadableArray args) { switch (commandName) { ::_COMMAND_CASES_:: } @@ -198,7 +199,7 @@ function generateCommandCasesString( const commandMethods = component.commands .map(command => { return `case "${command.name}": - viewManager.${toSafeJavaString( + mViewManager.${toSafeJavaString( command.name, false, )}(${getCommandArguments(command)}); diff --git a/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GeneratePropsJavaDelegate-test.js.snap b/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GeneratePropsJavaDelegate-test.js.snap index 914f8e56da7..fda2e0e535e 100644 --- a/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GeneratePropsJavaDelegate-test.js.snap +++ b/packages/react-native-codegen/src/generators/components/__tests__/__snapshots__/GeneratePropsJavaDelegate-test.js.snap @@ -214,13 +214,14 @@ public class CommandNativeComponentManagerDelegate viewManager, T view, String commandName, ReadableArray args) { + @Override + public void receiveCommand(T view, String commandName, ReadableArray args) { switch (commandName) { case \\"flashScrollIndicators\\": - viewManager.flashScrollIndicators(view); + mViewManager.flashScrollIndicators(view); break; case \\"allTypes\\": - viewManager.allTypes(view, args.getInt(0), (float) args.getDouble(1), args.getDouble(2), args.getString(3), args.getBoolean(4)); + mViewManager.allTypes(view, args.getInt(0), (float) args.getDouble(1), args.getDouble(2), args.getString(3), args.getBoolean(4)); break; } } @@ -264,13 +265,14 @@ public class CommandNativeComponentManagerDelegate viewManager, T view, String commandName, ReadableArray args) { + @Override + public void receiveCommand(T view, String commandName, ReadableArray args) { switch (commandName) { case \\"handleRootTag\\": - viewManager.handleRootTag(view, args.getDouble(0)); + mViewManager.handleRootTag(view, args.getDouble(0)); break; case \\"hotspotUpdate\\": - viewManager.hotspotUpdate(view, args.getInt(0), args.getInt(1)); + mViewManager.hotspotUpdate(view, args.getInt(0), args.getInt(1)); break; } }