Skip to content

Commit dff97ed

Browse files
sammy-SCfacebook-github-bot
sammy-SC
authored and
facebook-github-bot
committed
filter out yoga style props during folly::dynamic conversion (#41607)
Summary: X-link: facebook/hermes#1198 changelog: [internal] Reviewed By: javache Differential Revision: D51471665
1 parent 1e68e48 commit dff97ed

File tree

7 files changed

+169
-125
lines changed

7 files changed

+169
-125
lines changed

‎packages/react-native/ReactCommon/jsi/jsi/JSIDynamic.cpp

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,10 @@ void dynamicFromValueShallow(
150150

151151
} // namespace
152152

153-
folly::dynamic dynamicFromValue(Runtime& runtime, const Value& valueInput) {
153+
folly::dynamic dynamicFromValue(
154+
Runtime& runtime,
155+
const Value& valueInput,
156+
std::function<bool(const std::string&)> filterObjectKeys) {
154157
std::vector<FromValue> stack;
155158
folly::dynamic ret;
156159

@@ -182,13 +185,17 @@ folly::dynamic dynamicFromValue(Runtime& runtime, const Value& valueInput) {
182185
if (prop.isUndefined()) {
183186
continue;
184187
}
188+
auto nameStr = name.utf8(runtime);
189+
if (filterObjectKeys && filterObjectKeys(nameStr)) {
190+
continue;
191+
}
185192
// The JSC conversion uses JSON.stringify, which substitutes
186193
// null for a function, so we do the same here. Just dropping
187194
// the pair might also work, but would require more testing.
188195
if (prop.isObject() && prop.getObject(runtime).isFunction(runtime)) {
189196
prop = Value::null();
190197
}
191-
props.emplace_back(name.utf8(runtime), std::move(prop));
198+
props.emplace_back(std::move(nameStr), std::move(prop));
192199
top.dyn->insert(props.back().first, nullptr);
193200
}
194201
for (const auto& prop : props) {

‎packages/react-native/ReactCommon/jsi/jsi/JSIDynamic.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,8 @@ facebook::jsi::Value valueFromDynamic(
1919

2020
folly::dynamic dynamicFromValue(
2121
facebook::jsi::Runtime& runtime,
22-
const facebook::jsi::Value& value);
22+
const facebook::jsi::Value& value,
23+
std::function<bool(const std::string&)> filterObjectKeys = nullptr);
2324

2425
} // namespace jsi
2526
} // namespace facebook

‎packages/react-native/ReactCommon/react/renderer/components/view/YogaStylableProps.cpp

Lines changed: 6 additions & 121 deletions
Original file line numberDiff line numberDiff line change
@@ -19,134 +19,19 @@
1919

2020
namespace facebook::react {
2121

22-
namespace {
23-
inline RawProps filterYogaProps(const RawProps& rawProps) {
24-
const static std::unordered_set<std::string> yogaStylePropNames = {
25-
{"direction",
26-
"flexDirection",
27-
"justifyContent",
28-
"alignContent",
29-
"alignItems",
30-
"alignSelf",
31-
"position",
32-
"flexWrap",
33-
"display",
34-
"flex",
35-
"flexGrow",
36-
"flexShrink",
37-
"flexBasis",
38-
"margin",
39-
"padding",
40-
"rowGap",
41-
"columnGap",
42-
"gap",
43-
// TODO: T163711275 also filter out width/height when SVG no longer read
44-
// them from RawProps
45-
"minWidth",
46-
"maxWidth",
47-
"minHeight",
48-
"maxHeight",
49-
"aspectRatio",
50-
51-
// edges
52-
"left",
53-
"right",
54-
"top",
55-
"bottom",
56-
"start",
57-
"end",
58-
59-
// variants of inset
60-
"inset",
61-
"insetStart",
62-
"insetEnd",
63-
"insetInline",
64-
"insetInlineStart",
65-
"insetInlineEnd",
66-
"insetBlock",
67-
"insetBlockEnd",
68-
"insetBlockStart",
69-
"insetVertical",
70-
"insetHorizontal",
71-
"insetTop",
72-
"insetBottom",
73-
"insetLeft",
74-
"insetRight",
75-
76-
// variants of margin
77-
"marginStart",
78-
"marginEnd",
79-
"marginInline",
80-
"marginInlineStart",
81-
"marginInlineEnd",
82-
"marginBlock",
83-
"marginBlockStart",
84-
"marginBlockEnd",
85-
"marginVertical",
86-
"marginHorizontal",
87-
"marginTop",
88-
"marginBottom",
89-
"marginLeft",
90-
"marginRight",
91-
92-
// variants of padding
93-
"paddingStart",
94-
"paddingEnd",
95-
"paddingInline",
96-
"paddingInlineStart",
97-
"paddingInlineEnd",
98-
"paddingBlock",
99-
"paddingBlockStart",
100-
"paddingBlockEnd",
101-
"paddingVertical",
102-
"paddingHorizontal",
103-
"paddingTop",
104-
"paddingBottom",
105-
"paddingLeft",
106-
"paddingRight"}};
107-
108-
auto filteredRawProps = (folly::dynamic)rawProps;
109-
110-
auto it = filteredRawProps.items().begin();
111-
while (it != filteredRawProps.items().end()) {
112-
auto key = it->first.asString();
113-
if (yogaStylePropNames.find(key) != yogaStylePropNames.end()) {
114-
it = filteredRawProps.erase(it);
115-
} else {
116-
++it;
117-
}
118-
}
119-
120-
return RawProps(std::move(filteredRawProps));
121-
}
122-
} // namespace
123-
12422
YogaStylableProps::YogaStylableProps(
12523
const PropsParserContext& context,
12624
const YogaStylableProps& sourceProps,
12725
const RawProps& rawProps)
12826
: Props() {
129-
if (CoreFeatures::excludeYogaFromRawProps) {
130-
const auto filteredRawProps = filterYogaProps(rawProps);
131-
initialize(context, sourceProps, filteredRawProps);
132-
133-
yogaStyle = CoreFeatures::enablePropIteratorSetter
134-
? sourceProps.yogaStyle
135-
: convertRawProp(context, filteredRawProps, sourceProps.yogaStyle);
136-
137-
if (!CoreFeatures::enablePropIteratorSetter) {
138-
convertRawPropAliases(context, sourceProps, filteredRawProps);
139-
}
140-
} else {
141-
initialize(context, sourceProps, rawProps);
27+
initialize(context, sourceProps, rawProps);
14228

143-
yogaStyle = CoreFeatures::enablePropIteratorSetter
144-
? sourceProps.yogaStyle
145-
: convertRawProp(context, rawProps, sourceProps.yogaStyle);
29+
yogaStyle = CoreFeatures::enablePropIteratorSetter
30+
? sourceProps.yogaStyle
31+
: convertRawProp(context, rawProps, sourceProps.yogaStyle);
14632

147-
if (!CoreFeatures::enablePropIteratorSetter) {
148-
convertRawPropAliases(context, sourceProps, rawProps);
149-
}
33+
if (!CoreFeatures::enablePropIteratorSetter) {
34+
convertRawPropAliases(context, sourceProps, rawProps);
15035
}
15136
};
15237

‎packages/react-native/ReactCommon/react/renderer/core/ConcreteComponentDescriptor.h

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -105,6 +105,13 @@ class ConcreteComponentDescriptor : public ComponentDescriptor {
105105
return ShadowNodeT::defaultSharedProps();
106106
}
107107

108+
if (CoreFeatures::excludeYogaFromRawProps) {
109+
if (ShadowNodeT::IdentifierTrait() ==
110+
ShadowNodeTraits::Trait::YogaLayoutableKind) {
111+
rawProps.filterYogaStylePropsInDynamicConversion();
112+
}
113+
}
114+
108115
rawProps.parse(rawPropsParser_);
109116

110117
// Call old-style constructor

‎packages/react-native/ReactCommon/react/renderer/core/RawProps.cpp

Lines changed: 98 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,96 @@
1313

1414
namespace facebook::react {
1515

16+
namespace {
17+
inline bool isYogaStyleProp(const std::string& prop) {
18+
const static std::unordered_set<std::string> yogaStylePropNames = {
19+
{"direction",
20+
"flexDirection",
21+
"justifyContent",
22+
"alignContent",
23+
"alignItems",
24+
"alignSelf",
25+
"position",
26+
"flexWrap",
27+
"display",
28+
"flex",
29+
"flexGrow",
30+
"flexShrink",
31+
"flexBasis",
32+
"margin",
33+
"padding",
34+
"rowGap",
35+
"columnGap",
36+
"gap",
37+
// TODO: T163711275 also filter out width/height when SVG no longer read
38+
// them from RawProps
39+
"minWidth",
40+
"maxWidth",
41+
"minHeight",
42+
"maxHeight",
43+
"aspectRatio",
44+
45+
// edges
46+
"left",
47+
"right",
48+
"top",
49+
"bottom",
50+
"start",
51+
"end",
52+
53+
// variants of inset
54+
"inset",
55+
"insetStart",
56+
"insetEnd",
57+
"insetInline",
58+
"insetInlineStart",
59+
"insetInlineEnd",
60+
"insetBlock",
61+
"insetBlockEnd",
62+
"insetBlockStart",
63+
"insetVertical",
64+
"insetHorizontal",
65+
"insetTop",
66+
"insetBottom",
67+
"insetLeft",
68+
"insetRight",
69+
70+
// variants of margin
71+
"marginStart",
72+
"marginEnd",
73+
"marginInline",
74+
"marginInlineStart",
75+
"marginInlineEnd",
76+
"marginBlock",
77+
"marginBlockStart",
78+
"marginBlockEnd",
79+
"marginVertical",
80+
"marginHorizontal",
81+
"marginTop",
82+
"marginBottom",
83+
"marginLeft",
84+
"marginRight",
85+
86+
// variants of padding
87+
"paddingStart",
88+
"paddingEnd",
89+
"paddingInline",
90+
"paddingInlineStart",
91+
"paddingInlineEnd",
92+
"paddingBlock",
93+
"paddingBlockStart",
94+
"paddingBlockEnd",
95+
"paddingVertical",
96+
"paddingHorizontal",
97+
"paddingTop",
98+
"paddingBottom",
99+
"paddingLeft",
100+
"paddingRight"}};
101+
102+
return yogaStylePropNames.find(prop) != yogaStylePropNames.end();
103+
}
104+
} // namespace
105+
16106
RawProps::RawProps() {
17107
mode_ = Mode::Empty;
18108
}
@@ -55,6 +145,7 @@ RawProps::RawProps(const RawProps& other) noexcept {
55145
} else if (mode_ == Mode::Dynamic) {
56146
dynamic_ = other.dynamic_;
57147
}
148+
ignoreYogaStyleProps_ = other.ignoreYogaStyleProps_;
58149
}
59150

60151
RawProps& RawProps::operator=(const RawProps& other) noexcept {
@@ -65,6 +156,7 @@ RawProps& RawProps::operator=(const RawProps& other) noexcept {
65156
} else if (mode_ == Mode::Dynamic) {
66157
dynamic_ = other.dynamic_;
67158
}
159+
ignoreYogaStyleProps_ = other.ignoreYogaStyleProps_;
68160
return *this;
69161
}
70162

@@ -84,12 +176,17 @@ RawProps::operator folly::dynamic() const noexcept {
84176
case Mode::Empty:
85177
return folly::dynamic::object();
86178
case Mode::JSI:
87-
return jsi::dynamicFromValue(*runtime_, value_);
179+
return jsi::dynamicFromValue(
180+
*runtime_, value_, ignoreYogaStyleProps_ ? isYogaStyleProp : nullptr);
88181
case Mode::Dynamic:
89182
return dynamic_;
90183
}
91184
}
92185

186+
void RawProps::filterYogaStylePropsInDynamicConversion() noexcept {
187+
ignoreYogaStyleProps_ = true;
188+
}
189+
93190
/*
94191
* Returns `true` if the object is empty.
95192
* Empty `RawProps` does not have any stored data.

‎packages/react-native/ReactCommon/react/renderer/core/RawProps.h

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -73,6 +73,15 @@ class RawProps final {
7373
*/
7474
explicit operator folly::dynamic() const noexcept;
7575

76+
/*
77+
* Once called, Yoga style props will be filtered out during conversion to
78+
* folly::dynamic. folly::dynamic conversion is only used on Android and props
79+
* specific to Yoga do not need to be send over JNI to Android.
80+
* This is a performance optimisation to minimise traffic between C++ and
81+
* Java.
82+
*/
83+
void filterYogaStylePropsInDynamicConversion() noexcept;
84+
7685
/*
7786
* Returns `true` if the object is empty.
7887
* Empty `RawProps` does not have any stored data.
@@ -124,6 +133,8 @@ class RawProps final {
124133
*/
125134
mutable std::vector<RawPropsValueIndex> keyIndexToValueIndex_;
126135
mutable std::vector<RawValue> values_;
136+
137+
bool ignoreYogaStyleProps_{false};
127138
};
128139

129140
} // namespace facebook::react

‎packages/react-native/ReactCommon/react/renderer/core/tests/RawPropsTest.cpp

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -467,3 +467,39 @@ TEST(RawPropsTest, copyJSIRawProps) {
467467
EXPECT_NEAR(
468468
copyProps->derivedFloatValue, originalProps->derivedFloatValue, 0.00001);
469469
}
470+
471+
TEST(RawPropsTest, filterYogaRawProps) {
472+
auto runtime = facebook::hermes::makeHermesRuntime();
473+
474+
ContextContainer contextContainer{};
475+
PropsParserContext parserContext{-1, contextContainer};
476+
477+
auto object = jsi::Object(*runtime);
478+
object.setProperty(*runtime, "floatValue", 10.0);
479+
object.setProperty(*runtime, "flex", 1);
480+
481+
auto rawProps = RawProps(*runtime, jsi::Value(*runtime, object));
482+
483+
EXPECT_FALSE(rawProps.isEmpty());
484+
485+
auto dynamicProps = (folly::dynamic)rawProps;
486+
487+
EXPECT_EQ(dynamicProps["floatValue"], 10.0);
488+
EXPECT_EQ(dynamicProps["flex"], 1);
489+
490+
rawProps.filterYogaStylePropsInDynamicConversion();
491+
492+
dynamicProps = (folly::dynamic)rawProps;
493+
494+
EXPECT_EQ(dynamicProps["floatValue"], 10.0);
495+
EXPECT_EQ(dynamicProps["flex"], nullptr);
496+
497+
// The fact that filterYogaStylePropsInDynamicConversion should
498+
// must apply to a copy as well.
499+
auto copy = RawProps(rawProps);
500+
501+
auto dynamicPropsFromCopy = (folly::dynamic)copy;
502+
503+
EXPECT_EQ(dynamicPropsFromCopy["floatValue"], 10.0);
504+
EXPECT_EQ(dynamicPropsFromCopy["flex"], nullptr);
505+
}

0 commit comments

Comments
 (0)