Prop parsing: always require explicit default argument to `convertRawProp`

Summary: Having automatic defaults/an optional arg for `convertRawProp` has caused way more problems than it is worth. Remove the default argument. Changelog: [Internal] Reviewed By: shergin Differential Revision: D19151594 fbshipit-source-id: 839ec8d138b2c3c083f221a2871582454004648c
2019-12-18 14:59:24 -08:00 · 2019-12-18 14:59:24 -08:00 · bb5622dd89
--- a/ReactCommon/fabric/attributedstring/conversions.h
+++ b/ReactCommon/fabric/attributedstring/conversions.h
@ -414,33 +414,40 @@ inline std::string toString(

 inline ParagraphAttributes convertRawProp(
    RawProps const &rawProps,
+    ParagraphAttributes const &sourceParagraphAttributes,
    ParagraphAttributes const &defaultParagraphAttributes) {
  auto paragraphAttributes = ParagraphAttributes{};

  paragraphAttributes.maximumNumberOfLines = convertRawProp(
      rawProps,
      "numberOfLines",
+      sourceParagraphAttributes.maximumNumberOfLines,
      defaultParagraphAttributes.maximumNumberOfLines);
  paragraphAttributes.ellipsizeMode = convertRawProp(
-      rawProps, "ellipsizeMode", defaultParagraphAttributes.ellipsizeMode);
+      rawProps,
+      "ellipsizeMode",
+      sourceParagraphAttributes.ellipsizeMode,
+      defaultParagraphAttributes.ellipsizeMode);
  paragraphAttributes.textBreakStrategy = convertRawProp(
      rawProps,
      "textBreakStrategy",
+      sourceParagraphAttributes.textBreakStrategy,
      defaultParagraphAttributes.textBreakStrategy);
  paragraphAttributes.adjustsFontSizeToFit = convertRawProp(
      rawProps,
      "adjustsFontSizeToFit",
+      sourceParagraphAttributes.adjustsFontSizeToFit,
      defaultParagraphAttributes.adjustsFontSizeToFit);
  paragraphAttributes.minimumFontSize = convertRawProp(
      rawProps,
      "minimumFontSize",
-      defaultParagraphAttributes.minimumFontSize,
-      std::numeric_limits<Float>::quiet_NaN());
+      sourceParagraphAttributes.minimumFontSize,
+      defaultParagraphAttributes.minimumFontSize);
  paragraphAttributes.maximumFontSize = convertRawProp(
      rawProps,
      "maximumFontSize",
-      defaultParagraphAttributes.maximumFontSize,
-      std::numeric_limits<Float>::quiet_NaN());
+      sourceParagraphAttributes.maximumFontSize,
+      defaultParagraphAttributes.maximumFontSize);

  return paragraphAttributes;
 }
--- a/ReactCommon/fabric/components/image/ImageProps.cpp
+++ b/ReactCommon/fabric/components/image/ImageProps.cpp
@ -14,20 +14,23 @@ namespace react {

 ImageProps::ImageProps(const ImageProps &sourceProps, const RawProps &rawProps)
    : ViewProps(sourceProps, rawProps),
-      sources(convertRawProp(rawProps, "source", sourceProps.sources)),
+      sources(convertRawProp(rawProps, "source", sourceProps.sources, {})),
      defaultSources(convertRawProp(
          rawProps,
          "defaultSource",
-          sourceProps.defaultSources)),
+          sourceProps.defaultSources,
+          {})),
      resizeMode(convertRawProp(
          rawProps,
          "resizeMode",
          sourceProps.resizeMode,
          ImageResizeMode::Stretch)),
      blurRadius(
-          convertRawProp(rawProps, "blurRadius", sourceProps.blurRadius)),
-      capInsets(convertRawProp(rawProps, "capInsets", sourceProps.capInsets)),
-      tintColor(convertRawProp(rawProps, "tintColor", sourceProps.tintColor)) {}
+          convertRawProp(rawProps, "blurRadius", sourceProps.blurRadius, {})),
+      capInsets(
+          convertRawProp(rawProps, "capInsets", sourceProps.capInsets, {})),
+      tintColor(
+          convertRawProp(rawProps, "tintColor", sourceProps.tintColor, {})) {}

 } // namespace react
 } // namespace facebook
--- a/ReactCommon/fabric/components/scrollview/ScrollViewProps.cpp
+++ b/ReactCommon/fabric/components/scrollview/ScrollViewProps.cpp
@ -23,11 +23,13 @@ ScrollViewProps::ScrollViewProps(
      alwaysBounceHorizontal(convertRawProp(
          rawProps,
          "alwaysBounceHorizontal",
-          sourceProps.alwaysBounceHorizontal)),
+          sourceProps.alwaysBounceHorizontal,
+          {})),
      alwaysBounceVertical(convertRawProp(
          rawProps,
          "alwaysBounceVertical",
-          sourceProps.alwaysBounceVertical)),
+          sourceProps.alwaysBounceVertical,
+          {})),
      bounces(convertRawProp(rawProps, "bounces", sourceProps.bounces, true)),
      bouncesZoom(convertRawProp(
          rawProps,
@ -37,13 +39,18 @@ ScrollViewProps::ScrollViewProps(
      canCancelContentTouches(convertRawProp(
          rawProps,
          "canCancelContentTouches",
-          sourceProps.canCancelContentTouches)),
-      centerContent(
-          convertRawProp(rawProps, "centerContent", sourceProps.centerContent)),
+          sourceProps.canCancelContentTouches,
+          true)),
+      centerContent(convertRawProp(
+          rawProps,
+          "centerContent",
+          sourceProps.centerContent,
+          {})),
      automaticallyAdjustContentInsets(convertRawProp(
          rawProps,
          "automaticallyAdjustContentInsets",
-          sourceProps.automaticallyAdjustContentInsets)),
+          sourceProps.automaticallyAdjustContentInsets,
+          {})),
      decelerationRate(convertRawProp(
          rawProps,
          "decelerationRate",
@ -52,15 +59,18 @@ ScrollViewProps::ScrollViewProps(
      directionalLockEnabled(convertRawProp(
          rawProps,
          "directionalLockEnabled",
-          sourceProps.directionalLockEnabled)),
+          sourceProps.directionalLockEnabled,
+          {})),
      indicatorStyle(convertRawProp(
          rawProps,
          "indicatorStyle",
-          sourceProps.indicatorStyle)),
+          sourceProps.indicatorStyle,
+          {})),
      keyboardDismissMode(convertRawProp(
          rawProps,
          "keyboardDismissMode",
-          sourceProps.keyboardDismissMode)),
+          sourceProps.keyboardDismissMode,
+          {})),
      maximumZoomScale(convertRawProp(
          rawProps,
          "maximumZoomScale",
@ -76,8 +86,11 @@ ScrollViewProps::ScrollViewProps(
          "scrollEnabled",
          sourceProps.scrollEnabled,
          true)),
-      pagingEnabled(
-          convertRawProp(rawProps, "pagingEnabled", sourceProps.pagingEnabled)),
+      pagingEnabled(convertRawProp(
+          rawProps,
+          "pagingEnabled",
+          sourceProps.pagingEnabled,
+          {})),
      pinchGestureEnabled(convertRawProp(
          rawProps,
          "pinchGestureEnabled",
@ -101,26 +114,33 @@ ScrollViewProps::ScrollViewProps(
      scrollEventThrottle(convertRawProp(
          rawProps,
          "scrollEventThrottle",
-          sourceProps.scrollEventThrottle)),
+          sourceProps.scrollEventThrottle,
+          {})),
      zoomScale(convertRawProp(
          rawProps,
          "zoomScale",
          sourceProps.zoomScale,
          (Float)1.0)),
-      contentInset(
-          convertRawProp(rawProps, "contentInset", sourceProps.contentInset)),
+      contentInset(convertRawProp(
+          rawProps,
+          "contentInset",
+          sourceProps.contentInset,
+          {})),
      scrollIndicatorInsets(convertRawProp(
          rawProps,
          "scrollIndicatorInsets",
-          sourceProps.scrollIndicatorInsets)),
+          sourceProps.scrollIndicatorInsets,
+          {})),
      snapToInterval(convertRawProp(
          rawProps,
          "snapToInterval",
-          sourceProps.snapToInterval)),
+          sourceProps.snapToInterval,
+          {})),
      snapToAlignment(convertRawProp(
          rawProps,
          "snapToAlignment",
-          sourceProps.snapToAlignment)) {}
+          sourceProps.snapToAlignment,
+          {})) {}

 #pragma mark - DebugStringConvertible

--- a/ReactCommon/fabric/components/text/paragraph/ParagraphProps.cpp
+++ b/ReactCommon/fabric/components/text/paragraph/ParagraphProps.cpp
@ -23,9 +23,12 @@ ParagraphProps::ParagraphProps(
    : ViewProps(sourceProps, rawProps),
      BaseTextProps(sourceProps, rawProps),
      paragraphAttributes(
-          convertRawProp(rawProps, sourceProps.paragraphAttributes)),
-      isSelectable(
-          convertRawProp(rawProps, "selectable", sourceProps.isSelectable)){};
+          convertRawProp(rawProps, sourceProps.paragraphAttributes, {})),
+      isSelectable(convertRawProp(
+          rawProps,
+          "selectable",
+          sourceProps.isSelectable,
+          {})){};

 #pragma mark - DebugStringConvertible

--- a/ReactCommon/fabric/components/text/rawtext/RawTextProps.cpp
+++ b/ReactCommon/fabric/components/text/rawtext/RawTextProps.cpp
@ -17,7 +17,7 @@ RawTextProps::RawTextProps(
    const RawTextProps &sourceProps,
    const RawProps &rawProps)
    : Props(sourceProps, rawProps),
-      text(convertRawProp(rawProps, "text", sourceProps.text)){};
+      text(convertRawProp(rawProps, "text", sourceProps.text, {})){};

 #pragma mark - DebugStringConvertible

--- a/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputProps.cpp
+++ b/ReactCommon/fabric/components/textinput/androidtextinput/AndroidTextInputProps.cpp
@ -222,7 +222,7 @@ AndroidTextInputProps::AndroidTextInputProps(
          {0})),
      text(convertRawProp(rawProps, "text", sourceProps.text, {})),
      paragraphAttributes(
-          convertRawProp(rawProps, sourceProps.paragraphAttributes)) {}
+          convertRawProp(rawProps, sourceProps.paragraphAttributes, {})) {}

 // TODO T53300085: support this in codegen; this was hand-written
 folly::dynamic AndroidTextInputProps::getDynamic() const {
--- a/ReactCommon/fabric/components/view/ViewProps.cpp
+++ b/ReactCommon/fabric/components/view/ViewProps.cpp
@ -29,48 +29,68 @@ ViewProps::ViewProps(ViewProps const &sourceProps, RawProps const &rawProps)
      foregroundColor(convertRawProp(
          rawProps,
          "foregroundColor",
-          sourceProps.foregroundColor)),
+          sourceProps.foregroundColor,
+          {})),
      backgroundColor(convertRawProp(
          rawProps,
          "backgroundColor",
-          sourceProps.backgroundColor)),
+          sourceProps.backgroundColor,
+          {})),
      borderRadii(convertRawProp(
          rawProps,
          "border",
          "Radius",
-          sourceProps.borderRadii)),
+          sourceProps.borderRadii,
+          {})),
      borderColors(convertRawProp(
          rawProps,
          "border",
          "Color",
-          sourceProps.borderColors)),
+          sourceProps.borderColors,
+          {})),
      borderStyles(convertRawProp(
          rawProps,
          "border",
          "Style",
-          sourceProps.borderStyles)),
+          sourceProps.borderStyles,
+          {})),
      shadowColor(
-          convertRawProp(rawProps, "shadowColor", sourceProps.shadowColor)),
-      shadowOffset(
-          convertRawProp(rawProps, "shadowOffset", sourceProps.shadowOffset)),
-      shadowOpacity(
-          convertRawProp(rawProps, "shadowOpacity", sourceProps.shadowOpacity)),
-      shadowRadius(
-          convertRawProp(rawProps, "shadowRadius", sourceProps.shadowRadius)),
-      transform(convertRawProp(rawProps, "transform", sourceProps.transform)),
+          convertRawProp(rawProps, "shadowColor", sourceProps.shadowColor, {})),
+      shadowOffset(convertRawProp(
+          rawProps,
+          "shadowOffset",
+          sourceProps.shadowOffset,
+          {})),
+      shadowOpacity(convertRawProp(
+          rawProps,
+          "shadowOpacity",
+          sourceProps.shadowOpacity,
+          {})),
+      shadowRadius(convertRawProp(
+          rawProps,
+          "shadowRadius",
+          sourceProps.shadowRadius,
+          {})),
+      transform(
+          convertRawProp(rawProps, "transform", sourceProps.transform, {})),
      backfaceVisibility(convertRawProp(
          rawProps,
          "backfaceVisibility",
-          sourceProps.backfaceVisibility)),
+          sourceProps.backfaceVisibility,
+          {})),
      shouldRasterize(convertRawProp(
          rawProps,
          "shouldRasterize",
-          sourceProps.shouldRasterize)),
-      zIndex(convertRawProp(rawProps, "zIndex", sourceProps.zIndex)),
-      pointerEvents(
-          convertRawProp(rawProps, "pointerEvents", sourceProps.pointerEvents)),
-      hitSlop(convertRawProp(rawProps, "hitSlop", sourceProps.hitSlop)),
-      onLayout(convertRawProp(rawProps, "onLayout", sourceProps.onLayout)),
+          sourceProps.shouldRasterize,
+          {})),
+      zIndex(convertRawProp(rawProps, "zIndex", sourceProps.zIndex, {})),
+      pointerEvents(convertRawProp(
+          rawProps,
+          "pointerEvents",
+          sourceProps.pointerEvents,
+          {})),
+      hitSlop(convertRawProp(rawProps, "hitSlop", sourceProps.hitSlop, {})),
+      onLayout(convertRawProp(rawProps, "onLayout", sourceProps.onLayout, {})),
      collapsable(convertRawProp(
          rawProps,
          "collapsable",
--- a/ReactCommon/fabric/components/view/accessibility/AccessibilityProps.cpp
+++ b/ReactCommon/fabric/components/view/accessibility/AccessibilityProps.cpp
@ -18,53 +18,67 @@ namespace react {
 AccessibilityProps::AccessibilityProps(
    AccessibilityProps const &sourceProps,
    RawProps const &rawProps)
-    : accessible(
-          convertRawProp(rawProps, "accessible", sourceProps.accessible)),
+    : accessible(convertRawProp(
+          rawProps,
+          "accessible",
+          sourceProps.accessible,
+          false)),
      accessibilityTraits(convertRawProp(
          rawProps,
          "accessibilityRole",
-          sourceProps.accessibilityTraits)),
+          sourceProps.accessibilityTraits,
+          AccessibilityTraits::None)),
      accessibilityLabel(convertRawProp(
          rawProps,
          "accessibilityLabel",
-          sourceProps.accessibilityLabel)),
+          sourceProps.accessibilityLabel,
+          "")),
      accessibilityHint(convertRawProp(
          rawProps,
          "accessibilityHint",
-          sourceProps.accessibilityHint)),
+          sourceProps.accessibilityHint,
+          "")),
      accessibilityActions(convertRawProp(
          rawProps,
          "accessibilityActions",
-          sourceProps.accessibilityActions)),
+          sourceProps.accessibilityActions,
+          {})),
      accessibilityViewIsModal(convertRawProp(
          rawProps,
          "accessibilityViewIsModal",
-          sourceProps.accessibilityViewIsModal)),
+          sourceProps.accessibilityViewIsModal,
+          false)),
      accessibilityElementsHidden(convertRawProp(
          rawProps,
          "accessibilityElementsHidden",
-          sourceProps.accessibilityElementsHidden)),
+          sourceProps.accessibilityElementsHidden,
+          false)),
      accessibilityIgnoresInvertColors(convertRawProp(
          rawProps,
          "accessibilityIgnoresInvertColors",
-          sourceProps.accessibilityIgnoresInvertColors)),
+          sourceProps.accessibilityIgnoresInvertColors,
+          false)),
      onAccessibilityTap(convertRawProp(
          rawProps,
          "onAccessibilityTap",
-          sourceProps.onAccessibilityTap)),
+          sourceProps.onAccessibilityTap,
+          {})),
      onAccessibilityMagicTap(convertRawProp(
          rawProps,
          "onAccessibilityMagicTap",
-          sourceProps.onAccessibilityMagicTap)),
+          sourceProps.onAccessibilityMagicTap,
+          {})),
      onAccessibilityEscape(convertRawProp(
          rawProps,
          "onAccessibilityEscape",
-          sourceProps.onAccessibilityEscape)),
+          sourceProps.onAccessibilityEscape,
+          {})),
      onAccessibilityAction(convertRawProp(
          rawProps,
          "onAccessibilityAction",
-          sourceProps.onAccessibilityAction)),
-      testId(convertRawProp(rawProps, "testId", sourceProps.testId)) {}
+          sourceProps.onAccessibilityAction,
+          {})),
+      testId(convertRawProp(rawProps, "testId", sourceProps.testId, "")) {}

 #pragma mark - DebugStringConvertible

--- a/ReactCommon/fabric/components/view/propsConversions.h
+++ b/ReactCommon/fabric/components/view/propsConversions.h
@ -212,29 +212,70 @@ static inline CascadedRectangleCorners<T> convertRawProp(
    RawProps const &rawProps,
    char const *prefix,
    char const *suffix,
-    CascadedRectangleCorners<T> const &sourceValue) {
+    CascadedRectangleCorners<T> const &sourceValue,
+    CascadedRectangleCorners<T> const &defaultValue) {
  CascadedRectangleCorners<T> result;

  result.topLeft = convertRawProp(
-      rawProps, "TopLeft", sourceValue.topLeft, {}, prefix, suffix);
+      rawProps,
+      "TopLeft",
+      sourceValue.topLeft,
+      defaultValue.topLeft,
+      prefix,
+      suffix);
  result.topRight = convertRawProp(
-      rawProps, "TopRight", sourceValue.topRight, {}, prefix, suffix);
+      rawProps,
+      "TopRight",
+      sourceValue.topRight,
+      defaultValue.topRight,
+      prefix,
+      suffix);
  result.bottomLeft = convertRawProp(
-      rawProps, "BottomLeft", sourceValue.bottomLeft, {}, prefix, suffix);
+      rawProps,
+      "BottomLeft",
+      sourceValue.bottomLeft,
+      defaultValue.bottomLeft,
+      prefix,
+      suffix);
  result.bottomRight = convertRawProp(
-      rawProps, "BottomRight", sourceValue.bottomRight, {}, prefix, suffix);
+      rawProps,
+      "BottomRight",
+      sourceValue.bottomRight,
+      defaultValue.bottomRight,
+      prefix,
+      suffix);

  result.topStart = convertRawProp(
-      rawProps, "TopStart", sourceValue.topStart, {}, prefix, suffix);
+      rawProps,
+      "TopStart",
+      sourceValue.topStart,
+      defaultValue.topStart,
+      prefix,
+      suffix);
  result.topEnd = convertRawProp(
-      rawProps, "TopEnd", sourceValue.topEnd, {}, prefix, suffix);
+      rawProps,
+      "TopEnd",
+      sourceValue.topEnd,
+      defaultValue.topEnd,
+      prefix,
+      suffix);
  result.bottomStart = convertRawProp(
-      rawProps, "BottomStart", sourceValue.bottomStart, {}, prefix, suffix);
+      rawProps,
+      "BottomStart",
+      sourceValue.bottomStart,
+      defaultValue.bottomStart,
+      prefix,
+      suffix);
  result.bottomEnd = convertRawProp(
-      rawProps, "BottomEnd", sourceValue.bottomEnd, {}, prefix, suffix);
+      rawProps,
+      "BottomEnd",
+      sourceValue.bottomEnd,
+      defaultValue.bottomEnd,
+      prefix,
+      suffix);

-  result.all =
-      convertRawProp(rawProps, "", sourceValue.all, {}, prefix, suffix);
+  result.all = convertRawProp(
+      rawProps, "", sourceValue.all, defaultValue.all, prefix, suffix);

  return result;
 }
@ -244,29 +285,45 @@ static inline CascadedRectangleEdges<T> convertRawProp(
    RawProps const &rawProps,
    char const *prefix,
    char const *suffix,
-    CascadedRectangleEdges<T> const &sourceValue) {
+    CascadedRectangleEdges<T> const &sourceValue,
+    CascadedRectangleEdges<T> const &defaultValue) {
  CascadedRectangleEdges<T> result;

-  result.left =
-      convertRawProp(rawProps, "Left", sourceValue.left, {}, prefix, suffix);
-  result.right =
-      convertRawProp(rawProps, "Right", sourceValue.right, {}, prefix, suffix);
-  result.top =
-      convertRawProp(rawProps, "Top", sourceValue.top, {}, prefix, suffix);
+  result.left = convertRawProp(
+      rawProps, "Left", sourceValue.left, defaultValue.left, prefix, suffix);
+  result.right = convertRawProp(
+      rawProps, "Right", sourceValue.right, defaultValue.right, prefix, suffix);
+  result.top = convertRawProp(
+      rawProps, "Top", sourceValue.top, defaultValue.top, prefix, suffix);
  result.bottom = convertRawProp(
-      rawProps, "Bottom", sourceValue.bottom, {}, prefix, suffix);
+      rawProps,
+      "Bottom",
+      sourceValue.bottom,
+      defaultValue.bottom,
+      prefix,
+      suffix);

-  result.start =
-      convertRawProp(rawProps, "Start", sourceValue.start, {}, prefix, suffix);
-  result.end =
-      convertRawProp(rawProps, "End", sourceValue.end, {}, prefix, suffix);
+  result.start = convertRawProp(
+      rawProps, "Start", sourceValue.start, defaultValue.start, prefix, suffix);
+  result.end = convertRawProp(
+      rawProps, "End", sourceValue.end, defaultValue.end, prefix, suffix);
  result.horizontal = convertRawProp(
-      rawProps, "Horizontal", sourceValue.horizontal, {}, prefix, suffix);
+      rawProps,
+      "Horizontal",
+      sourceValue.horizontal,
+      defaultValue.horizontal,
+      prefix,
+      suffix);
  result.vertical = convertRawProp(
-      rawProps, "Vertical", sourceValue.vertical, {}, prefix, suffix);
+      rawProps,
+      "Vertical",
+      sourceValue.vertical,
+      defaultValue.vertical,
+      prefix,
+      suffix);

-  result.all =
-      convertRawProp(rawProps, "", sourceValue.all, {}, prefix, suffix);
+  result.all = convertRawProp(
+      rawProps, "", sourceValue.all, defaultValue.all, prefix, suffix);

  return result;
 }
--- a/ReactCommon/fabric/core/propsConversions.h
+++ b/ReactCommon/fabric/core/propsConversions.h
@ -76,7 +76,7 @@ T convertRawProp(
    RawProps const &rawProps,
    char const *name,
    T const &sourceValue,
-    U const &defaultValue = U(),
+    U const &defaultValue,
    char const *namePrefix = nullptr,
    char const *nameSuffix = nullptr) {
  const auto *rawValue = rawProps.at(name, namePrefix, nameSuffix);
@ -101,7 +101,7 @@ static better::optional<T> convertRawProp(
    RawProps const &rawProps,
    char const *name,
    better::optional<T> const &sourceValue,
-    better::optional<T> const &defaultValue = {},
+    better::optional<T> const &defaultValue,
    char const *namePrefix = nullptr,
    char const *nameSuffix = nullptr) {
  const auto *rawValue = rawProps.at(name, namePrefix, nameSuffix);
--- a/ReactCommon/fabric/core/shadownode/Props.cpp
+++ b/ReactCommon/fabric/core/shadownode/Props.cpp
@ -14,7 +14,7 @@ namespace facebook {
 namespace react {

 Props::Props(const Props &sourceProps, const RawProps &rawProps)
-    : nativeId(convertRawProp(rawProps, "nativeID", sourceProps.nativeId)),
+    : nativeId(convertRawProp(rawProps, "nativeID", sourceProps.nativeId, {})),
      revision(sourceProps.revision + 1)
 #ifdef ANDROID
      ,