Skip to content
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.

Sign up for GitHub

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

Merge enablePropIteratorSetter flags into a single one in CoreFeatures #34869

Closed
wants to merge 1 commit into from
Closed

Merge enablePropIteratorSetter flags into a single one in CoreFeatures #34869

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
2 changes: 0 additions & 2 deletions React/Fabric/RCTSurfacePresenter.mm
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -273,8 +273,6 @@ - (RCTScheduler *)_createScheduler

if (reactNativeConfig && reactNativeConfig->getBool("react_fabric:enable_cpp_props_iterator_setter_ios")) {
CoreFeatures::enablePropIteratorSetter = true;
AccessibilityProps::enablePropIteratorSetter = true;
BaseTextProps::enablePropIteratorSetter = true;
}

auto componentRegistryFactory =
Expand Down
4 changes: 0 additions & 4 deletions ReactAndroid/src/main/jni/react/fabric/Binding.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -458,10 +458,6 @@ void Binding::installFabricUIManager(
// Props setter pattern feature
CoreFeatures::enablePropIteratorSetter =
getFeatureFlagValue("enableCppPropsIteratorSetter");
AccessibilityProps::enablePropIteratorSetter =
CoreFeatures::enablePropIteratorSetter;
BaseTextProps::enablePropIteratorSetter =
CoreFeatures::enablePropIteratorSetter;

// RemoveDelete mega-op
ShadowViewMutation::PlatformSupportsRemoveDeleteTreeInstruction =
Expand Down
5 changes: 2 additions & 3 deletions ReactCommon/react/renderer/components/text/BaseTextProps.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,15 +8,14 @@
#include "BaseTextProps.h"

#include <react/renderer/attributedstring/conversions.h>
#include <react/renderer/core/CoreFeatures.h>
#include <react/renderer/core/propsConversions.h>
#include <react/renderer/debug/DebugStringConvertibleItem.h>
#include <react/renderer/graphics/conversions.h>

namespace facebook {
namespace react {

bool BaseTextProps::enablePropIteratorSetter = false;

static TextAttributes convertRawProp(
PropsParserContext const &context,
RawProps const &rawProps,
Expand Down Expand Up @@ -195,7 +194,7 @@ BaseTextProps::BaseTextProps(
const BaseTextProps &sourceProps,
const RawProps &rawProps)
: textAttributes(
BaseTextProps::enablePropIteratorSetter
CoreFeatures::enablePropIteratorSetter
? sourceProps.textAttributes
: convertRawProp(
context,
Expand Down
2 changes: 0 additions & 2 deletions ReactCommon/react/renderer/components/text/BaseTextProps.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,8 +34,6 @@ class BaseTextProps {
const char *propName,
RawValue const &value);

static bool enablePropIteratorSetter;

#pragma mark - Props

TextAttributes textAttributes{};
Expand Down
247 changes: 130 additions & 117 deletions ReactCommon/react/renderer/components/view/AccessibilityProps.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,100 +9,108 @@

#include <react/renderer/components/view/accessibilityPropsConversions.h>
#include <react/renderer/components/view/propsConversions.h>
#include <react/renderer/core/CoreFeatures.h>
#include <react/renderer/core/propsConversions.h>
#include <react/renderer/debug/debugStringConvertibleUtils.h>

namespace facebook {
namespace react {

bool AccessibilityProps::enablePropIteratorSetter = false;

AccessibilityProps::AccessibilityProps(
const PropsParserContext &context,
AccessibilityProps const &sourceProps,
RawProps const &rawProps)
: accessible(
enablePropIteratorSetter ? sourceProps.accessible
: convertRawProp(
context,
rawProps,
"accessible",
sourceProps.accessible,
false)),
CoreFeatures::enablePropIteratorSetter ? sourceProps.accessible
: convertRawProp(
context,
rawProps,
"accessible",
sourceProps.accessible,
false)),
accessibilityState(
enablePropIteratorSetter ? sourceProps.accessibilityState
: convertRawProp(
context,
rawProps,
"accessibilityState",
sourceProps.accessibilityState,
{})),
CoreFeatures::enablePropIteratorSetter
? sourceProps.accessibilityState
: convertRawProp(
context,
rawProps,
"accessibilityState",
sourceProps.accessibilityState,
{})),
accessibilityLabel(
enablePropIteratorSetter ? sourceProps.accessibilityLabel
: convertRawProp(
context,
rawProps,
"accessibilityLabel",
sourceProps.accessibilityLabel,
"")),
CoreFeatures::enablePropIteratorSetter
? sourceProps.accessibilityLabel
: convertRawProp(
context,
rawProps,
"accessibilityLabel",
sourceProps.accessibilityLabel,
"")),
accessibilityLabelledBy(
enablePropIteratorSetter ? sourceProps.accessibilityLabelledBy
: convertRawProp(
context,
rawProps,
"accessibilityLabelledBy",
sourceProps.accessibilityLabelledBy,
{})),
CoreFeatures::enablePropIteratorSetter
? sourceProps.accessibilityLabelledBy
: convertRawProp(
context,
rawProps,
"accessibilityLabelledBy",
sourceProps.accessibilityLabelledBy,
{})),
accessibilityLiveRegion(
enablePropIteratorSetter ? sourceProps.accessibilityLiveRegion
: convertRawProp(
context,
rawProps,
"accessibilityLiveRegion",
sourceProps.accessibilityLiveRegion,
AccessibilityLiveRegion::None)),
CoreFeatures::enablePropIteratorSetter
? sourceProps.accessibilityLiveRegion
: convertRawProp(
context,
rawProps,
"accessibilityLiveRegion",
sourceProps.accessibilityLiveRegion,
AccessibilityLiveRegion::None)),
accessibilityHint(
enablePropIteratorSetter ? sourceProps.accessibilityHint
: convertRawProp(
context,
rawProps,
"accessibilityHint",
sourceProps.accessibilityHint,
"")),
CoreFeatures::enablePropIteratorSetter
? sourceProps.accessibilityHint
: convertRawProp(
context,
rawProps,
"accessibilityHint",
sourceProps.accessibilityHint,
"")),
accessibilityLanguage(
enablePropIteratorSetter ? sourceProps.accessibilityLanguage
: convertRawProp(
context,
rawProps,
"accessibilityLanguage",
sourceProps.accessibilityLanguage,
"")),
CoreFeatures::enablePropIteratorSetter
? sourceProps.accessibilityLanguage
: convertRawProp(
context,
rawProps,
"accessibilityLanguage",
sourceProps.accessibilityLanguage,
"")),
accessibilityValue(
enablePropIteratorSetter ? sourceProps.accessibilityValue
: convertRawProp(
context,
rawProps,
"accessibilityValue",
sourceProps.accessibilityValue,
{})),
CoreFeatures::enablePropIteratorSetter
? sourceProps.accessibilityValue
: convertRawProp(
context,
rawProps,
"accessibilityValue",
sourceProps.accessibilityValue,
{})),
accessibilityActions(
enablePropIteratorSetter ? sourceProps.accessibilityActions
: convertRawProp(
context,
rawProps,
"accessibilityActions",
sourceProps.accessibilityActions,
{})),
CoreFeatures::enablePropIteratorSetter
? sourceProps.accessibilityActions
: convertRawProp(
context,
rawProps,
"accessibilityActions",
sourceProps.accessibilityActions,
{})),
accessibilityViewIsModal(
enablePropIteratorSetter ? sourceProps.accessibilityViewIsModal
: convertRawProp(
context,
rawProps,
"accessibilityViewIsModal",
sourceProps.accessibilityViewIsModal,
false)),
CoreFeatures::enablePropIteratorSetter
? sourceProps.accessibilityViewIsModal
: convertRawProp(
context,
rawProps,
"accessibilityViewIsModal",
sourceProps.accessibilityViewIsModal,
false)),
accessibilityElementsHidden(
enablePropIteratorSetter
CoreFeatures::enablePropIteratorSetter
? sourceProps.accessibilityElementsHidden
: convertRawProp(
context,
Expand All @@ -111,7 +119,7 @@ AccessibilityProps::AccessibilityProps(
sourceProps.accessibilityElementsHidden,
false)),
accessibilityIgnoresInvertColors(
enablePropIteratorSetter
CoreFeatures::enablePropIteratorSetter
? sourceProps.accessibilityIgnoresInvertColors
: convertRawProp(
context,
Expand All @@ -120,61 +128,66 @@ AccessibilityProps::AccessibilityProps(
sourceProps.accessibilityIgnoresInvertColors,
false)),
onAccessibilityTap(
enablePropIteratorSetter ? sourceProps.onAccessibilityTap
: convertRawProp(
context,
rawProps,
"onAccessibilityTap",
sourceProps.onAccessibilityTap,
{})),
CoreFeatures::enablePropIteratorSetter
? sourceProps.onAccessibilityTap
: convertRawProp(
context,
rawProps,
"onAccessibilityTap",
sourceProps.onAccessibilityTap,
{})),
onAccessibilityMagicTap(
enablePropIteratorSetter ? sourceProps.onAccessibilityMagicTap
: convertRawProp(
context,
rawProps,
"onAccessibilityMagicTap",
sourceProps.onAccessibilityMagicTap,
{})),
CoreFeatures::enablePropIteratorSetter
? sourceProps.onAccessibilityMagicTap
: convertRawProp(
context,
rawProps,
"onAccessibilityMagicTap",
sourceProps.onAccessibilityMagicTap,
{})),
onAccessibilityEscape(
enablePropIteratorSetter ? sourceProps.onAccessibilityEscape
: convertRawProp(
context,
rawProps,
"onAccessibilityEscape",
sourceProps.onAccessibilityEscape,
{})),
CoreFeatures::enablePropIteratorSetter
? sourceProps.onAccessibilityEscape
: convertRawProp(
context,
rawProps,
"onAccessibilityEscape",
sourceProps.onAccessibilityEscape,
{})),
onAccessibilityAction(
enablePropIteratorSetter ? sourceProps.onAccessibilityAction
: convertRawProp(
context,
rawProps,
"onAccessibilityAction",
sourceProps.onAccessibilityAction,
{})),
CoreFeatures::enablePropIteratorSetter
? sourceProps.onAccessibilityAction
: convertRawProp(
context,
rawProps,
"onAccessibilityAction",
sourceProps.onAccessibilityAction,
{})),
importantForAccessibility(
enablePropIteratorSetter ? sourceProps.importantForAccessibility
: convertRawProp(
context,
rawProps,
"importantForAccessibility",
sourceProps.importantForAccessibility,
ImportantForAccessibility::Auto)),
CoreFeatures::enablePropIteratorSetter
? sourceProps.importantForAccessibility
: convertRawProp(
context,
rawProps,
"importantForAccessibility",
sourceProps.importantForAccessibility,
ImportantForAccessibility::Auto)),
testId(
enablePropIteratorSetter ? sourceProps.testId
: convertRawProp(
context,
rawProps,
"testID",
sourceProps.testId,
"")) {
CoreFeatures::enablePropIteratorSetter ? sourceProps.testId
: convertRawProp(
context,
rawProps,
"testID",
sourceProps.testId,
"")) {
// It is a (severe!) perf deoptimization to request props out-of-order.
// Thus, since we need to request the same prop twice here
// (accessibilityRole) we "must" do them subsequently here to prevent
// a regression. It is reasonable to ask if the `at` function can be improved;
// it probably can, but this is a fairly rare edge-case that (1) is easy-ish
// to work around here, and (2) would require very careful work to address
// this case and not regress the more common cases.
if (!enablePropIteratorSetter) {
if (!CoreFeatures::enablePropIteratorSetter) {
const auto *rawPropValue =
rawProps.at("accessibilityRole", nullptr, nullptr);
AccessibilityTraits traits;
Expand Down
2 changes: 0 additions & 2 deletions ReactCommon/react/renderer/components/view/AccessibilityProps.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,6 @@ class AccessibilityProps {
const char *propName,
RawValue const &value);

static bool enablePropIteratorSetter;

#ifdef ANDROID
void propsDiffMapBuffer(Props const *oldProps, MapBufferBuilder &builder)
const;
Expand Down