Permalink
Browse files

Add support for async bridged methods to android

Differential Revision: D2595414

fb-gh-sync-id: 3b44ce1737bdd1e0861a285a45976631a57ab3b5
  • Loading branch information...
davidaurelio authored and facebook-github-bot-4 committed Oct 29, 2015
1 parent b73bf16 commit b86a6e3b44a63e92cf3a7976d2fa26c4bf412df1
@@ -11,6 +11,7 @@
import com.fasterxml.jackson.core.JsonGenerator;
+import com.facebook.infer.annotation.Assertions;
import com.facebook.systrace.Systrace;
import javax.annotation.Nullable;
@@ -46,8 +47,16 @@
* with the same name.
*/
public abstract class BaseJavaModule implements NativeModule {
- private interface ArgumentExtractor {
- @Nullable Object extractArgument(
+ // taken from Libraries/Utilities/MessageQueue.js
+ static final public String METHOD_TYPE_REMOTE = "remote";
+ static final public String METHOD_TYPE_REMOTE_ASYNC = "remoteAsync";
+
+ private static abstract class ArgumentExtractor {
+ public int getJSArgumentsNeeded() {
+ return 1;
+ }
+
+ public abstract @Nullable Object extractArgument(
CatalystInstance catalystInstance, ReadableNativeArray jsArguments, int atIndex);
}
@@ -129,67 +138,110 @@ public Object extractArgument(
}
};
- private static ArgumentExtractor[] buildArgumentExtractors(Class[] parameterTypes) {
- ArgumentExtractor[] argumentExtractors = new ArgumentExtractor[parameterTypes.length];
- for (int i = 0; i < parameterTypes.length; i++) {
- Class argumentClass = parameterTypes[i];
- if (argumentClass == Boolean.class || argumentClass == boolean.class) {
- argumentExtractors[i] = ARGUMENT_EXTRACTOR_BOOLEAN;
- } else if (argumentClass == Integer.class || argumentClass == int.class) {
- argumentExtractors[i] = ARGUMENT_EXTRACTOR_INTEGER;
- } else if (argumentClass == Double.class || argumentClass == double.class) {
- argumentExtractors[i] = ARGUMENT_EXTRACTOR_DOUBLE;
- } else if (argumentClass == Float.class || argumentClass == float.class) {
- argumentExtractors[i] = ARGUMENT_EXTRACTOR_FLOAT;
- } else if (argumentClass == String.class) {
- argumentExtractors[i] = ARGUMENT_EXTRACTOR_STRING;
- } else if (argumentClass == Callback.class) {
- argumentExtractors[i] = ARGUMENT_EXTRACTOR_CALLBACK;
- } else if (argumentClass == ReadableMap.class) {
- argumentExtractors[i] = ARGUMENT_EXTRACTOR_MAP;
- } else if (argumentClass == ReadableArray.class) {
- argumentExtractors[i] = ARGUMENT_EXTRACTOR_ARRAY;
- } else {
- throw new RuntimeException(
- "Got unknown argument class: " + argumentClass.getSimpleName());
- }
+ static final private ArgumentExtractor ARGUMENT_EXTRACTOR_PROMISE = new ArgumentExtractor() {
+ @Override
+ public int getJSArgumentsNeeded() {
+ return 2;
}
- return argumentExtractors;
- }
+
+ @Override
+ public Promise extractArgument(
+ CatalystInstance catalystInstance, ReadableNativeArray jsArguments, int atIndex) {
+ Callback resolve = (Callback) ARGUMENT_EXTRACTOR_CALLBACK
+ .extractArgument(catalystInstance, jsArguments, atIndex);
+ Callback reject = (Callback) ARGUMENT_EXTRACTOR_CALLBACK
+ .extractArgument(catalystInstance, jsArguments, atIndex + 1);
+ return new PromiseImpl(resolve, reject);
+ }
+ };
private class JavaMethod implements NativeMethod {
+
private Method mMethod;
- private ArgumentExtractor[] mArgumentExtractors;
- private Object[] mArguments;
+ private final ArgumentExtractor[] mArgumentExtractors;
+ private final Object[] mArguments;
+ private String mType = METHOD_TYPE_REMOTE;
+ private final int mJSArgumentsNeeded;
public JavaMethod(Method method) {
mMethod = method;
Class[] parameterTypes = method.getParameterTypes();
mArgumentExtractors = buildArgumentExtractors(parameterTypes);
// Since native methods are invoked from a message queue executed on a single thread, it is
// save to allocate only one arguments object per method that can be reused across calls
- mArguments = new Object[mArgumentExtractors.length];
+ mArguments = new Object[parameterTypes.length];
+ mJSArgumentsNeeded = calculateJSArgumentsNeeded();
+ }
+
+ private ArgumentExtractor[] buildArgumentExtractors(Class[] paramTypes) {
+ ArgumentExtractor[] argumentExtractors = new ArgumentExtractor[paramTypes.length];
+ for (int i = 0; i < paramTypes.length; i += argumentExtractors[i].getJSArgumentsNeeded()) {
+ Class argumentClass = paramTypes[i];
+ if (argumentClass == Boolean.class || argumentClass == boolean.class) {
+ argumentExtractors[i] = ARGUMENT_EXTRACTOR_BOOLEAN;
+ } else if (argumentClass == Integer.class || argumentClass == int.class) {
+ argumentExtractors[i] = ARGUMENT_EXTRACTOR_INTEGER;
+ } else if (argumentClass == Double.class || argumentClass == double.class) {
+ argumentExtractors[i] = ARGUMENT_EXTRACTOR_DOUBLE;
+ } else if (argumentClass == Float.class || argumentClass == float.class) {
+ argumentExtractors[i] = ARGUMENT_EXTRACTOR_FLOAT;
+ } else if (argumentClass == String.class) {
+ argumentExtractors[i] = ARGUMENT_EXTRACTOR_STRING;
+ } else if (argumentClass == Callback.class) {
+ argumentExtractors[i] = ARGUMENT_EXTRACTOR_CALLBACK;
+ } else if (argumentClass == Promise.class) {
+ argumentExtractors[i] = ARGUMENT_EXTRACTOR_PROMISE;
+ Assertions.assertCondition(
+ i == paramTypes.length - 1, "Promise must be used as last parameter only");
+ mType = METHOD_TYPE_REMOTE_ASYNC;
+ } else if (argumentClass == ReadableMap.class) {
+ argumentExtractors[i] = ARGUMENT_EXTRACTOR_MAP;
+ } else if (argumentClass == ReadableArray.class) {
+ argumentExtractors[i] = ARGUMENT_EXTRACTOR_ARRAY;
+ } else {
+ throw new RuntimeException(
+ "Got unknown argument class: " + argumentClass.getSimpleName());
+ }
+ }
+ return argumentExtractors;
+ }
+
+ private int calculateJSArgumentsNeeded() {
+ int n = 0;
+ for (ArgumentExtractor extractor : mArgumentExtractors) {
+ n += extractor.getJSArgumentsNeeded();
+ }
+ return n;
+ }
+
+ private String getAffectedRange(int startIndex, int jsArgumentsNeeded) {
+ return jsArgumentsNeeded > 1 ?
+ "" + startIndex + "-" + (startIndex + jsArgumentsNeeded - 1) : "" + startIndex;
}
@Override
public void invoke(CatalystInstance catalystInstance, ReadableNativeArray parameters) {
Systrace.beginSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE, "callJavaModuleMethod");
try {
- if (mArgumentExtractors.length != parameters.size()) {
+ if (mJSArgumentsNeeded != parameters.size()) {
throw new NativeArgumentsParseException(
BaseJavaModule.this.getName() + "." + mMethod.getName() + " got " +
- parameters.size() + " arguments, expected " + mArgumentExtractors.length);
+ parameters.size() + " arguments, expected " + mJSArgumentsNeeded);
}
- int i = 0;
+ int i = 0, jsArgumentsConsumed = 0;
try {
for (; i < mArgumentExtractors.length; i++) {
- mArguments[i] = mArgumentExtractors[i].extractArgument(catalystInstance, parameters, i);
+ mArguments[i] = mArgumentExtractors[i].extractArgument(
+ catalystInstance, parameters, jsArgumentsConsumed);
+ jsArgumentsConsumed += mArgumentExtractors[i].getJSArgumentsNeeded();
}
} catch (UnexpectedNativeTypeException e) {
throw new NativeArgumentsParseException(
e.getMessage() + " (constructing arguments for " + BaseJavaModule.this.getName() +
- "." + mMethod.getName() + " at argument index " + i + ")",
+ "." + mMethod.getName() + " at argument index " +
+ getAffectedRange(jsArgumentsConsumed, mArgumentExtractors[i].getJSArgumentsNeeded()) +
+ ")",
e);
}
@@ -214,6 +266,16 @@ public void invoke(CatalystInstance catalystInstance, ReadableNativeArray parame
Systrace.endSection(Systrace.TRACE_TAG_REACT_JAVA_BRIDGE);
}
}
+
+ /**
+ * Determines how the method is exported in JavaScript:
+ * METHOD_TYPE_REMOTE for regular methods
+ * METHOD_TYPE_REMOTE_ASYNC for methods that return a promise object to the caller.
+ */
+ @Override
+ public String getType() {
+ return mType;
+ }
}
@Override
@@ -24,6 +24,7 @@
public interface NativeModule {
public static interface NativeMethod {
void invoke(CatalystInstance catalystInstance, ReadableNativeArray parameters);
+ String getType();
}
/**
@@ -199,6 +199,7 @@ public NativeModuleRegistry build() {
MethodRegistration method = module.methods.get(i);
jg.writeObjectFieldStart(method.name);
jg.writeNumberField("methodID", i);
+ jg.writeStringField("type", method.method.getType());
jg.writeEndObject();
}
jg.writeEndObject();
@@ -0,0 +1,22 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+package com.facebook.react.bridge;
+
+/**
+ * Interface that represents a JavaScript Promise which can be passed to the native module as a
+ * method parameter.
+ *
+ * Methods annotated with {@link ReactMethod} that use {@link Promise} as type of the last parameter
+ * will be marked as "remoteAsync" and will return a promise when invoked from JavaScript.
+ */
+public interface Promise {

This comment has been minimized.

Show comment
Hide comment
@ide

ide Oct 29, 2015

Collaborator

An object with resolve and reject methods is typically called a "Deferred" object so you might want to rename this class. (The public API of standard Promises is then / catch but not resolve / reject.)
https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/Promise.jsm/Deferred

@ide

ide Oct 29, 2015

Collaborator

An object with resolve and reject methods is typically called a "Deferred" object so you might want to rename this class. (The public API of standard Promises is then / catch but not resolve / reject.)
https://developer.mozilla.org/en-US/docs/Mozilla/JavaScript_code_modules/Promise.jsm/Deferred

This comment has been minimized.

Show comment
Hide comment
@davidaurelio

davidaurelio Oct 30, 2015

Contributor

Thanks @ide. Many historical Deferred implementations were promises themselves (e.g. good old dojo, jQuery), and I feel the boundaries are a little bit blurry.

Even if Promise is not 100% correct, the only other suitable term in the spec is “executor”. And that’s not a 100% fit either.

I think we are keeping the mental distance for developers low by calling it Promise. My fear for misunderstandings, on the other hand, is rather low in this case.

@davidaurelio

davidaurelio Oct 30, 2015

Contributor

Thanks @ide. Many historical Deferred implementations were promises themselves (e.g. good old dojo, jQuery), and I feel the boundaries are a little bit blurry.

Even if Promise is not 100% correct, the only other suitable term in the spec is “executor”. And that’s not a 100% fit either.

I think we are keeping the mental distance for developers low by calling it Promise. My fear for misunderstandings, on the other hand, is rather low in this case.

This comment has been minimized.

Show comment
Hide comment
@ide

ide Oct 30, 2015

Collaborator

OK. I think BridgingPromise or PromiseFulfiller would be slightly clearer but it's easy enough to rename classes if it becomes an actual issue.

@ide

ide Oct 30, 2015

Collaborator

OK. I think BridgingPromise or PromiseFulfiller would be slightly clearer but it's easy enough to rename classes if it becomes an actual issue.

This comment has been minimized.

Show comment
Hide comment
@davidaurelio

davidaurelio Oct 31, 2015

Contributor

@ide I am not against renaming it, I just don’t want to spend time doing it :-) Feel free to create a PR, I’ll be happy to pull it in. I like BridgingPromise or BridgedPromise.

@davidaurelio

davidaurelio Oct 31, 2015

Contributor

@ide I am not against renaming it, I just don’t want to spend time doing it :-) Feel free to create a PR, I’ll be happy to pull it in. I like BridgingPromise or BridgedPromise.

+ void resolve(Object value);
+ void reject(String reason);
+}
@@ -0,0 +1,45 @@
+/**
+ * Copyright (c) 2015-present, Facebook, Inc.
+ * All rights reserved.
+ *
+ * This source code is licensed under the BSD-style license found in the
+ * LICENSE file in the root directory of this source tree. An additional grant
+ * of patent rights can be found in the PATENTS file in the same directory.
+ */
+
+/**
+ * Implementation of two javascript functions that can be used to resolve or reject a js promise.
+ */
+package com.facebook.react.bridge;
+
+import javax.annotation.Nullable;
+
+public class PromiseImpl implements Promise {
+ private @Nullable Callback mResolve;
+ private @Nullable Callback mReject;
+
+ public PromiseImpl(@Nullable Callback resolve, @Nullable Callback reject) {
+ mResolve = resolve;
+ mReject = reject;
+ }
+
+ @Override
+ public void resolve(Object value) {
+ if (mResolve != null) {
+ mResolve.invoke(value);
+ }
+ }
+
+ @Override
+ public void reject(String reason) {
+ if (mReject != null) {
+ // The JavaScript side expects a map with at least the error message.
+ // It is possible to expose all kind of information. It will be available on the JS
+ // error instance.
+ // TODO(8850038): add more useful information, e.g. the native stack trace.
+ WritableNativeMap errorInfo = new WritableNativeMap();
+ errorInfo.putString("message", reason);
+ mReject.invoke(errorInfo);
+ }
+ }
+}

9 comments on commit b86a6e3

@mkonicek

This comment has been minimized.

Show comment
Hide comment
@mkonicek

mkonicek Oct 29, 2015

Contributor

Context from @davidaurelio:

iOS has support for “async” bridged methods since May (see #1232). Async methods return a Promise object in JavaScript rather than accepting callback functions.

The implementation adds a new interface (Promise) and exports methods that use that as type for the last parameter as async functions.

Contributor

mkonicek replied Oct 29, 2015

Context from @davidaurelio:

iOS has support for “async” bridged methods since May (see #1232). Async methods return a Promise object in JavaScript rather than accepting callback functions.

The implementation adds a new interface (Promise) and exports methods that use that as type for the last parameter as async functions.

@vjeux

This comment has been minimized.

Show comment
Hide comment
@vjeux

vjeux Oct 29, 2015

Contributor

cc @ide it'll make you happy

Contributor

vjeux replied Oct 29, 2015

cc @ide it'll make you happy

@brentvatne

This comment has been minimized.

Show comment
Hide comment
@brentvatne

brentvatne Oct 29, 2015

Collaborator

woohoo!

Collaborator

brentvatne replied Oct 29, 2015

woohoo!

@ide

This comment has been minimized.

Show comment
Hide comment
@ide

ide Oct 29, 2015

Collaborator

@davidaurelio, wow this is really awesome! I have a couple of suggestions that I'll leave in inline comments. This is exciting to announce for 0.15-rc!

Collaborator

ide replied Oct 29, 2015

@davidaurelio, wow this is really awesome! I have a couple of suggestions that I'll leave in inline comments. This is exciting to announce for 0.15-rc!

@satya164

This comment has been minimized.

Show comment
Hide comment
@satya164

satya164 Oct 31, 2015

Collaborator

Any docs on how to use it?

Collaborator

satya164 replied Oct 31, 2015

Any docs on how to use it?

@davidaurelio

This comment has been minimized.

Show comment
Hide comment
@davidaurelio

davidaurelio Oct 31, 2015

Contributor

@satya164 I will sync up with @mkonicek on whether to add docs immediately or together with the release.

Contributor

davidaurelio replied Oct 31, 2015

@satya164 I will sync up with @mkonicek on whether to add docs immediately or together with the release.

@satya164

This comment has been minimized.

Show comment
Hide comment
@satya164

satya164 Oct 31, 2015

Collaborator

@davidaurelio Cool. I'm on master though, so would love a hint :)

Collaborator

satya164 replied Oct 31, 2015

@davidaurelio Cool. I'm on master though, so would love a hint :)

@davidaurelio

This comment has been minimized.

Show comment
Hide comment
@davidaurelio

davidaurelio Oct 31, 2015

Contributor

just accept a Promise as last parameter in your bridged method (instead of the usual Callback):

import com.facebook.react.bridge.Promise
//
@ReactMethod
void bridgedMethod(int parameter, Promise promise) { /**/ }

that’s it.

Contributor

davidaurelio replied Oct 31, 2015

just accept a Promise as last parameter in your bridged method (instead of the usual Callback):

import com.facebook.react.bridge.Promise
//
@ReactMethod
void bridgedMethod(int parameter, Promise promise) { /**/ }

that’s it.

@satya164

This comment has been minimized.

Show comment
Hide comment
@satya164

satya164 Nov 1, 2015

Collaborator
Collaborator

satya164 replied Nov 1, 2015

Please sign in to comment.