From 3b80531f3247bee83a2e63a1ee31ca56ab15d024 Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Thu, 21 Dec 2023 12:01:02 -0800 Subject: [PATCH] Move prop diffing for Interop Layer to Adapter Summary: We have 1 coordinator per class but 1 adapter per instance. Currently, the `oldProps` are stored in the coordinator and not into the adapter. Therefore, when we create multiple instances of the same legacy component, the props might get messy or not updated properly. This change moves the `oldProps` and the diffing code to the Adapter rather than to the coordinator, making them instance-specific. ## Changelog: [iOS][Fixed] - Move old props and prop diffing to the interop layer adapter Reviewed By: sammy-SC Differential Revision: D52368222 fbshipit-source-id: 0f0c47b586fe61404250c5bfe51a7e2c63012815 --- ...acyViewManagerInteropCoordinatorAdapter.mm | 94 +++++++++++++++++- .../RCTLegacyViewManagerInteropCoordinator.h | 2 +- .../RCTLegacyViewManagerInteropCoordinator.mm | 96 +------------------ 3 files changed, 98 insertions(+), 94 deletions(-) diff --git a/packages/react-native/React/Fabric/Mounting/ComponentViews/LegacyViewManagerInterop/RCTLegacyViewManagerInteropCoordinatorAdapter.mm b/packages/react-native/React/Fabric/Mounting/ComponentViews/LegacyViewManagerInterop/RCTLegacyViewManagerInteropCoordinatorAdapter.mm index 0fea1bfc95afbc..5470c02b935a6e 100644 --- a/packages/react-native/React/Fabric/Mounting/ComponentViews/LegacyViewManagerInterop/RCTLegacyViewManagerInteropCoordinatorAdapter.mm +++ b/packages/react-native/React/Fabric/Mounting/ComponentViews/LegacyViewManagerInterop/RCTLegacyViewManagerInteropCoordinatorAdapter.mm @@ -6,11 +6,13 @@ */ #import "RCTLegacyViewManagerInteropCoordinatorAdapter.h" +#import #import @implementation RCTLegacyViewManagerInteropCoordinatorAdapter { RCTLegacyViewManagerInteropCoordinator *_coordinator; NSInteger _tag; + NSDictionary *_oldProps; } - (instancetype)initWithCoordinator:(RCTLegacyViewManagerInteropCoordinator *)coordinator reactTag:(NSInteger)tag @@ -47,7 +49,14 @@ - (UIView *)paperView - (void)setProps:(const folly::dynamic &)props { - [_coordinator setProps:props forView:self.paperView]; + if (props.isObject()) { + NSDictionary *convertedProps = facebook::react::convertFollyDynamicToId(props); + NSDictionary *diffedProps = [self _diffProps:convertedProps]; + + [_coordinator setProps:diffedProps forView:self.paperView]; + + _oldProps = convertedProps; + } } - (void)handleCommand:(NSString *)commandName args:(NSArray *)args @@ -55,4 +64,87 @@ - (void)handleCommand:(NSString *)commandName args:(NSArray *)args [_coordinator handleCommand:commandName args:args reactTag:_tag paperView:self.paperView]; } +- (NSDictionary *)_diffProps:(NSDictionary *)newProps +{ + NSMutableDictionary *diffedProps = [NSMutableDictionary new]; + + [newProps enumerateKeysAndObjectsUsingBlock:^(NSString *key, id newProp, __unused BOOL *stop) { + id oldProp = _oldProps[key]; + if ([self _prop:newProp isDifferentFrom:oldProp]) { + diffedProps[key] = newProp; + } + }]; + + return diffedProps; +} + +#pragma mark - Private + +- (BOOL)_prop:(id)oldProp isDifferentFrom:(id)newProp +{ + // Check for JSON types. + // JSON types can be of: + // * number + // * bool + // * String + // * Array + // * Objects => Dictionaries in ObjectiveC + // * Null + + // Check for NULL + BOOL bothNil = !oldProp && !newProp; + if (bothNil) { + return NO; + } + + BOOL onlyOneNil = (oldProp && !newProp) || (!oldProp && newProp); + if (onlyOneNil) { + return YES; + } + + if ([self _propIsSameNumber:oldProp second:newProp]) { + // Boolean should be captured by NSNumber + return NO; + } + + if ([self _propIsSameString:oldProp second:newProp]) { + return NO; + } + + if ([self _propIsSameArray:oldProp second:newProp]) { + return NO; + } + + if ([self _propIsSameObject:oldProp second:newProp]) { + return NO; + } + + // Previous behavior, fallback to YES + return YES; +} + +- (BOOL)_propIsSameNumber:(id)first second:(id)second +{ + return [first isKindOfClass:[NSNumber class]] && [second isKindOfClass:[NSNumber class]] && + [(NSNumber *)first isEqualToNumber:(NSNumber *)second]; +} + +- (BOOL)_propIsSameString:(id)first second:(id)second +{ + return [first isKindOfClass:[NSString class]] && [second isKindOfClass:[NSString class]] && + [(NSString *)first isEqualToString:(NSString *)second]; +} + +- (BOOL)_propIsSameArray:(id)first second:(id)second +{ + return [first isKindOfClass:[NSArray class]] && [second isKindOfClass:[NSArray class]] && + [(NSArray *)first isEqualToArray:(NSArray *)second]; +} + +- (BOOL)_propIsSameObject:(id)first second:(id)second +{ + return [first isKindOfClass:[NSDictionary class]] && [second isKindOfClass:[NSDictionary class]] && + [(NSDictionary *)first isEqualToDictionary:(NSDictionary *)second]; +} + @end diff --git a/packages/react-native/ReactCommon/react/renderer/components/legacyviewmanagerinterop/RCTLegacyViewManagerInteropCoordinator.h b/packages/react-native/ReactCommon/react/renderer/components/legacyviewmanagerinterop/RCTLegacyViewManagerInteropCoordinator.h index 4d7289a1f94710..1a39901d1e2f41 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/legacyviewmanagerinterop/RCTLegacyViewManagerInteropCoordinator.h +++ b/packages/react-native/ReactCommon/react/renderer/components/legacyviewmanagerinterop/RCTLegacyViewManagerInteropCoordinator.h @@ -31,7 +31,7 @@ typedef void (^InterceptorBlock)(std::string eventName, folly::dynamic event); - (void)removeObserveForTag:(NSInteger)tag; -- (void)setProps:(const folly::dynamic &)props forView:(UIView *)view; +- (void)setProps:(NSDictionary *)props forView:(UIView *)view; - (NSString *)componentViewName; diff --git a/packages/react-native/ReactCommon/react/renderer/components/legacyviewmanagerinterop/RCTLegacyViewManagerInteropCoordinator.mm b/packages/react-native/ReactCommon/react/renderer/components/legacyviewmanagerinterop/RCTLegacyViewManagerInteropCoordinator.mm index 5424099638f374..92b53de78388f6 100644 --- a/packages/react-native/ReactCommon/react/renderer/components/legacyviewmanagerinterop/RCTLegacyViewManagerInteropCoordinator.mm +++ b/packages/react-native/ReactCommon/react/renderer/components/legacyviewmanagerinterop/RCTLegacyViewManagerInteropCoordinator.mm @@ -41,8 +41,6 @@ @implementation RCTLegacyViewManagerInteropCoordinator { */ NSMutableArray> *_moduleMethods; NSMutableDictionary> *_moduleMethodsByName; - - NSDictionary *_oldProps; } - (instancetype)initWithComponentData:(RCTComponentData *)componentData @@ -98,17 +96,12 @@ - (UIView *)createPaperViewWithTag:(NSInteger)tag; return view; } -- (void)setProps:(const folly::dynamic &)props forView:(UIView *)view +- (void)setProps:(NSDictionary *)props forView:(UIView *)view { - if (props.isObject()) { - NSDictionary *convertedProps = convertFollyDynamicToId(props); - NSDictionary *diffedProps = [self _diffProps:convertedProps]; - [_componentData setProps:diffedProps forView:view]; + [_componentData setProps:props forView:view]; - if ([view respondsToSelector:@selector(didSetProps:)]) { - [view performSelector:@selector(didSetProps:) withObject:[diffedProps allKeys]]; - } - _oldProps = convertedProps; + if ([view respondsToSelector:@selector(didSetProps:)]) { + [view performSelector:@selector(didSetProps:) withObject:[props allKeys]]; } } @@ -252,85 +245,4 @@ - (void)_lookupModuleMethodsIfNecessary } } -- (NSDictionary *)_diffProps:(NSDictionary *)newProps -{ - NSMutableDictionary *diffedProps = [NSMutableDictionary new]; - - [newProps enumerateKeysAndObjectsUsingBlock:^(NSString *key, id newProp, __unused BOOL *stop) { - id oldProp = _oldProps[key]; - if ([self _prop:newProp isDifferentFrom:oldProp]) { - diffedProps[key] = newProp; - } - }]; - - return diffedProps; -} - -- (BOOL)_prop:(id)oldProp isDifferentFrom:(id)newProp -{ - // Check for JSON types. - // JSON types can be of: - // * number - // * bool - // * String - // * Array - // * Objects => Dictionaries in ObjectiveC - // * Null - - // Check for NULL - BOOL bothNil = !oldProp && !newProp; - if (bothNil) { - return NO; - } - - BOOL onlyOneNil = (oldProp && !newProp) || (!oldProp && newProp); - if (onlyOneNil) { - return YES; - } - - if ([self _propIsSameNumber:oldProp second:newProp]) { - // Boolean should be captured by NSNumber - return NO; - } - - if ([self _propIsSameString:oldProp second:newProp]) { - return NO; - } - - if ([self _propIsSameArray:oldProp second:newProp]) { - return NO; - } - - if ([self _propIsSameObject:oldProp second:newProp]) { - return NO; - } - - // Previous behavior, fallback to YES - return YES; -} - -- (BOOL)_propIsSameNumber:(id)first second:(id)second -{ - return [first isKindOfClass:[NSNumber class]] && [second isKindOfClass:[NSNumber class]] && - [(NSNumber *)first isEqualToNumber:(NSNumber *)second]; -} - -- (BOOL)_propIsSameString:(id)first second:(id)second -{ - return [first isKindOfClass:[NSString class]] && [second isKindOfClass:[NSString class]] && - [(NSString *)first isEqualToString:(NSString *)second]; -} - -- (BOOL)_propIsSameArray:(id)first second:(id)second -{ - return [first isKindOfClass:[NSArray class]] && [second isKindOfClass:[NSArray class]] && - [(NSArray *)first isEqualToArray:(NSArray *)second]; -} - -- (BOOL)_propIsSameObject:(id)first second:(id)second -{ - return [first isKindOfClass:[NSDictionary class]] && [second isKindOfClass:[NSDictionary class]] && - [(NSDictionary *)first isEqualToDictionary:(NSDictionary *)second]; -} - @end