From 7a97043a3ee9911a7e873fb166136de4f8189dff Mon Sep 17 00:00:00 2001 From: Alex Akers Date: Thu, 26 Mar 2015 09:44:58 -0700 Subject: [PATCH] [React Kit] Remove embarrassing TODOs --- Libraries/Image/RCTImageDownloader.m | 2 -- Libraries/Text/RCTShadowText.m | 1 - Libraries/Text/RCTTextManager.m | 15 --------------- React/Base/RCTBridge.m | 1 - React/Base/RCTInvalidating.h | 6 ++---- React/Base/RCTLog.m | 2 +- React/Executors/RCTContextExecutor.m | 2 +- React/Modules/RCTUIManager.m | 4 ---- 8 files changed, 4 insertions(+), 29 deletions(-) diff --git a/Libraries/Image/RCTImageDownloader.m b/Libraries/Image/RCTImageDownloader.m index e204059f1d..c1916dc193 100644 --- a/Libraries/Image/RCTImageDownloader.m +++ b/Libraries/Image/RCTImageDownloader.m @@ -12,8 +12,6 @@ #import "RCTCache.h" #import "RCTUtils.h" -// TODO: something a bit more sophisticated - typedef void (^RCTCachedDataDownloadBlock)(BOOL cached, NSData *data, NSError *error); @implementation RCTImageDownloader diff --git a/Libraries/Text/RCTShadowText.m b/Libraries/Text/RCTShadowText.m index 03d327898b..da4b90c06b 100644 --- a/Libraries/Text/RCTShadowText.m +++ b/Libraries/Text/RCTShadowText.m @@ -153,7 +153,6 @@ static css_dim_t RCTMeasure(void *context, float width) } }]; - // TODO: umm, these can't be null, so we're mapping left to natural - is that right? self.textAlign = _textAlign ?: NSTextAlignmentNatural; self.writingDirection = _writingDirection ?: NSWritingDirectionNatural; diff --git a/Libraries/Text/RCTTextManager.m b/Libraries/Text/RCTTextManager.m index 70cbd9ab5f..95896fd058 100644 --- a/Libraries/Text/RCTTextManager.m +++ b/Libraries/Text/RCTTextManager.m @@ -72,24 +72,10 @@ RCT_CUSTOM_SHADOW_PROPERTY(numberOfLines, NSInteger, RCTShadowText) view.truncationMode = truncationMode; } -- (RCTViewManagerUIBlock)uiBlockToAmendWithShadowView:(RCTShadowText *)shadowView -{ - //TODO: This could be a cleaner replacement for uiBlockToAmendWithShadowViewRegistry - return nil; -} - -// TODO: the purpose of this block is effectively just to copy properties from the shadow views -// to their equivalent UIViews. In this case, the property being copied is the attributed text, -// but the same principle could be used to copy any property. The implementation is really ugly tho -// because the RCTViewManager doesn't retain a reference to the views that it manages, so it basically -// has to search the entire view hierarchy for relevant views. Not awesome. This seems like something -// where we could introduce a generic solution - perhaps a method on RCTShadowView that is called after -// layout to copy its properties across? - (RCTViewManagerUIBlock)uiBlockToAmendWithShadowViewRegistry:(RCTSparseArray *)shadowViewRegistry { NSMutableArray *uiBlocks = [NSMutableArray new]; - // TODO: are modules global, or specific to a given rootView? for (RCTShadowView *rootView in shadowViewRegistry.allObjects) { if (![rootView isReactRootView]) { // This isn't a root view @@ -101,7 +87,6 @@ RCT_CUSTOM_SHADOW_PROPERTY(numberOfLines, NSInteger, RCTShadowText) continue; } - // TODO: this is a slightly weird way to do this - a recursive approach would be cleaner RCTSparseArray *reactTaggedAttributedStrings = [[RCTSparseArray alloc] init]; NSMutableArray *queue = [NSMutableArray arrayWithObject:rootView]; for (NSInteger i = 0; i < [queue count]; i++) { diff --git a/React/Base/RCTBridge.m b/React/Base/RCTBridge.m index 8a81a23272..148a5d9332 100644 --- a/React/Base/RCTBridge.m +++ b/React/Base/RCTBridge.m @@ -612,7 +612,6 @@ static id _latestJSExecutor; // Wait for queued methods to finish dispatch_sync(self.shadowQueue, ^{ // Make sure all dispatchers have been executed before continuing - // TODO: is this still needed? }); // Invalidate modules diff --git a/React/Base/RCTInvalidating.h b/React/Base/RCTInvalidating.h index be105b485c..e961a39799 100644 --- a/React/Base/RCTInvalidating.h +++ b/React/Base/RCTInvalidating.h @@ -9,10 +9,8 @@ #import -// TODO (#5906496): is there a reason for this protocol? It seems to be -// used in a number of places where it isn't really required - only the -// RCTBridge actually ever calls casts to it - in all other -// cases it is simply a way of adding some method definitions to classes +// TODO (#5906496): This protocol is only used to add method definitions to +// classes. We should decide if it's actually necessary. @protocol RCTInvalidating diff --git a/React/Base/RCTLog.m b/React/Base/RCTLog.m index e8f520e7b7..cd035eda2f 100644 --- a/React/Base/RCTLog.m +++ b/React/Base/RCTLog.m @@ -34,7 +34,7 @@ static inline NSString *_RCTLogPreamble(const char *file, int lineNumber, const return [NSString stringWithFormat:@"[RCTLog][tid:%@][%@:%d]>", threadName, fileName, lineNumber]; } -// TODO (#5906496): // kinda ugly that this is tied to RCTBridge +// TODO (#5906496): Does this need to be tied to RCTBridge? NSString *RCTLogObjects(NSArray *objects, NSString *level) { NSString *str = objects[0]; diff --git a/React/Executors/RCTContextExecutor.m b/React/Executors/RCTContextExecutor.m index 4370bd7bd1..e2b289f80b 100644 --- a/React/Executors/RCTContextExecutor.m +++ b/React/Executors/RCTContextExecutor.m @@ -109,7 +109,7 @@ static NSError *RCTNSErrorFromJSError(JSContextRef context, JSValueRef jsError) // run the run loop while (kCFRunLoopRunStopped != CFRunLoopRunInMode(kCFRunLoopDefaultMode, [[NSDate distantFuture] timeIntervalSinceReferenceDate], NO)) { - RCTAssert(NO, @"not reached assertion"); // runloop spun. that's bad. + RCTAssert(NO, @"not reached assertion"); // runloop spun. that's bad. } } } diff --git a/React/Modules/RCTUIManager.m b/React/Modules/RCTUIManager.m index e6a540db2f..a0e7285c6b 100644 --- a/React/Modules/RCTUIManager.m +++ b/React/Modules/RCTUIManager.m @@ -1356,10 +1356,6 @@ static void RCTMeasureLayout(RCTShadowView *view, }]; } -// TODO: remove horrible hack - this is only here so that -// [rootView endAndResetInteractionTiming] below doesn't raise warnings -- (NSDictionary *)endAndResetInteractionTiming { return nil; } - - (void)endAndResetInteractionTiming:(RCTResponseSenderBlock)onSuccess onError:(RCTResponseSenderBlock)onError {