From 3155ddf2e8feb18c6ae9c6e872c9789d052bb19c Mon Sep 17 00:00:00 2001 From: Spencer Ahrens Date: Mon, 8 Apr 2019 09:10:44 -0700 Subject: [PATCH] Support callbacks in synchronous native functions Summary: We need to move animated native module calls to synchronous so we can properly thread them in with mounting instructions in Fabric. Some of them take callbacks so we need to add support for that when switching to synchronous. A side benefit is that we can unify codepaths a little more with async callbacks. Reviewed By: shergin Differential Revision: D14790898 fbshipit-source-id: dc222b9e74375e046e8a9b1b19d72f652dc6722c --- IntegrationTests/SyncMethodTest.js | 20 ++++++-- Libraries/BatchedBridge/MessageQueue.js | 50 +++++++++++++++---- Libraries/BatchedBridge/NativeModules.js | 37 +++++++------- .../RNTesterTestModule.m | 6 +++ 4 files changed, 79 insertions(+), 34 deletions(-) diff --git a/IntegrationTests/SyncMethodTest.js b/IntegrationTests/SyncMethodTest.js index d618ced47..8dbb5b94a 100644 --- a/IntegrationTests/SyncMethodTest.js +++ b/IntegrationTests/SyncMethodTest.js @@ -21,12 +21,26 @@ class SyncMethodTest extends React.Component<{}> { if ( RNTesterTestModule.echoString('test string value') !== 'test string value' ) { - throw new Error('Something wrong with sync method export'); + throw new Error('Something wrong with echoString sync method'); } if (RNTesterTestModule.methodThatReturnsNil() != null) { - throw new Error('Something wrong with sync method export'); + throw new Error('Something wrong with methodThatReturnsNil sync method'); } - TestModule.markTestCompleted(); + let response; + RNTesterTestModule.methodThatCallsCallbackWithString('test', echo => { + response = echo; + }); + requestAnimationFrame(() => { + if (response === 'test') { + TestModule.markTestCompleted(); + } else { + throw new Error( + 'Something wrong with methodThatCallsCallbackWithString sync method, ' + + 'got response ' + + JSON.stringify(response), + ); + } + }); } render(): React.Node { diff --git a/Libraries/BatchedBridge/MessageQueue.js b/Libraries/BatchedBridge/MessageQueue.js index 2b0d20fc2..1d49120b7 100644 --- a/Libraries/BatchedBridge/MessageQueue.js +++ b/Libraries/BatchedBridge/MessageQueue.js @@ -165,7 +165,27 @@ class MessageQueue { return getValue ? getValue() : null; } - enqueueNativeCall( + callNativeSyncHook( + moduleID: number, + methodID: number, + params: any[], + onFail: ?Function, + onSucc: ?Function, + ) { + if (__DEV__) { + invariant( + global.nativeCallSyncHook, + 'Calling synchronous methods on native ' + + 'modules is not supported in Chrome.\n\n Consider providing alternative ' + + 'methods to expose this method in debug mode, e.g. by exposing constants ' + + 'ahead-of-time.', + ); + } + this.processCallbacks(moduleID, methodID, params, onFail, onSucc); + return global.nativeCallSyncHook(moduleID, methodID, params); + } + + processCallbacks( moduleID: number, methodID: number, params: any[], @@ -188,7 +208,6 @@ class MessageQueue { this._successCallbacks[this._callID] = onSucc; this._failureCallbacks[this._callID] = onFail; } - if (__DEV__) { global.nativeTraceBeginAsyncFlow && global.nativeTraceBeginAsyncFlow( @@ -198,6 +217,16 @@ class MessageQueue { ); } this._callID++; + } + + enqueueNativeCall( + moduleID: number, + methodID: number, + params: any[], + onFail: ?Function, + onSucc: ?Function, + ) { + this.processCallbacks(moduleID, methodID, params, onFail, onSucc); this._queue[MODULE_IDS].push(moduleID); this._queue[METHOD_IDS].push(methodID); @@ -385,15 +414,14 @@ class MessageQueue { const debug = this._debugInfo[callID]; const module = debug && this._remoteModuleTable[debug[0]]; const method = debug && this._remoteMethodTable[debug[0]][debug[1]]; - if (!callback) { - let errorMessage = `Callback with id ${cbID}: ${module}.${method}() not found`; - if (method) { - errorMessage = - `The callback ${method}() exists in module ${module}, ` + - 'but only one callback may be registered to a function in a native module.'; - } - invariant(callback, errorMessage); - } + invariant( + callback, + `No callback found with cbID ${cbID} and callID ${callID} for ` + + (method + ? ` ${module}.${method} - most likely the callback was already invoked` + : `module ${module || ''}`) + + `. Args: '${stringifySafe(args)}'`, + ); const profileName = debug ? '' : cbID; diff --git a/Libraries/BatchedBridge/NativeModules.js b/Libraries/BatchedBridge/NativeModules.js index 27db8b9ad..c8487e888 100644 --- a/Libraries/BatchedBridge/NativeModules.js +++ b/Libraries/BatchedBridge/NativeModules.js @@ -105,19 +105,6 @@ function genMethod(moduleID: number, methodID: number, type: MethodType) { ); }); }; - } else if (type === 'sync') { - fn = function(...args: Array) { - if (__DEV__) { - invariant( - global.nativeCallSyncHook, - 'Calling synchronous methods on native ' + - 'modules is not supported in Chrome.\n\n Consider providing alternative ' + - 'methods to expose this method in debug mode, e.g. by exposing constants ' + - 'ahead-of-time.', - ); - } - return global.nativeCallSyncHook(moduleID, methodID, args); - }; } else { fn = function(...args: Array) { const lastArg = args.length > 0 ? args[args.length - 1] : null; @@ -133,13 +120,23 @@ function genMethod(moduleID: number, methodID: number, type: MethodType) { const onFail = hasErrorCallback ? secondLastArg : null; const callbackCount = hasSuccessCallback + hasErrorCallback; args = args.slice(0, args.length - callbackCount); - BatchedBridge.enqueueNativeCall( - moduleID, - methodID, - args, - onFail, - onSuccess, - ); + if (type === 'sync') { + return BatchedBridge.callNativeSyncHook( + moduleID, + methodID, + args, + onFail, + onSuccess, + ); + } else { + BatchedBridge.enqueueNativeCall( + moduleID, + methodID, + args, + onFail, + onSuccess, + ); + } }; } fn.type = type; diff --git a/RNTester/RNTesterIntegrationTests/RNTesterTestModule.m b/RNTester/RNTesterIntegrationTests/RNTesterTestModule.m index d5a4346c4..db62c8985 100644 --- a/RNTester/RNTesterIntegrationTests/RNTesterTestModule.m +++ b/RNTester/RNTesterIntegrationTests/RNTesterTestModule.m @@ -27,4 +27,10 @@ RCT_EXPORT_BLOCKING_SYNCHRONOUS_METHOD(methodThatReturnsNil) return nil; } +RCT_EXPORT_BLOCKING_SYNCHRONOUS_METHOD(methodThatCallsCallbackWithString:(NSString *)input callback:(RCTResponseSenderBlock)callback) +{ + callback(@[input]); + return nil; +} + @end