From 94ba059679aecb25b5e18266d62aab122b7e283b Mon Sep 17 00:00:00 2001 From: David Vacca Date: Mon, 4 Nov 2019 10:50:48 -0800 Subject: [PATCH] Introduce the IS_DEVELOPMENT_ENVIRONMENT flag Summary: This diff introduces the flag IS_DEVELOPMENT_ENVIRONMENT that will be used in Fabric to control the logging of props, localData and state ONLY during development. Using DEBUG mode to control the logging of this kind of data is not enough. Changelog: [internal] Reviewed By: JoshuaGross Differential Revision: D18290351 fbshipit-source-id: cf0824bd15b9f1c509bbb284b85761166099bc42 --- .../react/fabric/FabricUIManager.java | 15 +++++---- .../mountitems/PreAllocateViewMountItem.java | 32 +++++++++++++------ .../mountitems/UpdateLocalDataMountItem.java | 11 ++++++- .../mountitems/UpdatePropsMountItem.java | 11 ++++++- .../mountitems/UpdateStateMountItem.java | 11 ++++++- 5 files changed, 61 insertions(+), 19 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java index 5c7609b1076..3cb20d1429d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/FabricUIManager.java @@ -84,7 +84,10 @@ import java.util.concurrent.ConcurrentHashMap; public class FabricUIManager implements UIManager, LifecycleEventListener { public static final String TAG = "FabricUIManager"; - public static final boolean DEBUG = + // The IS_DEVELOPMENT_ENVIRONMENT variable is used to log extra data when running fabric in a + // development environment. DO NOT ENABLE THIS ON PRODUCTION OR YOU WILL BE FIRED! + public static final boolean IS_DEVELOPMENT_ENVIRONMENT = false; + public static final boolean ENABLE_FABRIC_LOGS = ReactFeatureFlags.enableFabricLogs || PrinterHolder.getPrinter() .shouldDisplayLogMessage(ReactDebugOverlayTags.FABRIC_UI_MANAGER); @@ -166,7 +169,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { mMountingManager.addRootView(rootTag, rootView); mReactContextForRootTag.put(rootTag, reactContext); String moduleName = ((ReactRoot) rootView).getJSModuleName(); - if (DEBUG) { + if (ENABLE_FABRIC_LOGS) { FLog.d(TAG, "Starting surface for module: %s and reactTag: %d", moduleName, rootTag); } mBinding.startSurface(rootTag, moduleName, (NativeMap) initialProps); @@ -186,7 +189,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { final int rootTag = ReactRootViewTagGenerator.getNextRootViewTag(); ThemedReactContext reactContext = new ThemedReactContext(mReactApplicationContext, rootView.getContext()); - if (DEBUG) { + if (ENABLE_FABRIC_LOGS) { FLog.d(TAG, "Starting surface for module: %s and reactTag: %d", moduleName, rootTag); } mMountingManager.addRootView(rootTag, rootView); @@ -429,7 +432,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { try { ReactMarker.logFabricMarker( ReactMarkerConstants.FABRIC_UPDATE_UI_MAIN_THREAD_START, null, commitNumber); - if (DEBUG) { + if (ENABLE_FABRIC_LOGS) { FLog.d(TAG, "SynchronouslyUpdateViewOnUIThread for tag %d", reactTag); } scheduleMountItem( @@ -547,7 +550,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { long batchedExecutionStartTime = SystemClock.uptimeMillis(); for (MountItem mountItem : mountItemsToDispatch) { - if (DEBUG) { + if (ENABLE_FABRIC_LOGS) { // If a MountItem description is split across multiple lines, it's because it's a compound // MountItem. Log each line separately. String[] mountItemLines = mountItem.toString().split("\n"); @@ -595,7 +598,7 @@ public class FabricUIManager implements UIManager, LifecycleEventListener { public void updateRootLayoutSpecs( final int rootTag, final int widthMeasureSpec, final int heightMeasureSpec) { - if (DEBUG) { + if (ENABLE_FABRIC_LOGS) { FLog.d(TAG, "Updating Root Layout Specs"); } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/PreAllocateViewMountItem.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/PreAllocateViewMountItem.java index fbee0addf1f..fad513a9ea0 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/PreAllocateViewMountItem.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/PreAllocateViewMountItem.java @@ -7,7 +7,8 @@ package com.facebook.react.fabric.mounting.mountitems; -import static com.facebook.react.fabric.FabricUIManager.DEBUG; +import static com.facebook.react.fabric.FabricUIManager.ENABLE_FABRIC_LOGS; +import static com.facebook.react.fabric.FabricUIManager.IS_DEVELOPMENT_ENVIRONMENT; import static com.facebook.react.fabric.FabricUIManager.TAG; import androidx.annotation.NonNull; @@ -48,7 +49,7 @@ public class PreAllocateViewMountItem implements MountItem { @Override public void execute(@NonNull MountingManager mountingManager) { - if (DEBUG) { + if (ENABLE_FABRIC_LOGS) { FLog.d(TAG, "Executing pre-allocation of: " + toString()); } mountingManager.preallocateView( @@ -57,13 +58,24 @@ public class PreAllocateViewMountItem implements MountItem { @Override public String toString() { - return "PreAllocateViewMountItem [" - + mReactTag - + "] - component: " - + mComponent - + " rootTag: " - + mRootTag - + " isLayoutable: " - + mIsLayoutable; + StringBuilder result = + new StringBuilder("PreAllocateViewMountItem [") + .append(mReactTag) + .append("] - component: ") + .append(mComponent) + .append(" rootTag: ") + .append(mRootTag) + .append(" isLayoutable: ") + .append(mIsLayoutable); + + if (IS_DEVELOPMENT_ENVIRONMENT) { + result + .append(" props: ") + .append(mProps != null ? mProps : "") + .append(" state: ") + .append(mStateWrapper != null ? mStateWrapper : ""); + } + + return result.toString(); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/UpdateLocalDataMountItem.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/UpdateLocalDataMountItem.java index c58e898e9f2..07104a3f529 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/UpdateLocalDataMountItem.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/UpdateLocalDataMountItem.java @@ -7,6 +7,8 @@ package com.facebook.react.fabric.mounting.mountitems; +import static com.facebook.react.fabric.FabricUIManager.IS_DEVELOPMENT_ENVIRONMENT; + import androidx.annotation.NonNull; import com.facebook.react.bridge.ReadableMap; import com.facebook.react.fabric.mounting.MountingManager; @@ -32,6 +34,13 @@ public class UpdateLocalDataMountItem implements MountItem { @Override public String toString() { - return "UpdateLocalDataMountItem [" + mReactTag + "]"; + StringBuilder result = + new StringBuilder("UpdateLocalDataMountItem [").append(mReactTag).append("]"); + + if (IS_DEVELOPMENT_ENVIRONMENT) { + result.append(" localData: ").append(mNewLocalData); + } + + return result.toString(); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/UpdatePropsMountItem.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/UpdatePropsMountItem.java index 8d2934e289e..8b980705e9d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/UpdatePropsMountItem.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/UpdatePropsMountItem.java @@ -7,6 +7,8 @@ package com.facebook.react.fabric.mounting.mountitems; +import static com.facebook.react.fabric.FabricUIManager.IS_DEVELOPMENT_ENVIRONMENT; + import androidx.annotation.NonNull; import com.facebook.react.bridge.ReadableMap; import com.facebook.react.fabric.mounting.MountingManager; @@ -28,6 +30,13 @@ public class UpdatePropsMountItem implements MountItem { @Override public String toString() { - return "UpdatePropsMountItem [" + mReactTag + "]"; + StringBuilder result = + new StringBuilder("UpdatePropsMountItem [").append(mReactTag).append("]"); + + if (IS_DEVELOPMENT_ENVIRONMENT) { + result.append(" props: ").append(mUpdatedProps); + } + + return result.toString(); } } diff --git a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/UpdateStateMountItem.java b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/UpdateStateMountItem.java index a8a73630077..2eae1879706 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/UpdateStateMountItem.java +++ b/ReactAndroid/src/main/java/com/facebook/react/fabric/mounting/mountitems/UpdateStateMountItem.java @@ -7,6 +7,8 @@ package com.facebook.react.fabric.mounting.mountitems; +import static com.facebook.react.fabric.FabricUIManager.IS_DEVELOPMENT_ENVIRONMENT; + import androidx.annotation.NonNull; import androidx.annotation.Nullable; import com.facebook.react.fabric.mounting.MountingManager; @@ -29,6 +31,13 @@ public class UpdateStateMountItem implements MountItem { @Override public String toString() { - return "UpdateStateMountItem [" + mReactTag + "]"; + StringBuilder result = + new StringBuilder("UpdateStateMountItem [").append(mReactTag).append("]"); + + if (IS_DEVELOPMENT_ENVIRONMENT) { + result.append(" state: ").append(mStateWrapper != null ? mStateWrapper : ""); + } + + return result.toString(); } }