pass RawProps by rvalue (#41604)

Summary:
Pull Request resolved: https://github.com/facebook/react-native/pull/41604

changelog: [internal]

Remove const requirement when using RawProps and prefer passing it by rvalue.

Reviewed By: javache

Differential Revision: D51471667

fbshipit-source-id: 479bcebe642e168ff1f110e7b2bfaf20a3e82821
This commit is contained in:
Samuel Susla
2023-11-22 04:33:02 -08:00
committed by Facebook GitHub Bot
parent 951efc8ce2
commit 91d4b1bd80
11 changed files with 186 additions and 74 deletions
@@ -21,10 +21,10 @@ ComponentName UnimplementedViewComponentDescriptor::getComponentName() const {
Props::Shared UnimplementedViewComponentDescriptor::cloneProps(
const PropsParserContext& context,
const Props::Shared& props,
const RawProps& rawProps) const {
RawProps rawProps) const {
auto clonedProps =
ConcreteComponentDescriptor<UnimplementedViewShadowNode>::cloneProps(
context, props, rawProps);
context, props, std::move(rawProps));
// We have to clone `Props` object one more time to make sure that we have
// an unshared (and non-`const`) copy of it which we can mutate.
@@ -33,7 +33,7 @@ Props::Shared UnimplementedViewComponentDescriptor::cloneProps(
auto unimplementedViewProps = std::make_shared<UnimplementedViewProps>(
context,
static_cast<const UnimplementedViewProps&>(*clonedProps),
emptyRawProps);
std::move(emptyRawProps));
unimplementedViewProps->setComponentName(getComponentName());
return unimplementedViewProps;
@@ -35,7 +35,7 @@ class UnimplementedViewComponentDescriptor final
Props::Shared cloneProps(
const PropsParserContext& context,
const Props::Shared& props,
const RawProps& rawProps) const override;
RawProps rawProps) const override;
};
} // namespace facebook::react
@@ -105,7 +105,7 @@ class ComponentDescriptor {
virtual Props::Shared cloneProps(
const PropsParserContext& context,
const Props::Shared& props,
const RawProps& rawProps) const = 0;
RawProps rawProps) const = 0;
/*
* Create an initial State object that represents (and contains) an initial
@@ -95,7 +95,7 @@ class ConcreteComponentDescriptor : public ComponentDescriptor {
virtual Props::Shared cloneProps(
const PropsParserContext& context,
const Props::Shared& props,
const RawProps& rawProps) const override {
RawProps rawProps) const override {
// Optimization:
// Quite often nodes are constructed with default/empty props: the base
// `props` object is `null` (there no base because it's not cloning) and the
@@ -26,7 +26,7 @@ RawProps::RawProps(jsi::Runtime& runtime, const jsi::Value& value) noexcept {
return;
}
mode_ = mode_ = Mode::JSI;
mode_ = Mode::JSI;
runtime_ = &runtime;
value_ = jsi::Value(runtime, value);
}
@@ -47,7 +47,28 @@ RawProps::RawProps(folly::dynamic dynamic) noexcept {
dynamic_ = std::move(dynamic);
}
void RawProps::parse(const RawPropsParser& parser) const noexcept {
RawProps::RawProps(const RawProps& other) noexcept {
mode_ = other.mode_;
if (mode_ == Mode::JSI) {
runtime_ = other.runtime_;
value_ = jsi::Value(*runtime_, other.value_);
} else if (mode_ == Mode::Dynamic) {
dynamic_ = other.dynamic_;
}
}
RawProps& RawProps::operator=(const RawProps& other) noexcept {
mode_ = other.mode_;
if (mode_ == Mode::JSI) {
runtime_ = other.runtime_;
value_ = jsi::Value(*runtime_, other.value_);
} else if (mode_ == Mode::Dynamic) {
dynamic_ = other.dynamic_;
}
return *this;
}
void RawProps::parse(const RawPropsParser& parser) noexcept {
react_native_assert(parser_ == nullptr && "A parser was already assigned.");
parser_ = &parser;
parser.preparse(*this);
@@ -50,6 +50,12 @@ class RawProps final {
*/
RawProps(jsi::Runtime& runtime, const jsi::Value& value) noexcept;
explicit RawProps(const RawProps& rawProps) noexcept;
RawProps& operator=(const RawProps& other) noexcept;
RawProps(RawProps&& other) noexcept = default;
RawProps& operator=(RawProps&& other) noexcept = default;
/*
* Creates an object with given `folly::dynamic` object.
* Deprecated. Do not use.
@@ -58,19 +64,7 @@ class RawProps final {
*/
explicit RawProps(folly::dynamic dynamic) noexcept;
/*
* Not moveable.
*/
RawProps(RawProps&& other) noexcept = delete;
RawProps& operator=(RawProps&& other) noexcept = delete;
/*
* Not copyable.
*/
RawProps(const RawProps& other) noexcept = delete;
RawProps& operator=(const RawProps& other) noexcept = delete;
void parse(const RawPropsParser& parser) const noexcept;
void parse(const RawPropsParser& parser) noexcept;
/*
* Deprecated. Do not use.
@@ -26,8 +26,9 @@ TEST(ComponentDescriptorTest, createShadowNode) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
const auto& raw = RawProps(folly::dynamic::object("nativeID", "abc"));
Props::Shared props = descriptor->cloneProps(parserContext, nullptr, raw);
auto rawProps = RawProps(folly::dynamic::object("nativeID", "abc"));
Props::Shared props =
descriptor->cloneProps(parserContext, nullptr, std::move(rawProps));
auto family = descriptor->createFamily(ShadowNodeFamilyFragment{
/* .tag = */ 9,
@@ -58,8 +59,9 @@ TEST(ComponentDescriptorTest, cloneShadowNode) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
const auto& raw = RawProps(folly::dynamic::object("nativeID", "abc"));
Props::Shared props = descriptor->cloneProps(parserContext, nullptr, raw);
auto rawProps = RawProps(folly::dynamic::object("nativeID", "abc"));
Props::Shared props =
descriptor->cloneProps(parserContext, nullptr, std::move(rawProps));
auto family = descriptor->createFamily(ShadowNodeFamilyFragment{
/* .tag = */ 9,
/* .surfaceId = */ 1,
@@ -91,8 +93,9 @@ TEST(ComponentDescriptorTest, appendChild) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
const auto& raw = RawProps(folly::dynamic::object("nativeID", "abc"));
Props::Shared props = descriptor->cloneProps(parserContext, nullptr, raw);
auto rawProps = RawProps(folly::dynamic::object("nativeID", "abc"));
Props::Shared props =
descriptor->cloneProps(parserContext, nullptr, std::move(rawProps));
auto family1 = descriptor->createFamily(ShadowNodeFamilyFragment{
/* .tag = */ 1,
/* .surfaceId = */ 1,
@@ -8,6 +8,7 @@
#include <memory>
#include <gtest/gtest.h>
#include <hermes/hermes.h>
#include <react/debug/flags.h>
#include <react/renderer/core/ConcreteShadowNode.h>
#include <react/renderer/core/PropsParserContext.h>
@@ -16,6 +17,7 @@
#include "TestComponent.h"
using namespace facebook;
using namespace facebook::react;
class PropsSingleFloat : public Props {
@@ -151,7 +153,7 @@ TEST(RawPropsTest, handleProps) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
const auto& raw = RawProps(folly::dynamic::object("nativeID", "abc"));
auto raw = RawProps(folly::dynamic::object("nativeID", "abc"));
auto parser = RawPropsParser();
parser.prepare<Props>();
raw.parse(parser);
@@ -168,7 +170,7 @@ TEST(RawPropsTest, handleRawPropsSingleString) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
const auto& raw = RawProps(folly::dynamic::object("nativeID", "abc"));
auto raw = RawProps(folly::dynamic::object("nativeID", "abc"));
auto parser = RawPropsParser();
parser.prepare<Props>();
raw.parse(parser);
@@ -182,8 +184,7 @@ TEST(RawPropsTest, handleRawPropsSingleFloat) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
const auto& raw =
RawProps(folly::dynamic::object("floatValue", (float)42.42));
auto raw = RawProps(folly::dynamic::object("floatValue", (float)42.42));
auto parser = RawPropsParser();
parser.prepare<PropsSingleFloat>();
raw.parse(parser);
@@ -197,8 +198,7 @@ TEST(RawPropsTest, handleRawPropsSingleDouble) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
const auto& raw =
RawProps(folly::dynamic::object("doubleValue", (double)42.42));
auto raw = RawProps(folly::dynamic::object("doubleValue", (double)42.42));
auto parser = RawPropsParser();
parser.prepare<PropsSingleDouble>();
raw.parse(parser);
@@ -212,7 +212,7 @@ TEST(RawPropsTest, handleRawPropsSingleInt) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
const auto& raw = RawProps(folly::dynamic::object("intValue", (int)42.42));
auto raw = RawProps(folly::dynamic::object("intValue", (int)42.42));
auto parser = RawPropsParser();
parser.prepare<PropsSingleInt>();
raw.parse(parser);
@@ -226,7 +226,7 @@ TEST(RawPropsTest, handleRawPropsSingleIntGetManyTimes) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
const auto& raw = RawProps(folly::dynamic::object("intValue", (int)42.42));
auto raw = RawProps(folly::dynamic::object("intValue", (int)42.42));
auto parser = RawPropsParser();
parser.prepare<PropsSingleInt>();
raw.parse(parser);
@@ -240,7 +240,7 @@ TEST(RawPropsTest, handleRawPropsPrimitiveTypes) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
const auto& raw = RawProps(
auto raw = RawProps(
folly::dynamic::object("intValue", (int)42)("doubleValue", (double)17.42)(
"floatValue",
(float)66.67)("stringValue", "helloworld")("boolValue", true));
@@ -262,7 +262,7 @@ TEST(RawPropsTest, handleRawPropsPrimitiveTypesGetTwice) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
const auto& raw = RawProps(
auto raw = RawProps(
folly::dynamic::object("intValue", (int)42)("doubleValue", (double)17.42)(
"floatValue",
(float)66.67)("stringValue", "helloworld")("boolValue", true));
@@ -292,7 +292,7 @@ TEST(RawPropsTest, handleRawPropsPrimitiveTypesGetOutOfOrder) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
const auto& raw = RawProps(
auto raw = RawProps(
folly::dynamic::object("intValue", (int)42)("doubleValue", (double)17.42)(
"floatValue",
(float)66.67)("stringValue", "helloworld")("boolValue", true));
@@ -322,7 +322,7 @@ TEST(RawPropsTest, handleRawPropsPrimitiveTypesIncomplete) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
const auto& raw = RawProps(folly::dynamic::object("intValue", (int)42));
auto raw = RawProps(folly::dynamic::object("intValue", (int)42));
auto parser = RawPropsParser();
parser.prepare<PropsPrimitiveTypes>();
@@ -342,7 +342,7 @@ TEST(RawPropsTest, handleRawPropsPrimitiveTypesIncorrectLookup) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
const auto& raw = RawProps(folly::dynamic::object("intValue", (int)42));
auto raw = RawProps(folly::dynamic::object("intValue", (int)42));
auto parser = RawPropsParser();
parser.prepare<PropsPrimitiveTypes>();
@@ -360,7 +360,7 @@ TEST(RawPropsTest, handlePropsMultiLookup) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
const auto& raw = RawProps(folly::dynamic::object("floatValue", (float)10.0));
auto raw = RawProps(folly::dynamic::object("floatValue", (float)10.0));
auto parser = RawPropsParser();
parser.prepare<PropsMultiLookup>();
raw.parse(parser);
@@ -374,3 +374,96 @@ TEST(RawPropsTest, handlePropsMultiLookup) {
EXPECT_NEAR(props->floatValue, 10.0, 0.00001);
EXPECT_NEAR(props->derivedFloatValue, 20.0, 0.00001);
}
TEST(RawPropsTest, copyDynamicRawProps) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
auto rawProps = RawProps(folly::dynamic::object("floatValue", (float)10.0));
auto copy = RawProps(rawProps);
EXPECT_FALSE(copy.isEmpty());
auto parser = RawPropsParser();
parser.prepare<PropsMultiLookup>();
rawProps.parse(parser);
copy.parse(parser);
auto originalProps = std::make_shared<PropsMultiLookup>(
parserContext, PropsMultiLookup(), rawProps);
auto copyProps = std::make_shared<PropsMultiLookup>(
parserContext, PropsMultiLookup(), copy);
// Props are not sealed after applying raw props.
EXPECT_FALSE(copyProps->getSealed());
EXPECT_NEAR(copyProps->floatValue, originalProps->floatValue, 0.00001);
EXPECT_NEAR(
copyProps->derivedFloatValue, originalProps->derivedFloatValue, 0.00001);
}
TEST(RawPropsTest, copyEmptyRawProps) {
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
auto rawProps = RawProps();
auto copy = RawProps(rawProps);
EXPECT_TRUE(rawProps.isEmpty());
EXPECT_TRUE(copy.isEmpty());
EXPECT_TRUE(((folly::dynamic)copy).empty());
}
TEST(RawPropsTest, copyNullJSIRawProps) {
auto runtime = facebook::hermes::makeHermesRuntime();
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
auto rawProps = RawProps(*runtime, jsi::Value::null());
auto copy = RawProps(rawProps);
EXPECT_TRUE(rawProps.isEmpty());
EXPECT_TRUE(copy.isEmpty());
EXPECT_TRUE(((folly::dynamic)copy).empty());
}
TEST(RawPropsTest, copyJSIRawProps) {
auto runtime = facebook::hermes::makeHermesRuntime();
ContextContainer contextContainer{};
PropsParserContext parserContext{-1, contextContainer};
auto object = jsi::Object(*runtime);
object.setProperty(*runtime, "floatValue", 10.0);
auto rawProps = RawProps(*runtime, jsi::Value(*runtime, object));
auto copy = RawProps(rawProps);
EXPECT_FALSE(rawProps.isEmpty());
EXPECT_FALSE(copy.isEmpty());
auto parser = RawPropsParser();
parser.prepare<PropsMultiLookup>();
rawProps.parse(parser);
copy.parse(parser);
auto originalProps = std::make_shared<PropsMultiLookup>(
parserContext, PropsMultiLookup(), rawProps);
auto copyProps = std::make_shared<PropsMultiLookup>(
parserContext, PropsMultiLookup(), copy);
// Props are not sealed after applying raw props.
EXPECT_FALSE(copyProps->getSealed());
EXPECT_NEAR(copyProps->floatValue, originalProps->floatValue, 0.00001);
EXPECT_NEAR(
copyProps->derivedFloatValue, originalProps->derivedFloatValue, 0.00001);
}
@@ -66,7 +66,7 @@ std::shared_ptr<ShadowNode> UIManager::createNode(
Tag tag,
const std::string& name,
SurfaceId surfaceId,
const RawProps& rawProps,
RawProps rawProps,
InstanceHandle::Shared instanceHandle) const {
SystraceSection s("UIManager::createNode", "componentName", name);
@@ -78,8 +78,8 @@ std::shared_ptr<ShadowNode> UIManager::createNode(
auto family = componentDescriptor.createFamily(
{tag, surfaceId, std::move(instanceHandle)});
const auto props =
componentDescriptor.cloneProps(propsParserContext, nullptr, rawProps);
const auto props = componentDescriptor.cloneProps(
propsParserContext, nullptr, std::move(rawProps));
const auto state = componentDescriptor.createInitialState(props, family);
auto shadowNode = componentDescriptor.createShadowNode(
@@ -111,7 +111,7 @@ std::shared_ptr<ShadowNode> UIManager::createNode(
std::shared_ptr<ShadowNode> UIManager::cloneNode(
const ShadowNode& shadowNode,
const ShadowNode::SharedListOfShared& children,
const RawProps* rawProps) const {
RawProps rawProps) const {
SystraceSection s(
"UIManager::cloneNode", "componentName", shadowNode.getComponentName());
@@ -122,24 +122,22 @@ std::shared_ptr<ShadowNode> UIManager::cloneNode(
auto& family = shadowNode.getFamily();
auto props = ShadowNodeFragment::propsPlaceholder();
if (rawProps != nullptr) {
if (family.nativeProps_DEPRECATED != nullptr) {
// 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`.
family.nativeProps_DEPRECATED =
std::make_unique<folly::dynamic>(mergeDynamicProps(
*family.nativeProps_DEPRECATED, (folly::dynamic)*rawProps));
if (family.nativeProps_DEPRECATED != nullptr) {
// 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`.
family.nativeProps_DEPRECATED =
std::make_unique<folly::dynamic>(mergeDynamicProps(
*family.nativeProps_DEPRECATED, (folly::dynamic)rawProps));
props = componentDescriptor.cloneProps(
propsParserContext,
shadowNode.getProps(),
RawProps(*family.nativeProps_DEPRECATED));
} else {
props = componentDescriptor.cloneProps(
propsParserContext, shadowNode.getProps(), *rawProps);
}
props = componentDescriptor.cloneProps(
propsParserContext,
shadowNode.getProps(),
RawProps(*family.nativeProps_DEPRECATED));
} else {
props = componentDescriptor.cloneProps(
propsParserContext, shadowNode.getProps(), std::move(rawProps));
}
auto clonedShadowNode = componentDescriptor.cloneShadowNode(
@@ -457,7 +455,7 @@ void UIManager::dispatchCommand(
void UIManager::setNativeProps_DEPRECATED(
const ShadowNode::Shared& shadowNode,
const RawProps& rawProps) const {
RawProps rawProps) const {
auto& family = shadowNode->getFamily();
if (family.nativeProps_DEPRECATED) {
// Values in `rawProps` patch (take precedence over)
@@ -474,6 +472,8 @@ void UIManager::setNativeProps_DEPRECATED(
shadowTreeRegistry_.visit(
family.getSurfaceId(), [&](const ShadowTree& shadowTree) {
// The lambda passed to `commit` may be executed multiple times.
// We need to create fresh copy of the `RawProps` object each time.
shadowTree.commit(
[&](RootShadowNode const& oldRootShadowNode) {
auto rootNode = oldRootShadowNode.cloneTree(
@@ -486,7 +486,7 @@ void UIManager::setNativeProps_DEPRECATED(
auto props = componentDescriptor.cloneProps(
propsParserContext,
getNewestCloneOfShadowNode(*shadowNode)->getProps(),
rawProps);
RawProps(rawProps));
return oldShadowNode.clone({/* .props = */ props});
});
@@ -134,13 +134,13 @@ class UIManager final : public ShadowTreeDelegate {
Tag tag,
const std::string& componentName,
SurfaceId surfaceId,
const RawProps& props,
RawProps props,
InstanceHandle::Shared instanceHandle) const;
std::shared_ptr<ShadowNode> cloneNode(
const ShadowNode& shadowNode,
const ShadowNode::SharedListOfShared& children = nullptr,
const RawProps* rawProps = nullptr) const;
const ShadowNode::SharedListOfShared& children,
RawProps rawProps) const;
void appendChild(
const ShadowNode::Shared& parentShadowNode,
@@ -183,7 +183,7 @@ class UIManager final : public ShadowTreeDelegate {
void setNativeProps_DEPRECATED(
const ShadowNode::Shared& shadowNode,
const RawProps& rawProps) const;
RawProps rawProps) const;
void sendAccessibilityEvent(
const ShadowNode::Shared& shadowNode,
@@ -271,7 +271,9 @@ jsi::Value UIManagerBinding::get(
return valueFromShadowNode(
runtime,
uiManager->cloneNode(
*shadowNodeFromValue(runtime, arguments[0])));
*shadowNodeFromValue(runtime, arguments[0]),
nullptr,
RawProps()));
});
}
@@ -351,7 +353,8 @@ jsi::Value UIManagerBinding::get(
uiManager->cloneNode(
*shadowNodeFromValue(runtime, arguments[0]),
count > 1 ? shadowNodeListFromValue(runtime, arguments[1])
: ShadowNode::emptySharedShadowNodeSharedList()));
: ShadowNode::emptySharedShadowNodeSharedList(),
RawProps()));
});
}
@@ -369,13 +372,12 @@ jsi::Value UIManagerBinding::get(
size_t count) -> jsi::Value {
validateArgumentCount(runtime, methodName, paramCount, count);
RawProps rawProps(runtime, arguments[1]);
return valueFromShadowNode(
runtime,
uiManager->cloneNode(
*shadowNodeFromValue(runtime, arguments[0]),
nullptr,
&rawProps));
RawProps(runtime, arguments[1])));
});
}
@@ -396,7 +398,6 @@ jsi::Value UIManagerBinding::get(
// validateArgumentCount(runtime, methodName, paramCount, count);
bool hasChildrenArg = count == 3;
RawProps rawProps(runtime, arguments[hasChildrenArg ? 2 : 1]);
return valueFromShadowNode(
runtime,
uiManager->cloneNode(
@@ -404,7 +405,7 @@ jsi::Value UIManagerBinding::get(
hasChildrenArg
? shadowNodeListFromValue(runtime, arguments[1])
: ShadowNode::emptySharedShadowNodeSharedList(),
&rawProps));
RawProps(runtime, arguments[hasChildrenArg ? 2 : 1])));
});
}