Skip to content

Commit 12228e3

Browse files
Nick Lefevermeta-codesync[bot]
authored andcommitted
Add runtime validation of Props 2.0 output (#54461)
Summary: Pull Request resolved: #54461 Compare the output from Props 1.5 with Props 2.0 to detect any missing props in the output coming from Props 2.0. The comparison doesn't run for initial mounts due to Props 1.5 not using the default property values to generate a diff, which is obviously leading to more properties being in the Props 1.5 output that aren't necessary. Changelog: [Internal] Reviewed By: sammy-SC, javache Differential Revision: D86577390 fbshipit-source-id: 8666fc3e5d6ddc588e7b934358eafc8a789ee491
1 parent 27b5ae2 commit 12228e3

File tree

1 file changed

+98
-3
lines changed

1 file changed

+98
-3
lines changed

packages/react-native/ReactAndroid/src/main/jni/react/fabric/FabricMountingManager.cpp

Lines changed: 98 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -24,9 +24,9 @@
2424
#include <fbjni/fbjni.h>
2525
#include <glog/logging.h>
2626

27+
#include <algorithm>
2728
#include <cfenv>
2829
#include <cmath>
29-
#include <unordered_set>
3030
#include <vector>
3131

3232
namespace facebook::react {
@@ -54,6 +54,80 @@ void FabricMountingManager::onSurfaceStop(SurfaceId surfaceId) {
5454

5555
namespace {
5656

57+
#ifdef REACT_NATIVE_DEBUG
58+
// List of layout-only props extracted from ViewProps.kt used to filter out
59+
// component props from Props 1.5 to validate the Props 2.0 output
60+
inline bool isLayoutOnlyProp(const std::string& propName) {
61+
static const std::vector<std::string> layoutOnlyProps = {
62+
// Flexbox Alignment
63+
"alignItems",
64+
"alignSelf",
65+
"alignContent",
66+
67+
// Flexbox Properties
68+
"flex",
69+
"flexBasis",
70+
"flexDirection",
71+
"flexGrow",
72+
"flexShrink",
73+
"flexWrap",
74+
"justifyContent",
75+
76+
// Gaps
77+
"rowGap",
78+
"columnGap",
79+
"gap",
80+
81+
// Display & Position
82+
"display",
83+
"position",
84+
85+
// Positioning
86+
"right",
87+
"top",
88+
"bottom",
89+
"left",
90+
"start",
91+
"end",
92+
93+
// Dimensions
94+
"width",
95+
"height",
96+
"minWidth",
97+
"maxWidth",
98+
"minHeight",
99+
"maxHeight",
100+
101+
// Margins
102+
"margin",
103+
"marginVertical",
104+
"marginHorizontal",
105+
"marginLeft",
106+
"marginRight",
107+
"marginTop",
108+
"marginBottom",
109+
"marginStart",
110+
"marginEnd",
111+
112+
// Paddings
113+
"padding",
114+
"paddingVertical",
115+
"paddingHorizontal",
116+
"paddingLeft",
117+
"paddingRight",
118+
"paddingTop",
119+
"paddingBottom",
120+
"paddingStart",
121+
"paddingEnd",
122+
123+
// Other
124+
"collapsable",
125+
};
126+
return std::find(layoutOnlyProps.begin(), layoutOnlyProps.end(), propName) !=
127+
layoutOnlyProps.end();
128+
}
129+
#endif
130+
57131
inline int getIntBufferSizeForType(CppMountItem::Type mountItemType) {
58132
switch (mountItemType) {
59133
case CppMountItem::Type::Create:
@@ -232,8 +306,29 @@ jni::local_ref<jobject> getProps(
232306
strcmp(
233307
newShadowView.componentName,
234308
newProps->getDiffPropsImplementationTarget()) == 0) {
235-
return ReadableNativeMap::newObjectCxxArgs(
236-
newProps->getDiffProps(oldProps));
309+
auto diff = newProps->getDiffProps(oldProps);
310+
311+
#ifdef REACT_NATIVE_DEBUG
312+
if (oldProps != nullptr) {
313+
auto controlDiff =
314+
diffDynamicProps(oldProps->rawProps, newProps->rawProps);
315+
316+
for (const auto& [prop, value] : controlDiff.items()) {
317+
if (diff.count(prop) == 0) {
318+
// Skip layout-only props since they are not included in Props 2.0
319+
if (!isLayoutOnlyProp(prop.asString())) {
320+
LOG(ERROR) << "Props diff validation failed: Props 1.5 has prop '"
321+
<< prop.asString()
322+
<< "' = " << (value != nullptr ? value : "NULL")
323+
<< " that Props 2.0 doesn't have for component "
324+
<< newShadowView.componentName;
325+
}
326+
}
327+
}
328+
}
329+
#endif
330+
331+
return ReadableNativeMap::newObjectCxxArgs(std::move(diff));
237332
}
238333
if (ReactNativeFeatureFlags::enableAccumulatedUpdatesInRawPropsAndroid()) {
239334
if (oldProps == nullptr) {

0 commit comments

Comments
 (0)