Permalink
Browse files

Make CameraRoll work with Promises

Summary:
public
This is the first module moving to the new model of working with Promises.

We now warn on uses of callback version.  At some point we will remove that.

Reviewed By: davidaurelio

Differential Revision: D2849811

fb-gh-sync-id: 8a31924cc2b438efc58f3ad22d5f27c273563472
  • Loading branch information...
Dave Miller facebook-github-bot-2
Dave Miller authored and facebook-github-bot-2 committed Jan 21, 2016
1 parent 34d5fa2 commit 9baff8f437eee49f8ab0e6f433bf86466ca16662
@@ -128,57 +128,56 @@ class CameraRoll {
* - a tag not matching any of the above, which means the image data will
* be stored in memory (and consume memory as long as the process is alive)
*
- * @param successCallback Invoked with the value of `tag` on success.
- * @param errorCallback Invoked with error message on error.
+ * Returns a Promise which when resolved will be passed the new uri
+ *
*/
- static saveImageWithTag(tag, successCallback, errorCallback) {
+ static saveImageWithTag(tag) {
invariant(
typeof tag === 'string',
'CameraRoll.saveImageWithTag tag must be a valid string.'
);
- RCTCameraRollManager.saveImageWithTag(
- tag,
- (imageTag) => {
- successCallback && successCallback(imageTag);
- },
- (errorMessage) => {
- errorCallback && errorCallback(errorMessage);
- });
+ if (arguments.length > 1) {
+ console.warn("CameraRoll.saveImageWithTag(tag, success, error) is deprecated. Use the returned Promise instead");
+ let successCallback = arguments[1];
+ let errorCallback = arguments[2] || ( () => {} );
+ RCTCameraRollManager.saveImageWithTag(tag).then(successCallback, errorCallback);
+ return;
+ }
+ return RCTCameraRollManager.saveImageWithTag(tag);
}
/**
- * Invokes `callback` with photo identifier objects from the local camera
+ * Returns a Promise with photo identifier objects from the local camera
* roll of the device matching shape defined by `getPhotosReturnChecker`.
*
* @param {object} params See `getPhotosParamChecker`.
- * @param {function} callback Invoked with arg of shape defined by
- * `getPhotosReturnChecker` on success.
- * @param {function} errorCallback Invoked with error message on error.
+ *
+ * Returns a Promise which when resolved will be of shape `getPhotosReturnChecker`
+ *
*/
- static getPhotos(params, callback, errorCallback) {
- var metaCallback = callback;
+ static getPhotos(params) {
if (__DEV__) {
getPhotosParamChecker({params}, 'params', 'CameraRoll.getPhotos');
- invariant(
- typeof callback === 'function',
- 'CameraRoll.getPhotos callback must be a valid function.'
- );
- invariant(
- typeof errorCallback === 'function',
- 'CameraRoll.getPhotos errorCallback must be a valid function.'
- );
}
- if (__DEV__) {
- metaCallback = (response) => {
- getPhotosReturnChecker(
- {response},
- 'response',
- 'CameraRoll.getPhotos callback'
- );
- callback(response);
- };
+ if (arguments.length > 1) {
+ console.warn("CameraRoll.getPhotos(tag, success, error) is deprecated. Use the returned Promise instead");
+ let successCallback = arguments[1];
+ if (__DEV__) {
+ let callback = arguments[1];
+ successCallback = (response) => {
+ getPhotosReturnChecker(
+ {response},
+ 'response',
+ 'CameraRoll.getPhotos callback'
+ );
+ callback(response);
+ };
+ }
+ let errorCallback = arguments[2] || ( () => {} );
+ RCTCameraRollManager.getPhotos(params).then(successCallback, errorCallback);
}
- RCTCameraRollManager.getPhotos(params, metaCallback, errorCallback);
+ // TODO: Add the __DEV__ check back in to verify the Promise result
+ return RCTCameraRollManager.getPhotos(params);
}
}
@@ -79,43 +79,46 @@ @implementation RCTCameraRollManager
@synthesize bridge = _bridge;
+NSString *const RCTErrorUnableToLoad = @"E_UNABLE_TO_LOAD";
+NSString *const RCTErrorUnableToSave = @"E_UNABLE_TO_SAVE";
+
RCT_EXPORT_METHOD(saveImageWithTag:(NSString *)imageTag
- successCallback:(RCTResponseSenderBlock)successCallback
- errorCallback:(RCTResponseErrorBlock)errorCallback)
+ resolve:(RCTPromiseResolveBlock)resolve
+ reject:(RCTPromiseRejectBlock)reject)
{
[_bridge.imageLoader loadImageWithTag:imageTag callback:^(NSError *loadError, UIImage *loadedImage) {
if (loadError) {
- errorCallback(loadError);
+ reject(RCTErrorUnableToLoad, nil, loadError);
return;
}
// It's unclear if writeImageToSavedPhotosAlbum is thread-safe
dispatch_async(dispatch_get_main_queue(), ^{
[_bridge.assetsLibrary writeImageToSavedPhotosAlbum:loadedImage.CGImage metadata:nil completionBlock:^(NSURL *assetURL, NSError *saveError) {
if (saveError) {
RCTLogWarn(@"Error saving cropped image: %@", saveError);
- errorCallback(saveError);
+ reject(RCTErrorUnableToSave, nil, saveError);
} else {
- successCallback(@[assetURL.absoluteString]);
+ resolve(@[assetURL.absoluteString]);
}
}];
});
}];
}
-static void RCTCallCallback(RCTResponseSenderBlock callback,
- NSArray<NSDictionary<NSString *, id> *> *assets,
- BOOL hasNextPage)
+static void RCTResolvePromise(RCTPromiseResolveBlock resolve,
+ NSArray<NSDictionary<NSString *, id> *> *assets,
+ BOOL hasNextPage)
{
if (!assets.count) {
- callback(@[@{
+ resolve(@[@{
@"edges": assets,
@"page_info": @{
@"has_next_page": @NO,
}
}]);
return;
}
- callback(@[@{
+ resolve(@[@{
@"edges": assets,
@"page_info": @{
@"start_cursor": assets[0][@"node"][@"image"][@"uri"],
@@ -126,8 +129,8 @@ static void RCTCallCallback(RCTResponseSenderBlock callback,
}
RCT_EXPORT_METHOD(getPhotos:(NSDictionary *)params
- successCallback:(RCTResponseSenderBlock)successCallback
- errorCallback:(RCTResponseErrorBlock)errorCallback)
+ resolve:(RCTPromiseResolveBlock)resolve
+ reject:(RCTPromiseRejectBlock)reject)
{
NSUInteger first = [RCTConvert NSInteger:params[@"first"]];
NSString *afterCursor = [RCTConvert NSString:params[@"after"]];
@@ -137,7 +140,7 @@ static void RCTCallCallback(RCTResponseSenderBlock callback,
BOOL __block foundAfter = NO;
BOOL __block hasNextPage = NO;
- BOOL __block calledCallback = NO;
+ BOOL __block resolvedPromise = NO;
NSMutableArray<NSDictionary<NSString *, id> *> *assets = [NSMutableArray new];
[_bridge.assetsLibrary enumerateGroupsWithTypes:groupTypes usingBlock:^(ALAssetsGroup *group, BOOL *stopGroups) {
@@ -157,9 +160,9 @@ static void RCTCallCallback(RCTResponseSenderBlock callback,
*stopAssets = YES;
*stopGroups = YES;
hasNextPage = YES;
- RCTAssert(calledCallback == NO, @"Called the callback before we finished processing the results.");
- RCTCallCallback(successCallback, assets, hasNextPage);
- calledCallback = YES;
+ RCTAssert(resolvedPromise == NO, @"Resolved the promise before we finished processing the results.");
+ RCTResolvePromise(resolve, assets, hasNextPage);
+ resolvedPromise = YES;
return;
}
CGSize dimensions = [result defaultRepresentation].dimensions;
@@ -188,18 +191,18 @@ static void RCTCallCallback(RCTResponseSenderBlock callback,
}
}];
} else {
- // Sometimes the enumeration continues even if we set stop above, so we guard against calling the callback
+ // Sometimes the enumeration continues even if we set stop above, so we guard against resolving the promise
// multiple times here.
- if (!calledCallback) {
- RCTCallCallback(successCallback, assets, hasNextPage);
- calledCallback = YES;
+ if (!resolvedPromise) {
+ RCTResolvePromise(resolve, assets, hasNextPage);
+ resolvedPromise = YES;
}
}
} failureBlock:^(NSError *error) {
if (error.code != ALAssetsLibraryAccessUserDeniedError) {
RCTLogError(@"Failure while iterating through asset groups %@", error);
}
- errorCallback(error);
+ reject(RCTErrorUnableToLoad, nil, error);
}];
}
@@ -16,6 +16,7 @@
let Systrace = require('Systrace');
let ErrorUtils = require('ErrorUtils');
let JSTimersExecution = require('JSTimersExecution');
+let Platform = require('Platform');
let invariant = require('invariant');
let keyMirror = require('keyMirror');
@@ -318,10 +319,21 @@ class MessageQueue {
if (type === MethodTypes.remoteAsync) {
fn = function(...args) {
return new Promise((resolve, reject) => {
- self.__nativeCall(module, method, args, resolve, (errorData) => {
- var error = createErrorFromErrorData(errorData);
- reject(error);
- });
+ self.__nativeCall(
+ module,
+ method,
+ args,
+ (data) => {
+ // iOS always wraps the data in an Array regardless of what the
+ // shape of the data so we strip it out
+ // Android sends the data back properly
+ // TODO: Remove this once iOS has support for Promises natively (t9774697)
+ resolve(Platform.OS == 'ios' ? data[0] : data);

This comment has been minimized.

Show comment
Hide comment
@dmmiller

dmmiller Jan 21, 2016

Contributor

@ide Notice this change to iOS promises. The parameter is no longer wrapped in an array. It is just whatever object is passed from the native side. I think you might be the only person relying on this. I changed it to match Android which doesn't put the extra wrapper around it. Hopefully I didn't break you or not too badly anyway.

@dmmiller

dmmiller Jan 21, 2016

Contributor

@ide Notice this change to iOS promises. The parameter is no longer wrapped in an array. It is just whatever object is passed from the native side. I think you might be the only person relying on this. I changed it to match Android which doesn't put the extra wrapper around it. Hopefully I didn't break you or not too badly anyway.

This comment has been minimized.

Show comment
Hide comment
@geof90

geof90 Feb 9, 2016

Contributor

@dmmiller Why is this needed? I'm facing issues with my native module in RN 0.20-rc1.

screen shot 2016-02-09 at 3 50 06 pm

Reverting this change made everything work fine again.

@geof90

geof90 Feb 9, 2016

Contributor

@dmmiller Why is this needed? I'm facing issues with my native module in RN 0.20-rc1.

screen shot 2016-02-09 at 3 50 06 pm

Reverting this change made everything work fine again.

+ },
+ (errorData) => {
+ var error = createErrorFromErrorData(errorData);
+ reject(error);
+ });
});
};
} else {
@@ -12,11 +12,9 @@
'use strict';
-var AndroidConstants = require('NativeModules').AndroidConstants;
-
var Platform = {
OS: 'android',
- Version: AndroidConstants.Version,
+ get Version() { return require('NativeModules').AndroidConstants.Version; },
};
module.exports = Platform;
@@ -9,6 +9,8 @@
package com.facebook.react.bridge;
+import javax.annotation.Nullable;
+
/**
* Interface that represents a JavaScript Promise which can be passed to the native module as a
* method parameter.
@@ -22,5 +24,5 @@
@Deprecated
void reject(String reason);
void reject(String code, Throwable extra);
- void reject(String code, String reason, Throwable extra);
+ void reject(String code, String reason, @Nullable Throwable extra);
}
@@ -50,7 +50,7 @@ public void reject(String code, Throwable extra) {
}
@Override
- public void reject(String code, String reason, Throwable extra) {
+ public void reject(String code, String reason, @Nullable Throwable extra) {
if (mReject != null) {
if (code == null) {
code = DEFAULT_ERROR;
Oops, something went wrong.

4 comments on commit 9baff8f

@dmmiller

This comment has been minimized.

Show comment
Hide comment
@dmmiller

dmmiller Jan 21, 2016

Contributor

@satya164 Here is the change to CameraRoll we were talking about.

Contributor

dmmiller replied Jan 21, 2016

@satya164 Here is the change to CameraRoll we were talking about.

@satya164

This comment has been minimized.

Show comment
Hide comment
@satya164

satya164 Jan 21, 2016

Collaborator

@dmmiller Thanks.

Collaborator

satya164 replied Jan 21, 2016

@dmmiller Thanks.

@samuelkraft

This comment has been minimized.

Show comment
Hide comment
@samuelkraft

samuelkraft Feb 18, 2016

Is there anywhere to see an example use with promises? The docs seems outdated.

Is there anywhere to see an example use with promises? The docs seems outdated.

@satya164

This comment has been minimized.

Show comment
Hide comment
@satya164

satya164 Feb 18, 2016

Collaborator

@samuelkraft It's the same API. Just that it returns a promise rather than taking success and error callbacks. A PR will be awesome to update the docs.

Collaborator

satya164 replied Feb 18, 2016

@samuelkraft It's the same API. Just that it returns a promise rather than taking success and error callbacks. A PR will be awesome to update the docs.

Please sign in to comment.