From b2e2f43ec3d43a36c4682bf3f92b0154cc2dc55f Mon Sep 17 00:00:00 2001 From: Craig Martin Date: Thu, 6 May 2021 11:18:35 -0700 Subject: [PATCH 01/12] fix: codegen - project paths with spaces (#31141) Summary: - Fixed iOS codegen script incorrectly splitting root project paths that contain spaces https://github.com/react-native-community/releases/issues/214#issuecomment-793089063 iOS builds were failing on 0.64.0-rc.4 for projects that contained spaces in the root directory path. The error logs pointed to the codegen script not being able to find a directory. The path was being split at a space in one of the folder names. This PR modifies the codegen script to include the spaces and use the entire project root path. ## Changelog [Internal] fix: codegen script failing for iOS builds on projects with spaces in root directory path Pull Request resolved: https://github.com/facebook/react-native/pull/31141 Test Plan: Failing Test: Upgrade or init a new project and make sure that the project root directory contains a space (ex: /Users/test/cool projects/app/). With a clean install of node_modules and pods, attempt to build the project with Xcode. The build fails with an error running the script in FBReactNativeSpec (no such file or directory). Passing Test: Include the changes presented in this PR and rerun the failing test (clean node_modules + PR patch/clean pods). The app should build. Reviewed By: mdvacca Differential Revision: D28255539 Pulled By: hramos fbshipit-source-id: d44011985750639bd2fabfd40ed645d4eb661bd7 --- scripts/react_native_pods.rb | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/scripts/react_native_pods.rb b/scripts/react_native_pods.rb index dc87a308de058f..b61e2085de354a 100644 --- a/scripts/react_native_pods.rb +++ b/scripts/react_native_pods.rb @@ -159,9 +159,9 @@ def use_react_native_codegen!(spec, options={}) modules_output_dir = "React/#{modules_library_name}/#{modules_library_name}" # Run the codegen as part of the Xcode build pipeline. - env_vars = "SRCS_DIR=#{js_srcs}" - env_vars += " MODULES_OUTPUT_DIR=#{prefix}/#{modules_output_dir}" - env_vars += " MODULES_LIBRARY_NAME=#{modules_library_name}" + env_vars = "SRCS_DIR='#{js_srcs}'" + env_vars += " MODULES_OUTPUT_DIR='#{prefix}/#{modules_output_dir}'" + env_vars += " MODULES_LIBRARY_NAME='#{modules_library_name}'" generated_dirs = [ modules_output_dir ] generated_filenames = [ "#{modules_library_name}.h", "#{modules_library_name}-generated.mm" ] @@ -172,7 +172,7 @@ def use_react_native_codegen!(spec, options={}) # Eventually, we want these to be part of the same library as #{modules_library_name} above. components_output_dir = "ReactCommon/react/renderer/components/rncore/" generated_dirs.push components_output_dir - env_vars += " COMPONENTS_OUTPUT_DIR=#{prefix}/#{components_output_dir}" + env_vars += " COMPONENTS_OUTPUT_DIR='#{prefix}/#{components_output_dir}'" components_generated_filenames = [ "ComponentDescriptors.h", "EventEmitters.cpp", @@ -194,5 +194,5 @@ def use_react_native_codegen!(spec, options={}) :execution_position => :before_compile, :show_env_vars_in_log => true } - spec.prepare_command = "mkdir -p #{generated_dirs.reduce("") { |str, dir| "#{str} ../../#{dir}" }} && touch #{generated_files.reduce("") { |str, filename| "#{str} ../../#{filename}" }}" + spec.prepare_command = "mkdir -p #{generated_dirs.map {|dir| "'../../#{dir}'"}.join(" ")} && touch #{generated_files.map {|file| "'../../#{file}'"}.join(" ")}" end From b0e8c1eac0a9edda12ecfa264209a8b3222afe27 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Thu, 6 May 2021 12:12:10 -0700 Subject: [PATCH 02/12] Refactor UIManagerHelper.getUIManager to return null when there's no UIManager registered Summary: This diff refactors the UIManagerHelper.getUIManager method to return null when there's no UIManager registered for the uiManagerType received as a parameter. This is necessary to workaround: https://github.com/facebook/react-native/issues/31245 changelog: [changed] UIManagerHelper.getUIManager now returns null when there's no UIManager registered for the uiManagerType received as a parameter Reviewed By: fkgozali Differential Revision: D28242592 fbshipit-source-id: c3a4979bcf6e547d0f0060737e41bbf19860a984 --- .../facebook/react/uimanager/UIManagerHelper.java | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java index b2b82aca343f29..d18ecb416627c7 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java @@ -82,9 +82,18 @@ private static UIManager getUIManager( } } CatalystInstance catalystInstance = context.getCatalystInstance(); - return uiManagerType == FABRIC - ? (UIManager) catalystInstance.getJSIModule(JSIModuleType.UIManager) - : catalystInstance.getNativeModule(UIManagerModule.class); + try { + return uiManagerType == FABRIC + ? (UIManager) catalystInstance.getJSIModule(JSIModuleType.UIManager) + : catalystInstance.getNativeModule(UIManagerModule.class); + } catch (IllegalArgumentException ex) { + // TODO T67518514 Clean this up once we migrate everything over to bridgeless mode + ReactSoftException.logSoftException( + "UIManagerHelper", + new ReactNoCrashSoftException( + "Cannot get UIManager for UIManagerType: " + uiManagerType)); + return catalystInstance.getNativeModule(UIManagerModule.class); + } } /** From 570c6f1f295c839c5d83b2fa6b4de3e57f1746c9 Mon Sep 17 00:00:00 2001 From: David Vacca Date: Thu, 6 May 2021 12:12:10 -0700 Subject: [PATCH 03/12] Quick refactor of string tags used in UIManagerHelper Summary: this is a quick refactor of the string tags used in UIManagerHelper changelog: [internal] internal Reviewed By: JoshuaGross Differential Revision: D28243264 fbshipit-source-id: c32c9908d40e6184d7e940b14c9782799db3f891 --- .../react/uimanager/UIManagerHelper.java | 18 +++++++++--------- 1 file changed, 9 insertions(+), 9 deletions(-) diff --git a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java index d18ecb416627c7..4419106085a28d 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/UIManagerHelper.java @@ -29,6 +29,7 @@ /** Helper class for {@link UIManager}. */ public class UIManagerHelper { + private static final String TAG = UIManagerHelper.class.getName(); public static final int PADDING_START_INDEX = 0; public static final int PADDING_END_INDEX = 1; public static final int PADDING_TOP_INDEX = 2; @@ -55,7 +56,7 @@ private static UIManager getUIManager( @Nullable UIManager uiManager = (UIManager) context.getJSIModule(JSIModuleType.UIManager); if (uiManager == null) { ReactSoftException.logSoftException( - "UIManagerHelper", + TAG, new ReactNoCrashSoftException( "Cannot get UIManager because the instance hasn't been initialized yet.")); return null; @@ -65,7 +66,7 @@ private static UIManager getUIManager( if (!context.hasCatalystInstance()) { ReactSoftException.logSoftException( - "UIManagerHelper", + TAG, new ReactNoCrashSoftException( "Cannot get UIManager because the context doesn't contain a CatalystInstance.")); return null; @@ -74,7 +75,7 @@ private static UIManager getUIManager( // down. if (!context.hasActiveReactInstance()) { ReactSoftException.logSoftException( - "UIManagerHelper", + TAG, new ReactNoCrashSoftException( "Cannot get UIManager because the context doesn't contain an active CatalystInstance.")); if (returnNullIfCatalystIsInactive) { @@ -89,7 +90,7 @@ private static UIManager getUIManager( } catch (IllegalArgumentException ex) { // TODO T67518514 Clean this up once we migrate everything over to bridgeless mode ReactSoftException.logSoftException( - "UIManagerHelper", + TAG, new ReactNoCrashSoftException( "Cannot get UIManager for UIManagerType: " + uiManagerType)); return catalystInstance.getNativeModule(UIManagerModule.class); @@ -105,8 +106,7 @@ public static EventDispatcher getEventDispatcherForReactTag(ReactContext context EventDispatcher eventDispatcher = getEventDispatcher(context, getUIManagerType(reactTag)); if (eventDispatcher == null) { ReactSoftException.logSoftException( - "UIManagerHelper", - new IllegalStateException("Cannot get EventDispatcher for reactTag " + reactTag)); + TAG, new IllegalStateException("Cannot get EventDispatcher for reactTag " + reactTag)); } return eventDispatcher; } @@ -128,7 +128,7 @@ public static EventDispatcher getEventDispatcher( UIManager uiManager = getUIManager(context, uiManagerType, false); if (uiManager == null) { ReactSoftException.logSoftException( - "UIManagerHelper", + TAG, new ReactNoCrashSoftException( "Unable to find UIManager for UIManagerType " + uiManagerType)); return null; @@ -136,7 +136,7 @@ public static EventDispatcher getEventDispatcher( EventDispatcher eventDispatcher = (EventDispatcher) uiManager.getEventDispatcher(); if (eventDispatcher == null) { ReactSoftException.logSoftException( - "UIManagerHelper", + TAG, new IllegalStateException( "Cannot get EventDispatcher for UIManagerType " + uiManagerType)); } @@ -181,7 +181,7 @@ public static int getSurfaceId(View view) { // All Fabric-managed Views (should) have a ThemedReactContext attached. if (surfaceId == -1) { ReactSoftException.logSoftException( - "UIManagerHelper", + TAG, new IllegalStateException( "Fabric View [" + reactTag + "] does not have SurfaceId associated with it")); } From 1d924549cad75912191005c8f68dd73e15b07183 Mon Sep 17 00:00:00 2001 From: Adrien HARNAY Date: Thu, 6 May 2021 12:40:27 -0700 Subject: [PATCH 04/12] Add onPressIn & onPressOut props to Text (#31288) Summary: I added onPressIn & onPressOut props to Text to help implement custom highlighting logic (e.g. when clicking on a Text segment). Since TouchableOpacity can't be nested in Text having custom lineHeights without bugs in some occasions, this modification helps to replicate its behavior. ## Changelog [General] [Added] - Add onPressIn & onPressOut props to Text Pull Request resolved: https://github.com/facebook/react-native/pull/31288 Test Plan: ``` const [pressing, setPressing] = useState(false); setPressing(true)} onPressOut={() => setPressing(false)} style={{ opacity: pressing ? 0.5 : 1 }} /> ``` Thanks in advance! Reviewed By: yungsters Differential Revision: D27945133 Pulled By: appden fbshipit-source-id: 8342ca5f75986b4644a193d2f71eab3bc0ef1a5f --- Libraries/Text/Text.js | 6 ++++++ Libraries/Text/TextProps.js | 2 ++ 2 files changed, 8 insertions(+) diff --git a/Libraries/Text/Text.js b/Libraries/Text/Text.js index 1b334d3fda0a70..97ce10d0a2099d 100644 --- a/Libraries/Text/Text.js +++ b/Libraries/Text/Text.js @@ -34,6 +34,8 @@ const Text: React.AbstractComponent< ellipsizeMode, onLongPress, onPress, + onPressIn, + onPressOut, onResponderGrant, onResponderMove, onResponderRelease, @@ -64,9 +66,11 @@ const Text: React.AbstractComponent< onPress, onPressIn(event) { setHighlighted(!suppressHighlighting); + onPressIn?.(event); }, onPressOut(event) { setHighlighted(false); + onPressOut?.(event); }, onResponderTerminationRequest_DEPRECATED: onResponderTerminationRequest, onStartShouldSetResponder_DEPRECATED: onStartShouldSetResponder, @@ -78,6 +82,8 @@ const Text: React.AbstractComponent< pressRetentionOffset, onLongPress, onPress, + onPressIn, + onPressOut, onResponderTerminationRequest, onStartShouldSetResponder, suppressHighlighting, diff --git a/Libraries/Text/TextProps.js b/Libraries/Text/TextProps.js index 9555e576618303..469f14522e3824 100644 --- a/Libraries/Text/TextProps.js +++ b/Libraries/Text/TextProps.js @@ -122,6 +122,8 @@ export type TextProps = $ReadOnly<{| * See https://reactnative.dev/docs/text.html#onpress */ onPress?: ?(event: PressEvent) => mixed, + onPressIn?: ?(event: PressEvent) => mixed, + onPressOut?: ?(event: PressEvent) => mixed, onResponderGrant?: ?(event: PressEvent) => void, onResponderMove?: ?(event: PressEvent) => void, onResponderRelease?: ?(event: PressEvent) => void, From 10a9a4ed89b3d8f059cd92a7d69185094fa5fe57 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Thu, 6 May 2021 19:58:47 -0700 Subject: [PATCH 05/12] Enable block guard for promises Summary: Changelog: Resolve memory leak in promises on iOS Enable block guard for promises Reviewed By: RSNara Differential Revision: D28252977 fbshipit-source-id: 052fefca0a6ac54597a46922fc467a2a39f7976f --- React/Base/RCTBridge.h | 4 ++++ React/Base/RCTBridge.m | 11 ++++++++++ .../core/platform/ios/RCTTurboModule.mm | 21 +++++++++++++++++-- 3 files changed, 34 insertions(+), 2 deletions(-) diff --git a/React/Base/RCTBridge.h b/React/Base/RCTBridge.h index 49b355f273604d..3cf0d8792d014e 100644 --- a/React/Base/RCTBridge.h +++ b/React/Base/RCTBridge.h @@ -164,6 +164,10 @@ RCT_EXTERN void RCTEnableTurboModuleSharedMutexInit(BOOL enabled); RCT_EXTERN BOOL RCTTurboModuleBlockGuardEnabled(void); RCT_EXTERN void RCTEnableTurboModuleBlockGuard(BOOL enabled); +// Turn on TurboModule block guard for promises. +RCT_EXTERN BOOL RCTTurboModulePromisesBlockGuardEnabled(void); +RCT_EXTERN void RCTEnableTurboModulePromisesBlockGuard(BOOL enabled); + /** * Async batched bridge used to communicate with the JavaScript application. */ diff --git a/React/Base/RCTBridge.m b/React/Base/RCTBridge.m index 585aa00c8310d2..2e402b9bcaf4d9 100644 --- a/React/Base/RCTBridge.m +++ b/React/Base/RCTBridge.m @@ -149,6 +149,17 @@ void RCTEnableTurboModuleBlockGuard(BOOL enabled) turboModuleBlockGuardEnabled = enabled; } +static BOOL turboModulePromisesBlockGuardEnabled = NO; +BOOL RCTTurboModulePromisesBlockGuardEnabled(void) +{ + return turboModulePromisesBlockGuardEnabled; +} + +void RCTEnableTurboModulePromisesBlockGuard(BOOL enabled) +{ + turboModulePromisesBlockGuardEnabled = enabled; +} + @interface RCTBridge () @end diff --git a/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm b/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm index b730492b9033d8..1ed7941aea1f98 100644 --- a/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm +++ b/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm @@ -254,6 +254,21 @@ static int32_t getUniqueId() __block BOOL resolveWasCalled = NO; __block BOOL rejectWasCalled = NO; + RCTBlockGuard *blockGuard; + if (RCTTurboModulePromisesBlockGuardEnabled()) { + blockGuard = [[RCTBlockGuard alloc] initWithCleanup:^() { + auto strongResolveWrapper = weakResolveWrapper.lock(); + if (strongResolveWrapper) { + strongResolveWrapper->destroy(); + } + + auto strongRejectWrapper = weakRejectWrapper.lock(); + if (strongRejectWrapper) { + strongRejectWrapper->destroy(); + } + }]; + } + RCTPromiseResolveBlock resolveBlock = ^(id result) { if (rejectWasCalled) { RCTLogError(@"%s: Tried to resolve a promise after it's already been rejected.", moduleMethod.c_str()); @@ -271,7 +286,7 @@ static int32_t getUniqueId() return; } - strongResolveWrapper->jsInvoker().invokeAsync([weakResolveWrapper, weakRejectWrapper, result]() { + strongResolveWrapper->jsInvoker().invokeAsync([weakResolveWrapper, weakRejectWrapper, result, blockGuard]() { auto strongResolveWrapper2 = weakResolveWrapper.lock(); auto strongRejectWrapper2 = weakRejectWrapper.lock(); if (!strongResolveWrapper2 || !strongRejectWrapper2) { @@ -284,6 +299,7 @@ static int32_t getUniqueId() strongResolveWrapper2->destroy(); strongRejectWrapper2->destroy(); + (void)blockGuard; }); resolveWasCalled = YES; @@ -307,7 +323,7 @@ static int32_t getUniqueId() } NSDictionary *jsError = RCTJSErrorFromCodeMessageAndNSError(code, message, error); - strongRejectWrapper->jsInvoker().invokeAsync([weakResolveWrapper, weakRejectWrapper, jsError]() { + strongRejectWrapper->jsInvoker().invokeAsync([weakResolveWrapper, weakRejectWrapper, jsError, blockGuard]() { auto strongResolveWrapper2 = weakResolveWrapper.lock(); auto strongRejectWrapper2 = weakRejectWrapper.lock(); if (!strongResolveWrapper2 || !strongRejectWrapper2) { @@ -320,6 +336,7 @@ static int32_t getUniqueId() strongResolveWrapper2->destroy(); strongRejectWrapper2->destroy(); + (void)blockGuard; }); rejectWasCalled = YES; From e4b6392ee7b060f5780edb8830f46ffd50790598 Mon Sep 17 00:00:00 2001 From: Samuel Susla Date: Thu, 6 May 2021 19:58:47 -0700 Subject: [PATCH 06/12] Remove gating for block guard Summary: Changelog: Remove gating around leak fix in TurboModules infra Reviewed By: RSNara Differential Revision: D28253054 fbshipit-source-id: 9b3c236d3752b5ca042f83127e42e69250ccd112 --- React/Base/RCTBridge.h | 4 ---- React/Base/RCTBridge.m | 11 ----------- .../core/platform/ios/RCTTurboModule.mm | 15 ++++++--------- 3 files changed, 6 insertions(+), 24 deletions(-) diff --git a/React/Base/RCTBridge.h b/React/Base/RCTBridge.h index 3cf0d8792d014e..c87454216db17d 100644 --- a/React/Base/RCTBridge.h +++ b/React/Base/RCTBridge.h @@ -160,10 +160,6 @@ RCT_EXTERN void RCTEnableTurboModuleEagerInit(BOOL enabled); RCT_EXTERN BOOL RCTTurboModuleSharedMutexInitEnabled(void); RCT_EXTERN void RCTEnableTurboModuleSharedMutexInit(BOOL enabled); -// Turn on TurboModule shared mutex initialization -RCT_EXTERN BOOL RCTTurboModuleBlockGuardEnabled(void); -RCT_EXTERN void RCTEnableTurboModuleBlockGuard(BOOL enabled); - // Turn on TurboModule block guard for promises. RCT_EXTERN BOOL RCTTurboModulePromisesBlockGuardEnabled(void); RCT_EXTERN void RCTEnableTurboModulePromisesBlockGuard(BOOL enabled); diff --git a/React/Base/RCTBridge.m b/React/Base/RCTBridge.m index 2e402b9bcaf4d9..441e57b6a14cbc 100644 --- a/React/Base/RCTBridge.m +++ b/React/Base/RCTBridge.m @@ -138,17 +138,6 @@ void RCTEnableTurboModuleSharedMutexInit(BOOL enabled) turboModuleSharedMutexInitEnabled = enabled; } -static BOOL turboModuleBlockGuardEnabled = NO; -BOOL RCTTurboModuleBlockGuardEnabled(void) -{ - return turboModuleBlockGuardEnabled; -} - -void RCTEnableTurboModuleBlockGuard(BOOL enabled) -{ - turboModuleBlockGuardEnabled = enabled; -} - static BOOL turboModulePromisesBlockGuardEnabled = NO; BOOL RCTTurboModulePromisesBlockGuardEnabled(void) { diff --git a/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm b/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm index 1ed7941aea1f98..d3a92a13c26fef 100644 --- a/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm +++ b/ReactCommon/react/nativemodule/core/platform/ios/RCTTurboModule.mm @@ -171,15 +171,12 @@ static int32_t getUniqueId() convertJSIFunctionToCallback(jsi::Runtime &runtime, const jsi::Function &value, std::shared_ptr jsInvoker) { auto weakWrapper = CallbackWrapper::createWeak(value.getFunction(runtime), runtime, jsInvoker); - RCTBlockGuard *blockGuard; - if (RCTTurboModuleBlockGuardEnabled()) { - blockGuard = [[RCTBlockGuard alloc] initWithCleanup:^() { - auto strongWrapper = weakWrapper.lock(); - if (strongWrapper) { - strongWrapper->destroy(); - } - }]; - } + RCTBlockGuard *blockGuard = [[RCTBlockGuard alloc] initWithCleanup:^() { + auto strongWrapper = weakWrapper.lock(); + if (strongWrapper) { + strongWrapper->destroy(); + } + }]; BOOL __block wrapperWasCalled = NO; RCTResponseSenderBlock callback = ^(NSArray *responses) { From f2157a05588708275b6fff9014b05caec80c7787 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Thu, 6 May 2021 21:22:44 -0700 Subject: [PATCH 07/12] Migrate RCTDevSettings to RCTBundleManager Summary: This diff migrated RCTDevSettings to the RCTBundleManager, which works both in bridge/bridgeless mode. RCTDevSettings is the last TurboModule that synthesizes the bundleURL. So, after this diff, we can get rid of the RCTBundleHolderModule protocol. Changelog: [Internal] Reviewed By: PeteTheHeat Differential Revision: D28232320 fbshipit-source-id: ab53278fea0ea7e025cf748be62bc5d610593e7f --- React/CoreModules/RCTDevSettings.mm | 63 +++++++++++++---------------- 1 file changed, 27 insertions(+), 36 deletions(-) diff --git a/React/CoreModules/RCTDevSettings.mm b/React/CoreModules/RCTDevSettings.mm index cc8f740cce79d8..0f6244f76aab19 100644 --- a/React/CoreModules/RCTDevSettings.mm +++ b/React/CoreModules/RCTDevSettings.mm @@ -12,7 +12,6 @@ #import #import #import -#import #import #import #import @@ -114,12 +113,7 @@ - (void)_reloadWithDefaults:(NSDictionary *)defaultValues @end -@interface RCTDevSettings () < - RCTBridgeModule, - RCTInvalidating, - NativeDevSettingsSpec, - RCTBundleHolderModule, - RCTDevSettingsInspectable> { +@interface RCTDevSettings () { BOOL _isJSLoaded; #if ENABLE_PACKAGER_CONNECTION RCTHandlerToken _reloadToken; @@ -133,8 +127,8 @@ @interface RCTDevSettings () < @implementation RCTDevSettings -@synthesize bundleURL = _bundleURL; @synthesize isInspectable = _isInspectable; +@synthesize bundleManager = _bundleManager; RCT_EXPORT_MODULE() @@ -172,16 +166,32 @@ - (instancetype)initWithDataSource:(id)dataSource return self; } -#if RCT_ENABLE_INSPECTOR -// In bridgeless mode, `setBridge` is not called, so dev server connection -// must be kicked off here. -- (void)setBundleURL:(NSURL *)bundleURL +- (void)setBundleManager:(RCTBundleManager *)bundleManager { - _bundleURL = bundleURL; - [RCTInspectorDevServerHelper connectWithBundleURL:_bundleURL]; -} + _bundleManager = bundleManager; + +#if RCT_ENABLE_INSPECTOR + if (self.bridge) { + // We need this dispatch to the main thread because the bridge is not yet + // finished with its initialisation. By the time it relinquishes control of + // the main thread, this operation can be performed. + dispatch_async(dispatch_get_main_queue(), ^{ + [self.bridge + dispatchBlock:^{ + [RCTInspectorDevServerHelper connectWithBundleURL:bundleManager.bundleURL]; + } + queue:RCTJSThread]; + }); + } else { + [RCTInspectorDevServerHelper connectWithBundleURL:bundleManager.bundleURL]; + } #endif + dispatch_async(dispatch_get_main_queue(), ^{ + [self _synchronizeAllSettings]; + }); +} + - (void)setBridge:(RCTBridge *)bridge { [super setBridge:bridge]; @@ -198,23 +208,6 @@ - (void)setBridge:(RCTBridge *)bridge queue:dispatch_get_main_queue() forMethod:@"reload"]; #endif - -#if RCT_ENABLE_INSPECTOR - // We need this dispatch to the main thread because the bridge is not yet - // finished with its initialisation. By the time it relinquishes control of - // the main thread, this operation can be performed. - dispatch_async(dispatch_get_main_queue(), ^{ - [bridge - dispatchBlock:^{ - [RCTInspectorDevServerHelper connectWithBundleURL:bridge.bundleURL]; - } - queue:RCTJSThread]; - }); -#endif - - dispatch_async(dispatch_get_main_queue(), ^{ - [self _synchronizeAllSettings]; - }); } - (dispatch_queue_t)methodQueue @@ -269,10 +262,8 @@ - (BOOL)isRemoteDebuggingAvailable - (BOOL)isHotLoadingAvailable { - if (self.bridge.bundleURL) { - return !self.bridge.bundleURL.fileURL; // Only works when running from server - } else if (self.bundleURL) { - return !self.bundleURL.fileURL; + if (self.bundleManager.bundleURL) { + return !self.bundleManager.bundleURL.fileURL; } return NO; } From c17674e12f0b3efcdb1caa2d65b73ac81ab12f06 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Thu, 6 May 2021 21:22:44 -0700 Subject: [PATCH 08/12] Delete RCTBundleHolderModule Summary: This protocol is no longer necessary, because we migrated all our NativeModules to synthesize bundleManager = _bundleManager; Changelog: [Internal] Reviewed By: fkgozali Differential Revision: D28232441 fbshipit-source-id: 0bd54fa49299574db8a75247c97b466476037f4f --- React/Base/RCTBundleHolderModule.h | 16 ---------------- 1 file changed, 16 deletions(-) delete mode 100644 React/Base/RCTBundleHolderModule.h diff --git a/React/Base/RCTBundleHolderModule.h b/React/Base/RCTBundleHolderModule.h deleted file mode 100644 index b06d5ee5ad6955..00000000000000 --- a/React/Base/RCTBundleHolderModule.h +++ /dev/null @@ -1,16 +0,0 @@ -/* - * Copyright (c) Facebook, Inc. and its affiliates. - * - * This source code is licensed under the MIT license found in the - * LICENSE file in the root directory of this source tree. - */ - -/** - * This protocol should be adopted when a turbo module needs to access the currently loaded JS bundle URL. - * In bridge-less React Native, it is a replacement for bridge.bundleURL. - */ -@protocol RCTBundleHolderModule - -@property (nonatomic, strong, readwrite) NSURL *bundleURL; - -@end From 1e39d8b82985849de15a8a744cefddb657bb5861 Mon Sep 17 00:00:00 2001 From: Ramanpreet Nara Date: Thu, 6 May 2021 21:22:44 -0700 Subject: [PATCH 09/12] Delete RCTJSInvokerModule setInvokeJSWithModuleDotMethod: Summary: RCTJSInvokerModule invokeJS can do the work done by setInvokeJSWithModuleDotMethod. Therefore, this diff reduces the surface area of Venice, and gets rid of setInvokeJSWithModuleDotMethod. Changelog: [Internal] Reviewed By: PeteTheHeat Differential Revision: D28232947 fbshipit-source-id: aa0d8497a1e0fba29109ca86a39de5d9e5b10c9c --- React/Base/RCTJSInvokerModule.h | 1 - React/CoreModules/RCTEventDispatcher.mm | 9 ++++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/React/Base/RCTJSInvokerModule.h b/React/Base/RCTJSInvokerModule.h index 59e79ceabb563f..cc293177f1e04e 100644 --- a/React/Base/RCTJSInvokerModule.h +++ b/React/Base/RCTJSInvokerModule.h @@ -13,6 +13,5 @@ @optional @property (nonatomic, copy) void (^invokeJS)(NSString *module, NSString *method, NSArray *args); -@property (nonatomic, copy) void (^invokeJSWithModuleDotMethod)(NSString *moduleDotMethod, NSArray *args); @end diff --git a/React/CoreModules/RCTEventDispatcher.mm b/React/CoreModules/RCTEventDispatcher.mm index 1edb079a9bfcd7..62ab713ff26434 100644 --- a/React/CoreModules/RCTEventDispatcher.mm +++ b/React/CoreModules/RCTEventDispatcher.mm @@ -43,7 +43,6 @@ @implementation RCTEventDispatcher { @synthesize bridge = _bridge; @synthesize dispatchToJSThread = _dispatchToJSThread; @synthesize invokeJS = _invokeJS; -@synthesize invokeJSWithModuleDotMethod = _invokeJSWithModuleDotMethod; RCT_EXPORT_MODULE() @@ -198,8 +197,12 @@ - (void)dispatchEvent:(id)event { if (_bridge) { [_bridge enqueueJSCall:[[event class] moduleDotMethod] args:[event arguments]]; - } else if (_invokeJSWithModuleDotMethod) { - _invokeJSWithModuleDotMethod([[event class] moduleDotMethod], [event arguments]); + } else if (_invokeJS) { + NSString *moduleDotMethod = [[event class] moduleDotMethod]; + NSArray *const components = [moduleDotMethod componentsSeparatedByString:@"."]; + NSString *const moduleName = components[0]; + NSString *const methodName = components[1]; + _invokeJS(moduleName, methodName, [event arguments]); } } From bf8037cad0e2893017157abffc49787e0227181b Mon Sep 17 00:00:00 2001 From: David Vacca Date: Fri, 7 May 2021 20:39:25 -0700 Subject: [PATCH 10/12] Remove ReactFeatureFlags.useViewManagerDelegates Summary: This diff deleted the ReactFeatureFlags.useViewManagerDelegates, this has been enabled for 9+ months changelog: [internal] internal Reviewed By: JoshuaGross Differential Revision: D28265339 fbshipit-source-id: f5c97e77ca4fc72d2e2b8f891e800e362177d67a --- .../java/com/facebook/react/config/ReactFeatureFlags.java | 7 ------- .../java/com/facebook/react/uimanager/ViewManager.java | 5 ++--- 2 files changed, 2 insertions(+), 10 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 16619055908782..a2d04534a75fcc 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java +++ b/ReactAndroid/src/main/java/com/facebook/react/config/ReactFeatureFlags.java @@ -38,13 +38,6 @@ public class ReactFeatureFlags { */ public static boolean enableFabricLogs = false; - /** - * Should this application use a {@link com.facebook.react.uimanager.ViewManagerDelegate} (if - * provided) to update the view properties. If {@code false}, then the generated {@code - * ...$$PropsSetter} class will be used instead. - */ - 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 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 ccde14ae9c21ed..6af66a6f080174 100644 --- a/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java +++ b/ReactAndroid/src/main/java/com/facebook/react/uimanager/ViewManager.java @@ -15,7 +15,6 @@ import com.facebook.react.bridge.ReactApplicationContext; import com.facebook.react.bridge.ReadableArray; import com.facebook.react.bridge.ReadableMap; -import com.facebook.react.config.ReactFeatureFlags; import com.facebook.react.touch.JSResponderHandler; import com.facebook.react.touch.ReactInterceptingViewGroup; import com.facebook.react.uimanager.annotations.ReactProp; @@ -41,8 +40,8 @@ public abstract class ViewManager * @param props */ public void updateProperties(@NonNull T viewToUpdate, ReactStylesDiffMap props) { - final ViewManagerDelegate delegate; - if (ReactFeatureFlags.useViewManagerDelegates && (delegate = getDelegate()) != null) { + final ViewManagerDelegate delegate = getDelegate(); + if (delegate != null) { ViewManagerPropertyUpdater.updateProps(delegate, viewToUpdate, props); } else { ViewManagerPropertyUpdater.updateProps(this, viewToUpdate, props); From ae4946f98369141caa52fd09d63400d4a28e1d1f Mon Sep 17 00:00:00 2001 From: Tim Yung Date: Fri, 7 May 2021 21:04:47 -0700 Subject: [PATCH 11/12] Animated: Remove `Animated.__PropsOnlyForTests` Summary: Deletes `Animated.__PropsOnlyForTests` because it is unnecessary for actually testing `AnimatedProps`. Changelog: [General][Removed] - Removed `Animated.__PropsOnlyForTests`. Reviewed By: kacieb Differential Revision: D28271629 fbshipit-source-id: 7d4c83d72f7298ed43e3659126cb45d271c5dac7 --- Libraries/Animated/AnimatedImplementation.js | 2 -- Libraries/Animated/AnimatedMock.js | 1 - Libraries/Animated/__tests__/Animated-test.js | 8 +++++--- 3 files changed, 5 insertions(+), 6 deletions(-) diff --git a/Libraries/Animated/AnimatedImplementation.js b/Libraries/Animated/AnimatedImplementation.js index 12b225abab0616..706b87ea01e9e1 100644 --- a/Libraries/Animated/AnimatedImplementation.js +++ b/Libraries/Animated/AnimatedImplementation.js @@ -706,6 +706,4 @@ module.exports = { * Expose Event class, so it can be used as a type for type checkers. */ Event: AnimatedEvent, - - __PropsOnlyForTests: AnimatedProps, }; diff --git a/Libraries/Animated/AnimatedMock.js b/Libraries/Animated/AnimatedMock.js index eecfbb884ec42b..50e69439a50453 100644 --- a/Libraries/Animated/AnimatedMock.js +++ b/Libraries/Animated/AnimatedMock.js @@ -152,5 +152,4 @@ module.exports = { forkEvent: AnimatedImplementation.forkEvent, unforkEvent: AnimatedImplementation.unforkEvent, Event: AnimatedEvent, - __PropsOnlyForTests: AnimatedProps, }; diff --git a/Libraries/Animated/__tests__/Animated-test.js b/Libraries/Animated/__tests__/Animated-test.js index f7a8bb05ace086..e24a4e5620fa7c 100644 --- a/Libraries/Animated/__tests__/Animated-test.js +++ b/Libraries/Animated/__tests__/Animated-test.js @@ -8,6 +8,7 @@ * @emails oncall+react_native */ +import AnimatedProps from '../nodes/AnimatedProps'; import TestRenderer from 'react-test-renderer'; import * as React from 'react'; @@ -21,6 +22,7 @@ jest.mock('../../BatchedBridge/NativeModules', () => ({ })); let Animated = require('../Animated'); + describe('Animated tests', () => { beforeEach(() => { jest.resetModules(); @@ -32,7 +34,7 @@ describe('Animated tests', () => { const callback = jest.fn(); - const node = new Animated.__PropsOnlyForTests( + const node = new AnimatedProps( { style: { backgroundColor: 'red', @@ -786,7 +788,7 @@ describe('Animated tests', () => { const callback = jest.fn(); - const node = new Animated.__PropsOnlyForTests( + const node = new AnimatedProps( { style: { opacity: vec.x.interpolate({ @@ -890,7 +892,7 @@ describe('Animated tests', () => { const value3 = new Animated.Value(0); const value4 = Animated.add(value3, Animated.multiply(value1, value2)); const callback = jest.fn(); - const view = new Animated.__PropsOnlyForTests( + const view = new AnimatedProps( { style: { transform: [ From d87542ee4c241b3efc462708739f4593c7cf02e8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Rub=C3=A9n=20Norte?= Date: Mon, 10 May 2021 07:20:12 -0700 Subject: [PATCH 12/12] Create usePerformanceLogger hook Summary: Having to import `useContext` every time is annoying, so this just creates a convenience function to get it. Changelog: [Internal] Reviewed By: GijsWeterings Differential Revision: D28258305 fbshipit-source-id: 7293478f9baa11711a541f987225108871688e0e --- Libraries/Utilities/PerformanceLoggerContext.js | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/Libraries/Utilities/PerformanceLoggerContext.js b/Libraries/Utilities/PerformanceLoggerContext.js index 6e58752ccec1fb..f6977f6f2bc561 100644 --- a/Libraries/Utilities/PerformanceLoggerContext.js +++ b/Libraries/Utilities/PerformanceLoggerContext.js @@ -9,6 +9,7 @@ */ import * as React from 'react'; +import {useContext} from 'react'; import GlobalPerformanceLogger from './GlobalPerformanceLogger'; import type {IPerformanceLogger} from './createPerformanceLogger'; @@ -24,4 +25,9 @@ const PerformanceLoggerContext: React.Context = React.create if (__DEV__) { PerformanceLoggerContext.displayName = 'PerformanceLoggerContext'; } -module.exports = PerformanceLoggerContext; + +export function usePerformanceLogger(): IPerformanceLogger { + return useContext(PerformanceLoggerContext); +} + +export default PerformanceLoggerContext;