Skip to content

Commit 881942a

Browse files
sammy-SCfacebook-github-bot
authored andcommitted
avoid merging unrelated props into ShadowNodeFamily::nativeProps (#43305)
Summary: Pull Request resolved: #43305 changelog: [internal] Originally when I built setNativeProps, I assumed a node is either controlled by handled directly or it is controlled by React. But we can't make sure that's the case, users can do both and control one prop with setNativeProps and the others with React. Additionally, Suspense uses display: none to hide a subtree. Therefore, React controlled props must not be copied into `ShadowNodeFamily::nativeProps_DEPRECATED` Reviewed By: javache Differential Revision: D54453820 fbshipit-source-id: 5b4038f0dd366621d26a92f668d33f27ce60f4b4
1 parent f57be12 commit 881942a

File tree

6 files changed

+67
-14
lines changed

6 files changed

+67
-14
lines changed

packages/react-native/ReactCommon/react/renderer/components/legacyviewmanagerinterop/LegacyViewManagerInteropViewProps.cpp

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,9 @@ LegacyViewManagerInteropViewProps::LegacyViewManagerInteropViewProps(
1515
const LegacyViewManagerInteropViewProps& sourceProps,
1616
const RawProps& rawProps)
1717
: ViewProps(context, sourceProps, rawProps),
18-
otherProps(
19-
mergeDynamicProps(sourceProps.otherProps, (folly::dynamic)rawProps)) {
20-
}
18+
otherProps(mergeDynamicProps(
19+
sourceProps.otherProps,
20+
(folly::dynamic)rawProps,
21+
NullValueStrategy::Override)) {}
2122

2223
} // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/core/DynamicPropsUtilities.cpp

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,8 @@ namespace facebook::react {
1111

1212
folly::dynamic mergeDynamicProps(
1313
const folly::dynamic& source,
14-
const folly::dynamic& patch) {
14+
const folly::dynamic& patch,
15+
NullValueStrategy nullValueStrategy) {
1516
auto result = source;
1617

1718
if (!result.isObject()) {
@@ -25,6 +26,10 @@ folly::dynamic mergeDynamicProps(
2526
// Note, here we have to preserve sub-prop objects with `null` value as
2627
// an indication for the legacy mounting layer that it needs to clean them up.
2728
for (const auto& pair : patch.items()) {
29+
if (nullValueStrategy == NullValueStrategy::Ignore &&
30+
source.find(pair.first) == source.items().end()) {
31+
continue;
32+
}
2833
result[pair.first] = pair.second;
2934
}
3035

packages/react-native/ReactCommon/react/renderer/core/DynamicPropsUtilities.h

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -12,13 +12,40 @@
1212

1313
namespace facebook::react {
1414

15+
/*
16+
* Enum defining how missing value in `source` is handled if it is present in
17+
* `patch`.
18+
*/
19+
enum class NullValueStrategy {
20+
/*
21+
* Key in source will be overriden by the matching key in patch.
22+
*
23+
* Example:
24+
* source: {"key": "value"}
25+
* patch: {"key": "new value"}
26+
* returned: {"key": "new value"}
27+
*/
28+
Override,
29+
30+
/*
31+
* In case key is missing in source, value from patch will be ignored.
32+
*
33+
* Example:
34+
* source: {"key 1": "value 1"}
35+
* patch: {"key": "new value 1", "key 2": "new value 2"}
36+
* returned: {"key": "new value 1"}
37+
*/
38+
Ignore
39+
};
40+
1541
/*
1642
* Accepts two `folly::dynamic` objects as arguments. Both arguments need to
1743
* represent a dictionary. It updates `source` with key/value pairs from
18-
* `patch`, overriding existing keys.
44+
* `patch`.
1945
*/
2046
folly::dynamic mergeDynamicProps(
2147
const folly::dynamic& source,
22-
const folly::dynamic& patch);
48+
const folly::dynamic& patch,
49+
NullValueStrategy nullValueStrategy);
2350

2451
} // namespace facebook::react

packages/react-native/ReactCommon/react/renderer/core/ShadowNode.cpp

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,7 +42,9 @@ Props::Shared ShadowNode::propsForClonedShadowNode(
4242
if (!hasBeenMounted && sourceNodeHasRawProps && props) {
4343
auto& castedProps = const_cast<Props&>(*props);
4444
castedProps.rawProps = mergeDynamicProps(
45-
sourceShadowNode.getProps()->rawProps, props->rawProps);
45+
sourceShadowNode.getProps()->rawProps,
46+
props->rawProps,
47+
NullValueStrategy::Override);
4648
return props;
4749
}
4850
#endif

packages/react-native/ReactCommon/react/renderer/core/tests/DynamicPropsUtilitiesTest.cpp

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,7 +24,7 @@ TEST(DynamicPropsUtilitiesTest, handleNestedObjects) {
2424
map2["style"] = dynamic::object("backgroundColor", "blue")("color", "black");
2525
map2["height"] = 100;
2626

27-
auto result = mergeDynamicProps(map1, map2);
27+
auto result = mergeDynamicProps(map1, map2, NullValueStrategy::Override);
2828

2929
EXPECT_TRUE(result["style"].isObject());
3030
EXPECT_TRUE(result["style"]["backgroundColor"].isString());
@@ -42,25 +42,39 @@ TEST(DynamicPropsUtilitiesTest, handleEmptyObject) {
4242
dynamic map2 = dynamic::object;
4343
map2["height"] = 100;
4444

45-
auto result = mergeDynamicProps(map1, map2);
45+
auto result = mergeDynamicProps(map1, map2, NullValueStrategy::Override);
4646

4747
EXPECT_TRUE(result["height"].isInt());
4848
EXPECT_EQ(result["height"], 100);
4949

50-
result = mergeDynamicProps(map1, map2);
50+
result = mergeDynamicProps(map1, map2, NullValueStrategy::Override);
5151

5252
EXPECT_TRUE(result["height"].isInt());
5353
EXPECT_EQ(result["height"], 100);
5454
}
5555

56-
TEST(DynamicPropsUtilitiesTest, handleNull) {
56+
TEST(DynamicPropsUtilitiesTest, handleNullValue) {
5757
dynamic map1 = dynamic::object;
5858
map1["height"] = 100;
5959

6060
dynamic map2 = dynamic::object;
6161
map2["height"] = nullptr;
6262

63-
auto result = mergeDynamicProps(map1, map2);
63+
auto result = mergeDynamicProps(map1, map2, NullValueStrategy::Override);
6464

6565
EXPECT_TRUE(result["height"].isNull());
6666
}
67+
68+
TEST(DynamicPropsUtilitiesTest, testNullValueStrategyIgnore) {
69+
dynamic map1 = dynamic::object;
70+
map1["height"] = 100;
71+
72+
dynamic map2 = dynamic::object;
73+
map2["width"] = 200;
74+
map2["height"] = 101;
75+
76+
auto result = mergeDynamicProps(map1, map2, NullValueStrategy::Ignore);
77+
78+
EXPECT_EQ(result["height"], 101);
79+
EXPECT_TRUE(result["width"].isNull());
80+
}

packages/react-native/ReactCommon/react/renderer/uimanager/UIManager.cpp

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -129,7 +129,9 @@ std::shared_ptr<ShadowNode> UIManager::cloneNode(
129129
// was previously in `nativeProps_DEPRECATED`.
130130
family.nativeProps_DEPRECATED =
131131
std::make_unique<folly::dynamic>(mergeDynamicProps(
132-
*family.nativeProps_DEPRECATED, (folly::dynamic)rawProps));
132+
*family.nativeProps_DEPRECATED,
133+
(folly::dynamic)rawProps,
134+
NullValueStrategy::Ignore));
133135

134136
props = componentDescriptor.cloneProps(
135137
propsParserContext,
@@ -513,7 +515,9 @@ void UIManager::setNativeProps_DEPRECATED(
513515
// previously in `nativeProps_DEPRECATED`.
514516
family.nativeProps_DEPRECATED =
515517
std::make_unique<folly::dynamic>(mergeDynamicProps(
516-
*family.nativeProps_DEPRECATED, (folly::dynamic)rawProps));
518+
*family.nativeProps_DEPRECATED,
519+
(folly::dynamic)rawProps,
520+
NullValueStrategy::Override));
517521
} else {
518522
family.nativeProps_DEPRECATED =
519523
std::make_unique<folly::dynamic>((folly::dynamic)rawProps);

0 commit comments

Comments
 (0)