Skip to content

filter out yoga style props during folly::dynamic conversion #41607

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions packages/react-native/ReactCommon/jsi/jsi/JSIDynamic.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -150,7 +150,10 @@ void dynamicFromValueShallow(

} // namespace

folly::dynamic dynamicFromValue(Runtime& runtime, const Value& valueInput) {
folly::dynamic dynamicFromValue(
Runtime& runtime,
const Value& valueInput,
std::function<bool(const std::string&)> filterObjectKeys) {
std::vector<FromValue> stack;
folly::dynamic ret;

Expand Down Expand Up @@ -182,13 +185,17 @@ folly::dynamic dynamicFromValue(Runtime& runtime, const Value& valueInput) {
if (prop.isUndefined()) {
continue;
}
auto nameStr = name.utf8(runtime);
if (filterObjectKeys && filterObjectKeys(nameStr)) {
continue;
}
// The JSC conversion uses JSON.stringify, which substitutes
// null for a function, so we do the same here. Just dropping
// the pair might also work, but would require more testing.
if (prop.isObject() && prop.getObject(runtime).isFunction(runtime)) {
prop = Value::null();
}
props.emplace_back(name.utf8(runtime), std::move(prop));
props.emplace_back(std::move(nameStr), std::move(prop));
top.dyn->insert(props.back().first, nullptr);
}
for (const auto& prop : props) {
Expand Down
3 changes: 2 additions & 1 deletion packages/react-native/ReactCommon/jsi/jsi/JSIDynamic.h
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ facebook::jsi::Value valueFromDynamic(

folly::dynamic dynamicFromValue(
facebook::jsi::Runtime& runtime,
const facebook::jsi::Value& value);
const facebook::jsi::Value& value,
std::function<bool(const std::string&)> filterObjectKeys = nullptr);

} // namespace jsi
} // namespace facebook
Original file line number Diff line number Diff line change
Expand Up @@ -19,134 +19,19 @@

namespace facebook::react {

namespace {
inline RawProps filterYogaProps(const RawProps& rawProps) {
const static std::unordered_set<std::string> yogaStylePropNames = {
{"direction",
"flexDirection",
"justifyContent",
"alignContent",
"alignItems",
"alignSelf",
"position",
"flexWrap",
"display",
"flex",
"flexGrow",
"flexShrink",
"flexBasis",
"margin",
"padding",
"rowGap",
"columnGap",
"gap",
// TODO: T163711275 also filter out width/height when SVG no longer read
// them from RawProps
"minWidth",
"maxWidth",
"minHeight",
"maxHeight",
"aspectRatio",

// edges
"left",
"right",
"top",
"bottom",
"start",
"end",

// variants of inset
"inset",
"insetStart",
"insetEnd",
"insetInline",
"insetInlineStart",
"insetInlineEnd",
"insetBlock",
"insetBlockEnd",
"insetBlockStart",
"insetVertical",
"insetHorizontal",
"insetTop",
"insetBottom",
"insetLeft",
"insetRight",

// variants of margin
"marginStart",
"marginEnd",
"marginInline",
"marginInlineStart",
"marginInlineEnd",
"marginBlock",
"marginBlockStart",
"marginBlockEnd",
"marginVertical",
"marginHorizontal",
"marginTop",
"marginBottom",
"marginLeft",
"marginRight",

// variants of padding
"paddingStart",
"paddingEnd",
"paddingInline",
"paddingInlineStart",
"paddingInlineEnd",
"paddingBlock",
"paddingBlockStart",
"paddingBlockEnd",
"paddingVertical",
"paddingHorizontal",
"paddingTop",
"paddingBottom",
"paddingLeft",
"paddingRight"}};

auto filteredRawProps = (folly::dynamic)rawProps;

auto it = filteredRawProps.items().begin();
while (it != filteredRawProps.items().end()) {
auto key = it->first.asString();
if (yogaStylePropNames.find(key) != yogaStylePropNames.end()) {
it = filteredRawProps.erase(it);
} else {
++it;
}
}

return RawProps(std::move(filteredRawProps));
}
} // namespace

YogaStylableProps::YogaStylableProps(
const PropsParserContext& context,
const YogaStylableProps& sourceProps,
const RawProps& rawProps)
: Props() {
if (CoreFeatures::excludeYogaFromRawProps) {
const auto filteredRawProps = filterYogaProps(rawProps);
initialize(context, sourceProps, filteredRawProps);

yogaStyle = CoreFeatures::enablePropIteratorSetter
? sourceProps.yogaStyle
: convertRawProp(context, filteredRawProps, sourceProps.yogaStyle);

if (!CoreFeatures::enablePropIteratorSetter) {
convertRawPropAliases(context, sourceProps, filteredRawProps);
}
} else {
initialize(context, sourceProps, rawProps);
initialize(context, sourceProps, rawProps);

yogaStyle = CoreFeatures::enablePropIteratorSetter
? sourceProps.yogaStyle
: convertRawProp(context, rawProps, sourceProps.yogaStyle);
yogaStyle = CoreFeatures::enablePropIteratorSetter
? sourceProps.yogaStyle
: convertRawProp(context, rawProps, sourceProps.yogaStyle);

if (!CoreFeatures::enablePropIteratorSetter) {
convertRawPropAliases(context, sourceProps, rawProps);
}
if (!CoreFeatures::enablePropIteratorSetter) {
convertRawPropAliases(context, sourceProps, rawProps);
}
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,13 @@ class ConcreteComponentDescriptor : public ComponentDescriptor {
return ShadowNodeT::defaultSharedProps();
}

if (CoreFeatures::excludeYogaFromRawProps) {
if (ShadowNodeT::IdentifierTrait() ==
ShadowNodeTraits::Trait::YogaLayoutableKind) {
rawProps.filterYogaStylePropsInDynamicConversion();
}
}

rawProps.parse(rawPropsParser_);

// Call old-style constructor
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,96 @@

namespace facebook::react {

namespace {
inline bool isYogaStyleProp(const std::string& prop) {
const static std::unordered_set<std::string> yogaStylePropNames = {
{"direction",
"flexDirection",
"justifyContent",
"alignContent",
"alignItems",
"alignSelf",
"position",
"flexWrap",
"display",
"flex",
"flexGrow",
"flexShrink",
"flexBasis",
"margin",
"padding",
"rowGap",
"columnGap",
"gap",
// TODO: T163711275 also filter out width/height when SVG no longer read
// them from RawProps
"minWidth",
"maxWidth",
"minHeight",
"maxHeight",
"aspectRatio",

// edges
"left",
"right",
"top",
"bottom",
"start",
"end",

// variants of inset
"inset",
"insetStart",
"insetEnd",
"insetInline",
"insetInlineStart",
"insetInlineEnd",
"insetBlock",
"insetBlockEnd",
"insetBlockStart",
"insetVertical",
"insetHorizontal",
"insetTop",
"insetBottom",
"insetLeft",
"insetRight",

// variants of margin
"marginStart",
"marginEnd",
"marginInline",
"marginInlineStart",
"marginInlineEnd",
"marginBlock",
"marginBlockStart",
"marginBlockEnd",
"marginVertical",
"marginHorizontal",
"marginTop",
"marginBottom",
"marginLeft",
"marginRight",

// variants of padding
"paddingStart",
"paddingEnd",
"paddingInline",
"paddingInlineStart",
"paddingInlineEnd",
"paddingBlock",
"paddingBlockStart",
"paddingBlockEnd",
"paddingVertical",
"paddingHorizontal",
"paddingTop",
"paddingBottom",
"paddingLeft",
"paddingRight"}};

return yogaStylePropNames.find(prop) != yogaStylePropNames.end();
}
} // namespace

RawProps::RawProps() {
mode_ = Mode::Empty;
}
Expand Down Expand Up @@ -55,6 +145,7 @@ RawProps::RawProps(const RawProps& other) noexcept {
} else if (mode_ == Mode::Dynamic) {
dynamic_ = other.dynamic_;
}
ignoreYogaStyleProps_ = other.ignoreYogaStyleProps_;
}

RawProps& RawProps::operator=(const RawProps& other) noexcept {
Expand All @@ -65,6 +156,7 @@ RawProps& RawProps::operator=(const RawProps& other) noexcept {
} else if (mode_ == Mode::Dynamic) {
dynamic_ = other.dynamic_;
}
ignoreYogaStyleProps_ = other.ignoreYogaStyleProps_;
return *this;
}

Expand All @@ -84,12 +176,17 @@ RawProps::operator folly::dynamic() const noexcept {
case Mode::Empty:
return folly::dynamic::object();
case Mode::JSI:
return jsi::dynamicFromValue(*runtime_, value_);
return jsi::dynamicFromValue(
*runtime_, value_, ignoreYogaStyleProps_ ? isYogaStyleProp : nullptr);
case Mode::Dynamic:
return dynamic_;
}
}

void RawProps::filterYogaStylePropsInDynamicConversion() noexcept {
ignoreYogaStyleProps_ = true;
}

/*
* Returns `true` if the object is empty.
* Empty `RawProps` does not have any stored data.
Expand Down
11 changes: 11 additions & 0 deletions packages/react-native/ReactCommon/react/renderer/core/RawProps.h
Original file line number Diff line number Diff line change
Expand Up @@ -73,6 +73,15 @@ class RawProps final {
*/
explicit operator folly::dynamic() const noexcept;

/*
* Once called, Yoga style props will be filtered out during conversion to
* folly::dynamic. folly::dynamic conversion is only used on Android and props
* specific to Yoga do not need to be send over JNI to Android.
* This is a performance optimisation to minimise traffic between C++ and
* Java.
*/
void filterYogaStylePropsInDynamicConversion() noexcept;

/*
* Returns `true` if the object is empty.
* Empty `RawProps` does not have any stored data.
Expand Down Expand Up @@ -124,6 +133,8 @@ class RawProps final {
*/
mutable std::vector<RawPropsValueIndex> keyIndexToValueIndex_;
mutable std::vector<RawValue> values_;

bool ignoreYogaStyleProps_{false};
};

} // namespace facebook::react
Original file line number Diff line number Diff line change
Expand Up @@ -467,3 +467,39 @@ TEST(RawPropsTest, copyJSIRawProps) {
EXPECT_NEAR(
copyProps->derivedFloatValue, originalProps->derivedFloatValue, 0.00001);
}

TEST(RawPropsTest, filterYogaRawProps) {
auto runtime = facebook::hermes::makeHermesRuntime();

ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};

auto object = jsi::Object(*runtime);
object.setProperty(*runtime, "floatValue", 10.0);
object.setProperty(*runtime, "flex", 1);

auto rawProps = RawProps(*runtime, jsi::Value(*runtime, object));

EXPECT_FALSE(rawProps.isEmpty());

auto dynamicProps = (folly::dynamic)rawProps;

EXPECT_EQ(dynamicProps["floatValue"], 10.0);
EXPECT_EQ(dynamicProps["flex"], 1);

rawProps.filterYogaStylePropsInDynamicConversion();

dynamicProps = (folly::dynamic)rawProps;

EXPECT_EQ(dynamicProps["floatValue"], 10.0);
EXPECT_EQ(dynamicProps["flex"], nullptr);

// The fact that filterYogaStylePropsInDynamicConversion should
// must apply to a copy as well.
auto copy = RawProps(rawProps);

auto dynamicPropsFromCopy = (folly::dynamic)copy;

EXPECT_EQ(dynamicPropsFromCopy["floatValue"], 10.0);
EXPECT_EQ(dynamicPropsFromCopy["flex"], nullptr);
}