Skip to content

Commit

Permalink
Do not discard props when setNativeProps is used (#47669)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #47669

When investigating [#47476](#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
  • Loading branch information
cipolleschi authored and facebook-github-bot committed Nov 19, 2024
1 parent 35ab62c commit 4c3112c
Showing 1 changed file with 29 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -126,20 +126,41 @@ std::shared_ptr<ShadowNode> 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<folly::dynamic>(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));
Expand Down

0 comments on commit 4c3112c

Please sign in to comment.