diff --git a/Examples/UIExplorer/UIExplorerUnitTests/RCTAllocationTests.m b/Examples/UIExplorer/UIExplorerUnitTests/RCTAllocationTests.m index 8fc629fbe..3f7f4bbed 100644 --- a/Examples/UIExplorer/UIExplorerUnitTests/RCTAllocationTests.m +++ b/Examples/UIExplorer/UIExplorerUnitTests/RCTAllocationTests.m @@ -17,6 +17,7 @@ #import "RCTBridge.h" #import "RCTContextExecutor.h" +#import "RCTModuleMethod.h" #import "RCTRootView.h" #define RUN_RUNLOOP_WHILE(CONDITION) \ @@ -62,6 +63,11 @@ RCT_EXPORT_MODULE(); _valid = NO; } +RCT_EXPORT_METHOD(test:(__unused NSString *)a + :(__unused NSNumber *)b + :(__unused RCTResponseSenderBlock)c + :(__unused RCTResponseErrorBlock)d) {} + @end @interface RCTAllocationTests : XCTestCase @@ -124,6 +130,19 @@ RCT_EXPORT_MODULE(); XCTAssertNil(weakModule, @"AllocationTestModule should have been deallocated"); } +- (void)testModuleMethodsAreDeallocated +{ + __weak RCTModuleMethod *weakMethod; + @autoreleasepool { + __autoreleasing RCTModuleMethod *method = [[RCTModuleMethod alloc] initWithObjCMethodName:@"test:(NSString *)a :(nonnull NSNumber *)b :(RCTResponseSenderBlock)c :(RCTResponseErrorBlock)d" JSMethodName:@"" moduleClass:[AllocationTestModule class]]; + weakMethod = method; + XCTAssertNotNil(method, @"RCTModuleMethod should have been created"); + } + + RUN_RUNLOOP_WHILE(weakMethod) + XCTAssertNil(weakMethod, @"RCTModuleMethod should have been deallocated"); +} + - (void)testJavaScriptExecutorIsDeallocated { __weak id weakExecutor; diff --git a/React/Base/RCTModuleMethod.m b/React/Base/RCTModuleMethod.m index db4a3acf8..47b1de8d2 100644 --- a/React/Base/RCTModuleMethod.m +++ b/React/Base/RCTModuleMethod.m @@ -51,6 +51,14 @@ typedef void (^RCTArgumentBlock)(RCTBridge *, NSInvocation *, NSUInteger, id); NSArray *_argumentBlocks; } +static void RCTLogArgumentError(RCTModuleMethod *method, NSUInteger index, + id valueOrType, const char *issue) +{ + RCTLogError(@"Argument %tu (%@) of %@.%@ %s", index, valueOrType, + RCTBridgeModuleNameForClass(method->_moduleClass), + method->_JSMethodName, issue); +} + RCT_NOT_IMPLEMENTED(-init) void RCTParseObjCMethodName(NSString **, NSArray **); @@ -129,6 +137,7 @@ void RCTParseObjCMethodName(NSString **objCMethodName, NSArray **arguments) // Get method signature _methodSignature = [_moduleClass instanceMethodSignatureForSelector:_selector]; + RCTAssert(_methodSignature, @"%@ is not a recognized Objective-C method.", objCMethodName); // Process arguments NSUInteger numberOfArguments = _methodSignature.numberOfArguments; @@ -140,12 +149,12 @@ void RCTParseObjCMethodName(NSString **objCMethodName, NSArray **arguments) [invocation setArgument:&value atIndex:(index) + 2]; \ }]; + __weak RCTModuleMethod *weakSelf = self; void (^addBlockArgument)(void) = ^{ RCT_ARG_BLOCK( if (RCT_DEBUG && json && ![json isKindOfClass:[NSNumber class]]) { - RCTLogError(@"Argument %tu (%@) of %@.%@ should be a number", index, - json, RCTBridgeModuleNameForClass(_moduleClass), _JSMethodName); + RCTLogArgumentError(weakSelf, index, json, "should be a function"); return; } @@ -230,8 +239,7 @@ case _value: { \ RCT_ARG_BLOCK( if (RCT_DEBUG && json && ![json isKindOfClass:[NSNumber class]]) { - RCTLogError(@"Argument %tu (%@) of %@.%@ should be a number", index, - json, RCTBridgeModuleNameForClass(_moduleClass), _JSMethodName); + RCTLogArgumentError(weakSelf, index, json, "should be a function"); return; } @@ -248,8 +256,7 @@ case _value: { \ _moduleClass, objCMethodName); RCT_ARG_BLOCK( if (RCT_DEBUG && ![json isKindOfClass:[NSNumber class]]) { - RCTLogError(@"Argument %tu (%@) of %@.%@ must be a promise resolver ID", index, - json, RCTBridgeModuleNameForClass(_moduleClass), _JSMethodName); + RCTLogArgumentError(weakSelf, index, json, "should be a promise resolver function"); return; } @@ -267,8 +274,7 @@ case _value: { \ _moduleClass, objCMethodName); RCT_ARG_BLOCK( if (RCT_DEBUG && ![json isKindOfClass:[NSNumber class]]) { - RCTLogError(@"Argument %tu (%@) of %@.%@ must be a promise rejecter ID", index, - json, RCTBridgeModuleNameForClass(_moduleClass), _JSMethodName); + RCTLogArgumentError(weakSelf, index, json, "should be a promise rejecter function"); return; } @@ -293,9 +299,8 @@ case _value: { \ RCTNullability nullability = argument.nullability; if (!isNullableType) { if (nullability == RCTNullable) { - RCTLogError(@"Argument %tu (%@) of %@.%@ is marked as nullable, but " - "is not a nullable type.", i - 2, typeName, - RCTBridgeModuleNameForClass(_moduleClass), _JSMethodName); + RCTLogArgumentError(weakSelf, i - 2, typeName, "is marked as " + "nullable, but is not a nullable type."); } nullability = RCTNonnullable; } @@ -307,11 +312,11 @@ case _value: { \ if ([typeName isEqualToString:@"NSNumber"]) { BOOL unspecified = (nullability == RCTNullabilityUnspecified); if (!argument.unused && (nullability == RCTNullable || unspecified)) { - RCTLogError(@"Argument %tu (NSNumber) of %@.%@ %@, but React requires " - "that all NSNumber arguments are explicitly marked as " - "`nonnull` to ensure compatibility with Android.", i - 2, - RCTBridgeModuleNameForClass(_moduleClass), _JSMethodName, - unspecified ? @"has unspecified nullability" : @"is marked as nullable"); + RCTLogArgumentError(weakSelf, i - 2, typeName, + [unspecified ? @"has unspecified nullability" : @"is marked as nullable" + stringByAppendingString: @" but React requires that all NSNumber " + "arguments are explicitly marked as `nonnull` to ensure " + "compatibility with Android."].UTF8String); } nullability = RCTNonnullable; } @@ -320,8 +325,7 @@ case _value: { \ RCTArgumentBlock oldBlock = argumentBlocks[i - 2]; argumentBlocks[i - 2] = ^(RCTBridge *bridge, NSInvocation *invocation, NSUInteger index, id json) { if (json == nil || json == (id)kCFNull) { - RCTLogError(@"Argument %tu (%@) of %@.%@ must not be null", index, - typeName, RCTBridgeModuleNameForClass(_moduleClass), _JSMethodName); + RCTLogArgumentError(weakSelf, index, typeName, "must not be null"); id null = nil; [invocation setArgument:&null atIndex:index + 2]; } else {