Skip to content

Commit b81da72

Browse files
sammy-SCfacebook-github-bot
authored andcommitted
Simplify BaseTextProps::appendTextAttributesProps
Summary: `BaseTextProps::appendTextAttributesProps` builds a `folly::dynamic` diff of ~29 text attributes through a long flat sequence of per-attribute `if (changed) result[key] = ...` blocks, many with an embedded `has_value() ? convert(...) : nullptr` ternary. That gave the function a cyclomatic complexity (CCN) of 47, well into the "complex, hard to test" range. This is a pure, behavior-preserving refactor. The four recurring compare-and-assign shapes — plain `!=`, `floatEquality`, optional-with-converter, and color dereference — are pulled into small file-local `static` helpers, and the function body becomes a flat list of helper calls in the exact same order, with the exact same keys, conversions, and values. The serialized output (including `folly::dynamic` insertion order) is identical. Only the `.cpp` is touched; there are no public header or API changes. Changelog: [Internal] Differential Revision: D108027815
1 parent 4adca58 commit b81da72

1 file changed

Lines changed: 208 additions & 168 deletions

File tree

packages/react-native/ReactCommon/react/renderer/components/text/BaseTextProps.cpp

Lines changed: 208 additions & 168 deletions
Original file line numberDiff line numberDiff line change
@@ -354,185 +354,225 @@ static folly::dynamic toDynamic(const Size& size) {
354354
return sizeResult;
355355
}
356356

357-
void BaseTextProps::appendTextAttributesProps(
357+
// Behavior-preserving helpers extracted from appendTextAttributesProps below to
358+
// keep its cyclomatic complexity low. Each mirrors one of the recurring
359+
// compare-and-assign shapes used per text attribute, so the serialized output
360+
// (keys, values, and insertion order) is identical to the open-coded version.
361+
template <typename T>
362+
static void appendIfChanged(
358363
folly::dynamic& result,
359-
const BaseTextProps* oldProps) const {
360-
if (textAttributes.foregroundColor !=
361-
oldProps->textAttributes.foregroundColor) {
362-
result["color"] = *textAttributes.foregroundColor;
363-
}
364-
365-
if (textAttributes.fontFamily != oldProps->textAttributes.fontFamily) {
366-
result["fontFamily"] = textAttributes.fontFamily;
367-
}
368-
369-
if (!floatEquality(
370-
textAttributes.fontSize, oldProps->textAttributes.fontSize)) {
371-
result["fontSize"] = textAttributes.fontSize;
372-
}
373-
374-
if (!floatEquality(
375-
textAttributes.fontSizeMultiplier,
376-
oldProps->textAttributes.fontSizeMultiplier)) {
377-
result["fontSizeMultiplier"] = textAttributes.fontSizeMultiplier;
378-
}
379-
380-
if (textAttributes.fontWeight != oldProps->textAttributes.fontWeight) {
381-
result["fontWeight"] = textAttributes.fontWeight.has_value()
382-
? toString(textAttributes.fontWeight.value())
383-
: folly::dynamic(nullptr);
384-
}
385-
386-
if (textAttributes.fontStyle != oldProps->textAttributes.fontStyle) {
387-
result["fontStyle"] = textAttributes.fontStyle.has_value()
388-
? toString(textAttributes.fontStyle.value())
389-
: folly::dynamic(nullptr);
390-
}
391-
392-
if (textAttributes.fontVariant != oldProps->textAttributes.fontVariant) {
393-
result["fontVariant"] = textAttributes.fontVariant.has_value()
394-
? toString(textAttributes.fontVariant.value())
395-
: folly::dynamic(nullptr);
396-
}
397-
398-
if (textAttributes.allowFontScaling !=
399-
oldProps->textAttributes.allowFontScaling) {
400-
result["allowFontScaling"] = textAttributes.allowFontScaling.has_value()
401-
? textAttributes.allowFontScaling.value()
402-
: folly::dynamic(nullptr);
403-
}
404-
405-
if (!floatEquality(
406-
textAttributes.maxFontSizeMultiplier,
407-
oldProps->textAttributes.maxFontSizeMultiplier)) {
408-
result["maxFontSizeMultiplier"] = textAttributes.maxFontSizeMultiplier;
409-
}
410-
411-
if (textAttributes.dynamicTypeRamp !=
412-
oldProps->textAttributes.dynamicTypeRamp) {
413-
result["dynamicTypeRamp"] = textAttributes.dynamicTypeRamp.has_value()
414-
? toString(textAttributes.dynamicTypeRamp.value())
415-
: folly::dynamic(nullptr);
416-
}
417-
418-
if (!floatEquality(
419-
textAttributes.letterSpacing,
420-
oldProps->textAttributes.letterSpacing)) {
421-
result["letterSpacing"] = textAttributes.letterSpacing;
422-
}
423-
424-
if (textAttributes.textTransform != oldProps->textAttributes.textTransform) {
425-
result["textTransform"] = textAttributes.textTransform.has_value()
426-
? toString(textAttributes.textTransform.value())
427-
: folly::dynamic(nullptr);
428-
}
429-
430-
if (!floatEquality(
431-
textAttributes.lineHeight, oldProps->textAttributes.lineHeight)) {
432-
result["lineHeight"] = textAttributes.lineHeight;
433-
}
434-
435-
if (textAttributes.alignment != oldProps->textAttributes.alignment) {
436-
result["textAlign"] = textAttributes.alignment.has_value()
437-
? toString(textAttributes.alignment.value())
438-
: folly::dynamic(nullptr);
439-
}
440-
441-
if (textAttributes.baseWritingDirection !=
442-
oldProps->textAttributes.baseWritingDirection) {
443-
result["baseWritingDirection"] =
444-
textAttributes.baseWritingDirection.has_value()
445-
? toString(textAttributes.baseWritingDirection.value())
446-
: folly::dynamic(nullptr);
447-
}
448-
449-
if (textAttributes.lineBreakStrategy !=
450-
oldProps->textAttributes.lineBreakStrategy) {
451-
result["lineBreakStrategyIOS"] =
452-
textAttributes.lineBreakStrategy.has_value()
453-
? toString(textAttributes.lineBreakStrategy.value())
454-
: folly::dynamic(nullptr);
455-
}
456-
457-
if (textAttributes.lineBreakMode != oldProps->textAttributes.lineBreakMode) {
458-
result["lineBreakModeIOS"] = textAttributes.lineBreakMode.has_value()
459-
? toString(textAttributes.lineBreakMode.value())
460-
: folly::dynamic(nullptr);
461-
}
462-
463-
if (textAttributes.textDecorationColor !=
464-
oldProps->textAttributes.textDecorationColor) {
465-
result["textDecorationColor"] = *textAttributes.textDecorationColor;
466-
}
467-
468-
if (textAttributes.textDecorationLineType !=
469-
oldProps->textAttributes.textDecorationLineType) {
470-
result["textDecorationLine"] =
471-
textAttributes.textDecorationLineType.has_value()
472-
? toString(textAttributes.textDecorationLineType.value())
473-
: folly::dynamic(nullptr);
474-
}
475-
476-
if (textAttributes.textDecorationStyle !=
477-
oldProps->textAttributes.textDecorationStyle) {
478-
result["textDecorationStyle"] =
479-
textAttributes.textDecorationStyle.has_value()
480-
? toString(textAttributes.textDecorationStyle.value())
481-
: folly::dynamic(nullptr);
482-
}
483-
484-
if (textAttributes.textShadowOffset !=
485-
oldProps->textAttributes.textShadowOffset) {
486-
result["textShadowOffset"] = textAttributes.textShadowOffset.has_value()
487-
? toDynamic(textAttributes.textShadowOffset.value())
488-
: folly::dynamic(nullptr);
489-
}
490-
491-
if (!floatEquality(
492-
textAttributes.textShadowRadius,
493-
oldProps->textAttributes.textShadowRadius)) {
494-
result["textShadowRadius"] = textAttributes.textShadowRadius;
495-
}
496-
497-
if (textAttributes.textShadowColor !=
498-
oldProps->textAttributes.textShadowColor) {
499-
result["textShadowColor"] = *textAttributes.textShadowColor;
500-
}
501-
502-
if (textAttributes.isHighlighted != oldProps->textAttributes.isHighlighted) {
503-
result["isHighlighted"] = textAttributes.isHighlighted.has_value()
504-
? textAttributes.isHighlighted.value()
505-
: folly::dynamic(nullptr);
364+
const char* propName,
365+
const T& newValue,
366+
const T& oldValue) {
367+
if (newValue != oldValue) {
368+
result[propName] = newValue;
506369
}
370+
}
507371

508-
if (textAttributes.isPressable != oldProps->textAttributes.isPressable) {
509-
result["isPressable"] = textAttributes.isPressable.has_value()
510-
? textAttributes.isPressable.value()
511-
: folly::dynamic(nullptr);
372+
template <typename T>
373+
static void appendDerefIfChanged(
374+
folly::dynamic& result,
375+
const char* propName,
376+
const T& newValue,
377+
const T& oldValue) {
378+
if (newValue != oldValue) {
379+
result[propName] = *newValue;
512380
}
381+
}
513382

514-
if (textAttributes.accessibilityRole !=
515-
oldProps->textAttributes.accessibilityRole) {
516-
result["accessibilityRole"] = textAttributes.accessibilityRole.has_value()
517-
? toString(textAttributes.accessibilityRole.value())
518-
: folly::dynamic(nullptr);
383+
static void appendFloatIfChanged(
384+
folly::dynamic& result,
385+
const char* propName,
386+
Float newValue,
387+
Float oldValue) {
388+
if (!floatEquality(newValue, oldValue)) {
389+
result[propName] = newValue;
519390
}
391+
}
520392

521-
if (textAttributes.role != oldProps->textAttributes.role) {
522-
result["role"] = textAttributes.role.has_value()
523-
? toString(textAttributes.role.value())
393+
template <typename T, typename Convert>
394+
static void appendOptionalIfChanged(
395+
folly::dynamic& result,
396+
const char* propName,
397+
const std::optional<T>& newValue,
398+
const std::optional<T>& oldValue,
399+
Convert&& convert) {
400+
if (newValue != oldValue) {
401+
result[propName] = newValue.has_value()
402+
? folly::dynamic(convert(newValue.value()))
524403
: folly::dynamic(nullptr);
525404
}
405+
}
526406

527-
if (!floatEquality(
528-
textAttributes.opacity, oldProps->textAttributes.opacity)) {
529-
result["opacity"] = textAttributes.opacity;
530-
}
407+
void BaseTextProps::appendTextAttributesProps(
408+
folly::dynamic& result,
409+
const BaseTextProps* oldProps) const {
410+
auto asString = [](const auto& value) { return toString(value); };
411+
auto asIs = [](const auto& value) { return value; };
412+
auto asDynamic = [](const auto& value) { return toDynamic(value); };
531413

532-
if (textAttributes.backgroundColor !=
533-
oldProps->textAttributes.backgroundColor) {
534-
result["backgroundColor"] = *textAttributes.backgroundColor;
535-
}
414+
appendDerefIfChanged(
415+
result,
416+
"color",
417+
textAttributes.foregroundColor,
418+
oldProps->textAttributes.foregroundColor);
419+
appendIfChanged(
420+
result,
421+
"fontFamily",
422+
textAttributes.fontFamily,
423+
oldProps->textAttributes.fontFamily);
424+
appendFloatIfChanged(
425+
result,
426+
"fontSize",
427+
textAttributes.fontSize,
428+
oldProps->textAttributes.fontSize);
429+
appendFloatIfChanged(
430+
result,
431+
"fontSizeMultiplier",
432+
textAttributes.fontSizeMultiplier,
433+
oldProps->textAttributes.fontSizeMultiplier);
434+
appendOptionalIfChanged(
435+
result,
436+
"fontWeight",
437+
textAttributes.fontWeight,
438+
oldProps->textAttributes.fontWeight,
439+
asString);
440+
appendOptionalIfChanged(
441+
result,
442+
"fontStyle",
443+
textAttributes.fontStyle,
444+
oldProps->textAttributes.fontStyle,
445+
asString);
446+
appendOptionalIfChanged(
447+
result,
448+
"fontVariant",
449+
textAttributes.fontVariant,
450+
oldProps->textAttributes.fontVariant,
451+
asString);
452+
appendOptionalIfChanged(
453+
result,
454+
"allowFontScaling",
455+
textAttributes.allowFontScaling,
456+
oldProps->textAttributes.allowFontScaling,
457+
asIs);
458+
appendFloatIfChanged(
459+
result,
460+
"maxFontSizeMultiplier",
461+
textAttributes.maxFontSizeMultiplier,
462+
oldProps->textAttributes.maxFontSizeMultiplier);
463+
appendOptionalIfChanged(
464+
result,
465+
"dynamicTypeRamp",
466+
textAttributes.dynamicTypeRamp,
467+
oldProps->textAttributes.dynamicTypeRamp,
468+
asString);
469+
appendFloatIfChanged(
470+
result,
471+
"letterSpacing",
472+
textAttributes.letterSpacing,
473+
oldProps->textAttributes.letterSpacing);
474+
appendOptionalIfChanged(
475+
result,
476+
"textTransform",
477+
textAttributes.textTransform,
478+
oldProps->textAttributes.textTransform,
479+
asString);
480+
appendFloatIfChanged(
481+
result,
482+
"lineHeight",
483+
textAttributes.lineHeight,
484+
oldProps->textAttributes.lineHeight);
485+
appendOptionalIfChanged(
486+
result,
487+
"textAlign",
488+
textAttributes.alignment,
489+
oldProps->textAttributes.alignment,
490+
asString);
491+
appendOptionalIfChanged(
492+
result,
493+
"baseWritingDirection",
494+
textAttributes.baseWritingDirection,
495+
oldProps->textAttributes.baseWritingDirection,
496+
asString);
497+
appendOptionalIfChanged(
498+
result,
499+
"lineBreakStrategyIOS",
500+
textAttributes.lineBreakStrategy,
501+
oldProps->textAttributes.lineBreakStrategy,
502+
asString);
503+
appendOptionalIfChanged(
504+
result,
505+
"lineBreakModeIOS",
506+
textAttributes.lineBreakMode,
507+
oldProps->textAttributes.lineBreakMode,
508+
asString);
509+
appendDerefIfChanged(
510+
result,
511+
"textDecorationColor",
512+
textAttributes.textDecorationColor,
513+
oldProps->textAttributes.textDecorationColor);
514+
appendOptionalIfChanged(
515+
result,
516+
"textDecorationLine",
517+
textAttributes.textDecorationLineType,
518+
oldProps->textAttributes.textDecorationLineType,
519+
asString);
520+
appendOptionalIfChanged(
521+
result,
522+
"textDecorationStyle",
523+
textAttributes.textDecorationStyle,
524+
oldProps->textAttributes.textDecorationStyle,
525+
asString);
526+
appendOptionalIfChanged(
527+
result,
528+
"textShadowOffset",
529+
textAttributes.textShadowOffset,
530+
oldProps->textAttributes.textShadowOffset,
531+
asDynamic);
532+
appendFloatIfChanged(
533+
result,
534+
"textShadowRadius",
535+
textAttributes.textShadowRadius,
536+
oldProps->textAttributes.textShadowRadius);
537+
appendDerefIfChanged(
538+
result,
539+
"textShadowColor",
540+
textAttributes.textShadowColor,
541+
oldProps->textAttributes.textShadowColor);
542+
appendOptionalIfChanged(
543+
result,
544+
"isHighlighted",
545+
textAttributes.isHighlighted,
546+
oldProps->textAttributes.isHighlighted,
547+
asIs);
548+
appendOptionalIfChanged(
549+
result,
550+
"isPressable",
551+
textAttributes.isPressable,
552+
oldProps->textAttributes.isPressable,
553+
asIs);
554+
appendOptionalIfChanged(
555+
result,
556+
"accessibilityRole",
557+
textAttributes.accessibilityRole,
558+
oldProps->textAttributes.accessibilityRole,
559+
asString);
560+
appendOptionalIfChanged(
561+
result,
562+
"role",
563+
textAttributes.role,
564+
oldProps->textAttributes.role,
565+
asString);
566+
appendFloatIfChanged(
567+
result,
568+
"opacity",
569+
textAttributes.opacity,
570+
oldProps->textAttributes.opacity);
571+
appendDerefIfChanged(
572+
result,
573+
"backgroundColor",
574+
textAttributes.backgroundColor,
575+
oldProps->textAttributes.backgroundColor);
536576
}
537577

538578
#endif

0 commit comments

Comments
 (0)