Skip to content

Commit 39d6773

Browse files
RSNarafacebook-github-bot
authored andcommittedJun 3, 2020
Run getConstants method statements on main queue
Summary: If a NativeModule requires main queue setup, its `getConstants()` method must be executed on the main thead. The legacy NativeModule infra takes care of this for us. With TurboModules, however, all synchronous methods, including `getConstants()`, execute on the JS thread. Therefore, if a TurboModule requires main queue setup, and exports constants, we must execute its `getConstants()` method body on the main queue explicitly. **Notes:** - The changes in this diff should be a noop when TurboModules is off, because `RCTUnsafeExecuteOnMainQueueSync` synchronously execute its block on the current thread if the current thread is the main thread. - If a NativeModule doens't have the `requiresMainQueueSetup` method, but has the `constantsToExport` method, both NativeModules and TurboModules assume that it requires main queue setup. ## Script ``` const exec = require("../lib/exec"); const abspath = require("../lib/abspath"); const relpath = require("../lib/relpath"); const readFile = (filename) => require("fs").readFileSync(filename, "utf8"); const writeFile = (filename, content) => require("fs").writeFileSync(filename, content); function main() { const tmFiles = exec("cd ~/fbsource && xbgs -n 10000 -l constantsToExport") .split("\n") .filter(Boolean); const filesWithoutConstantsToExport = []; const filesWithConstantsToExportButNotGetConstants = []; const filesExplicitlyNotRequiringMainQueueSetup = []; tmFiles .filter((filename) => { if (filename.includes("microsoft-fork-of-react-native")) { return false; } return /\.mm?$/.test(filename); }) .map(abspath) .forEach((filename) => { const code = readFile(filename); const relFilename = relpath(filename); if (!/constantsToExport\s*{/.test(code)) { filesWithoutConstantsToExport.push(relFilename); return; } if (!/getConstants\s*{/.test(code)) { filesWithConstantsToExportButNotGetConstants.push(relFilename); return; } if (/requiresMainQueueSetup\s*{/.test(code)) { const requiresMainQueueSetupRegex = /requiresMainQueueSetup\s*{\s*return\s+(?<requiresMainQueueSetup>YES|NO)/; const requiresMainQueueSetupRegexMatch = requiresMainQueueSetupRegex.exec( code ); if (!requiresMainQueueSetupRegexMatch) { throw new Error( "Detected requiresMainQueueSetup method in file " + relFilename + " but was unable to parse the method return value" ); } const { requiresMainQueueSetup, } = requiresMainQueueSetupRegexMatch.groups; if (requiresMainQueueSetup == "NO") { filesExplicitlyNotRequiringMainQueueSetup.push(relFilename); return; } } const getConstantsTypeRegex = () => /-\s*\((?<type>.*)\)getConstants\s*{/; const getConstantsTypeRegexMatch = getConstantsTypeRegex().exec(code); if (!getConstantsTypeRegexMatch) { throw new Error( `Failed to parse return type of getConstants method in file ${relFilename}` ); } const getConstantsType = getConstantsTypeRegexMatch.groups.type; const getConstantsBody = code .split(getConstantsTypeRegex())[2] .split("\n}")[0]; const newGetConstantsBody = ` __block ${getConstantsType} constants; RCTUnsafeExecuteOnMainQueueSync(^{${getConstantsBody .replace(/\n/g, "\n ") .replace(/_bridge/g, "self->_bridge") .replace(/return /g, "constants = ")} }); return constants; `; writeFile( filename, code .replace(getConstantsBody, newGetConstantsBody) .replace("#import", "#import <React/RCTUtils.h>\n#import") ); }); console.log("Files without constantsToExport: "); filesWithoutConstantsToExport.forEach((file) => console.log(file)); console.log(); console.log("Files with constantsToExport but no getConstants: "); filesWithConstantsToExportButNotGetConstants.forEach((file) => console.log(file) ); console.log(); console.log("Files with requiresMainQueueSetup = NO: "); filesExplicitlyNotRequiringMainQueueSetup.forEach((file) => console.log(file) ); } if (!module.parent) { main(); } ``` Changelog: [Internal] Reviewed By: fkgozali Differential Revision: D21797048 fbshipit-source-id: a822a858fecdbe976e6197f8339e509dc7cd917f
1 parent e5bef73 commit 39d6773

File tree

5 files changed

+61
-35
lines changed

5 files changed

+61
-35
lines changed
 

‎React/CoreModules/RCTAppState.mm

+7-2
Original file line numberDiff line numberDiff line change
@@ -56,9 +56,14 @@ - (dispatch_queue_t)methodQueue
5656

5757
- (facebook::react::ModuleConstants<JS::NativeAppState::Constants>)getConstants
5858
{
59-
return facebook::react::typedConstants<JS::NativeAppState::Constants>({
60-
.initialAppState = RCTCurrentAppState(),
59+
__block facebook::react::ModuleConstants<JS::NativeAppState::Constants> constants;
60+
RCTUnsafeExecuteOnMainQueueSync(^{
61+
constants = facebook::react::typedConstants<JS::NativeAppState::Constants>({
62+
.initialAppState = RCTCurrentAppState(),
63+
});
6164
});
65+
66+
return constants;
6267
}
6368

6469
#pragma mark - Lifecycle

‎React/CoreModules/RCTDeviceInfo.mm

+13-8
Original file line numberDiff line numberDiff line change
@@ -122,14 +122,19 @@ static BOOL RCTIsIPhoneX()
122122

123123
- (NSDictionary<NSString *, id> *)getConstants
124124
{
125-
return @{
126-
@"Dimensions" : RCTExportedDimensions(_bridge),
127-
// Note:
128-
// This prop is deprecated and will be removed in a future release.
129-
// Please use this only for a quick and temporary solution.
130-
// Use <SafeAreaView> instead.
131-
@"isIPhoneX_deprecated" : @(RCTIsIPhoneX()),
132-
};
125+
__block NSDictionary<NSString *, id> *constants;
126+
RCTUnsafeExecuteOnMainQueueSync(^{
127+
constants = @{
128+
@"Dimensions" : RCTExportedDimensions(self->_bridge),
129+
// Note:
130+
// This prop is deprecated and will be removed in a future release.
131+
// Please use this only for a quick and temporary solution.
132+
// Use <SafeAreaView> instead.
133+
@"isIPhoneX_deprecated" : @(RCTIsIPhoneX()),
134+
};
135+
});
136+
137+
return constants;
133138
}
134139

135140
- (void)didReceiveNewContentSizeMultiplier

‎React/CoreModules/RCTPlatform.mm

+20-15
Original file line numberDiff line numberDiff line change
@@ -58,22 +58,27 @@ - (dispatch_queue_t)methodQueue
5858

5959
- (ModuleConstants<JS::NativePlatformConstantsIOS::Constants>)getConstants
6060
{
61-
UIDevice *device = [UIDevice currentDevice];
62-
auto versions = RCTGetReactNativeVersion();
63-
return typedConstants<JS::NativePlatformConstantsIOS::Constants>({
64-
.forceTouchAvailable = RCTForceTouchAvailable() ? true : false,
65-
.osVersion = [device systemVersion],
66-
.systemName = [device systemName],
67-
.interfaceIdiom = interfaceIdiom([device userInterfaceIdiom]),
68-
.isTesting = RCTRunningInTestEnvironment() ? true : false,
69-
.reactNativeVersion = JS::NativePlatformConstantsIOS::ConstantsReactNativeVersion::Builder(
70-
{.minor = [versions[@"minor"] doubleValue],
71-
.major = [versions[@"major"] doubleValue],
72-
.patch = [versions[@"patch"] doubleValue],
73-
.prerelease = [versions[@"prerelease"] isKindOfClass:[NSNull class]]
74-
? folly::Optional<double>{}
75-
: [versions[@"prerelease"] doubleValue]}),
61+
__block ModuleConstants<JS::NativePlatformConstantsIOS::Constants> constants;
62+
RCTUnsafeExecuteOnMainQueueSync(^{
63+
UIDevice *device = [UIDevice currentDevice];
64+
auto versions = RCTGetReactNativeVersion();
65+
constants = typedConstants<JS::NativePlatformConstantsIOS::Constants>({
66+
.forceTouchAvailable = RCTForceTouchAvailable() ? true : false,
67+
.osVersion = [device systemVersion],
68+
.systemName = [device systemName],
69+
.interfaceIdiom = interfaceIdiom([device userInterfaceIdiom]),
70+
.isTesting = RCTRunningInTestEnvironment() ? true : false,
71+
.reactNativeVersion = JS::NativePlatformConstantsIOS::ConstantsReactNativeVersion::Builder(
72+
{.minor = [versions[@"minor"] doubleValue],
73+
.major = [versions[@"major"] doubleValue],
74+
.patch = [versions[@"patch"] doubleValue],
75+
.prerelease = [versions[@"prerelease"] isKindOfClass:[NSNull class]]
76+
? folly::Optional<double>{}
77+
: [versions[@"prerelease"] doubleValue]}),
78+
});
7679
});
80+
81+
return constants;
7782
}
7883

7984
- (std::shared_ptr<TurboModule>)getTurboModule:(const ObjCTurboModule::InitParams &)params

‎React/CoreModules/RCTStatusBarManager.mm

+8-3
Original file line numberDiff line numberDiff line change
@@ -185,10 +185,15 @@ - (void)applicationWillChangeStatusBarFrame:(NSNotification *)notification
185185

186186
- (facebook::react::ModuleConstants<JS::NativeStatusBarManagerIOS::Constants>)getConstants
187187
{
188-
return facebook::react::typedConstants<JS::NativeStatusBarManagerIOS::Constants>({
189-
.HEIGHT = RCTSharedApplication().statusBarFrame.size.height,
190-
.DEFAULT_BACKGROUND_COLOR = folly::none,
188+
__block facebook::react::ModuleConstants<JS::NativeStatusBarManagerIOS::Constants> constants;
189+
RCTUnsafeExecuteOnMainQueueSync(^{
190+
constants = facebook::react::typedConstants<JS::NativeStatusBarManagerIOS::Constants>({
191+
.HEIGHT = RCTSharedApplication().statusBarFrame.size.height,
192+
.DEFAULT_BACKGROUND_COLOR = folly::none,
193+
});
191194
});
195+
196+
return constants;
192197
}
193198

194199
- (facebook::react::ModuleConstants<JS::NativeStatusBarManagerIOS::Constants>)constantsToExport

‎ReactCommon/turbomodule/samples/platform/ios/RCTSampleTurboModule.mm

+13-7
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@
77

88
#import "RCTSampleTurboModule.h"
99

10+
#import <React/RCTUtils.h>
1011
#import <UIKit/UIKit.h>
1112

1213
using namespace facebook::react;
@@ -45,14 +46,19 @@ - (void)invalidate
4546

4647
- (NSDictionary *)getConstants
4748
{
48-
UIScreen *mainScreen = UIScreen.mainScreen;
49-
CGSize screenSize = mainScreen.bounds.size;
49+
__block NSDictionary *constants;
50+
RCTUnsafeExecuteOnMainQueueSync(^{
51+
UIScreen *mainScreen = UIScreen.mainScreen;
52+
CGSize screenSize = mainScreen.bounds.size;
5053

51-
return @{
52-
@"const1" : @YES,
53-
@"const2" : @(screenSize.width),
54-
@"const3" : @"something",
55-
};
54+
constants = @{
55+
@"const1" : @YES,
56+
@"const2" : @(screenSize.width),
57+
@"const3" : @"something",
58+
};
59+
});
60+
61+
return constants;
5662
}
5763

5864
// TODO: Remove once fully migrated to TurboModule.

0 commit comments

Comments
 (0)
Please sign in to comment.