From 4c3112c8d8685d6c34be9acf07b18871b3cee5b2 Mon Sep 17 00:00:00 2001 From: Riccardo Cipolleschi Date: Tue, 19 Nov 2024 09:54:27 -0800 Subject: [PATCH] Do not discard props when setNativeProps is used (#47669) Summary: Pull Request resolved: https://github.com/facebook/react-native/pull/47669 When investigating [#47476](https://github.com/facebook/react-native/issues/47476), I found that the `secureTextInput` prop was not changing in the Mounting layer when changing it in JS. I track down the problem to the `UIManager::cloneNode` method. When we clone the node, we first merge the patch that arrives from React into the props controlled by setNativeProps, ignoring the patch's props that are controlled by React. But then, we forgot to merge back the React's controlled property into the final props, effectively losing them. This change adds an extra merging step, merging the props controlled with setNativeProps back into the patch of props controlled by React, and then using this new set of props as source of truth. ## Changelog: [General][Fixed] - do not discard props in the patch when they are not null while using `useNativeProps` Reviewed By: sammy-SC Differential Revision: D65948574 fbshipit-source-id: db4f2b793f4a6348456933c95a151012252b8ebc --- .../react/renderer/uimanager/UIManager.cpp | 37 +++++++++++++++---- 1 file changed, 29 insertions(+), 8 deletions(-) diff --git a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp index c381d77e375..9b9fe24cbe7 100644 --- a/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp +++ b/packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp @@ -126,20 +126,41 @@ std::shared_ptr UIManager::cloneNode( if (!rawProps.isEmpty()) { if (family.nativeProps_DEPRECATED != nullptr) { + // 1. update the nativeProps_DEPRECATED props. + // + // In this step, we want the most recent value for the props + // managed by setNativeProps. // Values in `rawProps` patch (take precedence over) - // `nativeProps_DEPRECATED`. For example, if both `nativeProps_DEPRECATED` - // and `rawProps` contain key 'A'. Value from `rawProps` overrides what - // was previously in `nativeProps_DEPRECATED`. + // `nativeProps_DEPRECATED`. For example, if both + // `nativeProps_DEPRECATED` and `rawProps` contain key 'A'. + // Value from `rawProps` overrides what was previously in + // `nativeProps_DEPRECATED`. Notice that the `nativeProps_DEPRECATED` + // patch will not get more props from `rawProps`: if the key is not + // present in `nativeProps_DEPRECATED`, it will not be added. + // + // The result of this operation is the new `nativeProps_DEPRECATED`. family.nativeProps_DEPRECATED = std::make_unique(mergeDynamicProps( - *family.nativeProps_DEPRECATED, - (folly::dynamic)rawProps, + *family.nativeProps_DEPRECATED, // source + (folly::dynamic)rawProps, // patch NullValueStrategy::Ignore)); + // 2. Compute the final set of props. + // + // This step takes the new props handled by `setNativeProps` and + // merges them in the `rawProps` managed by React. + // The new props handled by `nativeProps` now takes precedence + // on the props handled by React, as we want to make sure that + // all the props are applied to the component. + // We use these finalProps as source of truth for the component. + auto finalProps = mergeDynamicProps( + (folly::dynamic)rawProps, // source + *family.nativeProps_DEPRECATED, // patch + NullValueStrategy::Override); + + // 3. Clone the props by using finalProps. props = componentDescriptor.cloneProps( - propsParserContext, - shadowNode.getProps(), - RawProps(*family.nativeProps_DEPRECATED)); + propsParserContext, shadowNode.getProps(), RawProps(finalProps)); } else { props = componentDescriptor.cloneProps( propsParserContext, shadowNode.getProps(), std::move(rawProps));