fix(views): accept null for a removed function prop

apps/example/__tests__/nitro.views.harness.tsx

@@ -358,6 +358,50 @@ describe('TestView', () => {
     expect(view.getNativeDefaultValueSetterCallCount()).toBe(0)
   })
 
+  it('accepts a function prop being removed on a later render', async () => {
+    const viewRef = deferred<TestViewRef>()
+    const stableHybridRef = callback((view: TestViewRef) =>
+      viewRef.resolve(view)
+    )
+    const stableSomeCallback = callback(fn())
+    const renderResult = await render(
+      <TestView
+        testID="test-view-optional-callback"
+        style={INITIAL_SIZE}
+        hybridRef={stableHybridRef}
+        isBlue={false}
+        hasBeenCalled={false}
+        colorScheme="dark"
+        someCallback={stableSomeCallback}
+        someOptionalCallback={callback(fn())}
+      />,
+      { timeout: RENDER_TIMEOUT }
+    )
+
+    const view = await viewRef.promise
+    expect(view.hasSomeOptionalCallback()).toBe(true)
+
+    // Dropping a function prop sends `null` for it, which the native prop
+    // parser must treat the same as "not set".
+    const updatedViewRef = deferred<TestViewRef>()
+    await renderResult.rerender(
+      <TestView
+        testID="test-view-optional-callback"
+        style={INITIAL_SIZE}
+        hybridRef={callback((updatedView) =>
+          updatedViewRef.resolve(updatedView)
+        )}
+        isBlue={false}
+        hasBeenCalled={false}
+        colorScheme="dark"
+        someCallback={stableSomeCallback}
+      />
+    )
+
+    await updatedViewRef.promise
+    expect(view.hasSomeOptionalCallback()).toBe(false)
+  })
+
   it('only calls native setters for changed Nitro props', async () => {
     const viewRef = deferred<TestViewRef>()
     const stableHybridRef = callback((view: TestViewRef) =>

packages/react-native-nitro-modules/cpp/jsi/JSIConverter+Optional.hpp

@@ -23,7 +23,7 @@ using namespace facebook;
 template <typename TInner>
 struct JSIConverter<std::optional<TInner>> final {
   static inline std::optional<TInner> fromJSI(jsi::Runtime& runtime, const jsi::Value& arg) {
-    if (arg.isUndefined()) {
+    if (arg.isUndefined() || arg.isNull()) {
       return std::nullopt;
     } else {
       return JSIConverter<TInner>::fromJSI(runtime, arg);
@@ -37,7 +37,7 @@ struct JSIConverter<std::optional<TInner>> final {
     }
   }
   static inline bool canConvert(jsi::Runtime& runtime, const jsi::Value& value) {
-    if (value.isUndefined()) {
+    if (value.isUndefined() || value.isNull()) {
       return true;
     }
     if (JSIConverter<TInner>::canConvert(runtime, value)) {

packages/react-native-nitro-modules/cpp/views/ReactProp.hpp

@@ -102,7 +102,12 @@ class ReactProp final {
         // React Native cannot transport functions as regular props. Nitrogen
         // wraps them as `{ f: function }`, so we unwrap `f` before converting
         // and caching the JSI value.
-        value = value.asObject(*runtime).getProperty(*runtime, PropNameIDCache::get(*runtime, "f"));
+        // A function prop that was set on a previous render and is omitted on this one arrives
+        // as `null` (React diffs the removed prop as null), so only unwrap when there is an object;
+        // null/undefined fall through to the std::optional converter as "no value".
+        if (value.isObject()) {
+          value = value.asObject(*runtime).getProperty(*runtime, PropNameIDCache::get(*runtime, "f"));
+        }
       }
 
       if (previousProp.equals(*runtime, value)) {

packages/react-native-nitro-test/android/src/main/java/com/margelo/nitro/test/HybridTestView.kt

@@ -29,6 +29,7 @@ class HybridTestView(
   override var hasBeenCalled: Boolean = false
   override var colorScheme: ColorScheme = ColorScheme.LIGHT
   override var someCallback: () -> Unit = {}
+  override var someOptionalCallback: (() -> Unit)? = null
   override var nativeDefaultValue: Double? = 42.0
     set(value) {
       field = value
@@ -42,6 +43,8 @@ class HybridTestView(
 
   override fun getNativeDefaultValueSetterCallCount(): Double = nativeDefaultValueSetterCallCount
 
+  override fun hasSomeOptionalCallback(): Boolean = someOptionalCallback != null
+
   override fun someMethod() {
     hasBeenCalled = true
     someCallback()

packages/react-native-nitro-test/ios/HybridTestView.swift

@@ -26,6 +26,7 @@ class HybridTestView: HybridTestViewSpec {
   var hasBeenCalled: Bool = false
   var colorScheme: ColorScheme = .light
   var someCallback: () -> Void = {}
+  var someOptionalCallback: (() -> Void)? = nil
   var nativeDefaultValue: Double? = 42 {
     didSet {
       nativeDefaultValueSetterCallCount += 1
@@ -45,6 +46,10 @@ class HybridTestView: HybridTestViewSpec {
     return nativeDefaultValueSetterCallCount
   }
 
+  func hasSomeOptionalCallback() throws -> Bool {
+    return someOptionalCallback != nil
+  }
+
   func someMethod() throws {
     hasBeenCalled = true
     someCallback()

packages/react-native-nitro-test/nitrogen/generated/android/c++/JHybridTestViewSpec.cpp

@@ -91,6 +91,23 @@ namespace margelo::nitro::test {
     static const auto method = _javaPart->javaClassStatic()->getMethod<void(jni::alias_ref<JFunc_void::javaobject> /* someCallback */)>("setSomeCallback_cxx");
     method(_javaPart, JFunc_void_cxx::fromCpp(someCallback));
   }
+  std::optional<std::function<void()>> JHybridTestViewSpec::getSomeOptionalCallback() {
+    static const auto method = _javaPart->javaClassStatic()->getMethod<jni::local_ref<JFunc_void::javaobject>()>("getSomeOptionalCallback_cxx");
+    auto __result = method(_javaPart);
+    return __result != nullptr ? std::make_optional([&]() -> std::function<void()> {
+      if (__result->isInstanceOf(JFunc_void_cxx::javaClassStatic())) [[likely]] {
+        auto downcast = jni::static_ref_cast<JFunc_void_cxx::javaobject>(__result);
+        return downcast->cthis()->getFunction();
+      } else {
+        auto __resultRef = jni::make_global(__result);
+        return JNICallable<JFunc_void, void()>(std::move(__resultRef));
+      }
+    }()) : std::nullopt;
+  }
+  void JHybridTestViewSpec::setSomeOptionalCallback(const std::optional<std::function<void()>>& someOptionalCallback) {
+    static const auto method = _javaPart->javaClassStatic()->getMethod<void(jni::alias_ref<JFunc_void::javaobject> /* someOptionalCallback */)>("setSomeOptionalCallback_cxx");
+    method(_javaPart, someOptionalCallback.has_value() ? JFunc_void_cxx::fromCpp(someOptionalCallback.value()) : nullptr);
+  }
   std::optional<double> JHybridTestViewSpec::getNativeDefaultValue() {
     static const auto method = _javaPart->javaClassStatic()->getMethod<jni::local_ref<jni::JDouble>()>("getNativeDefaultValue");
     auto __result = method(_javaPart);
@@ -121,5 +138,10 @@ namespace margelo::nitro::test {
     static const auto method = _javaPart->javaClassStatic()->getMethod<void()>("someMethod");
     method(_javaPart);
   }
+  bool JHybridTestViewSpec::hasSomeOptionalCallback() {
+    static const auto method = _javaPart->javaClassStatic()->getMethod<jboolean()>("hasSomeOptionalCallback");
+    auto __result = method(_javaPart);
+    return static_cast<bool>(__result);
+  }
 
 } // namespace margelo::nitro::test

packages/react-native-nitro-test/nitrogen/generated/android/c++/JHybridTestViewSpec.hpp

@@ -58,6 +58,8 @@ namespace margelo::nitro::test {
     void setColorScheme(ColorScheme colorScheme) override;
     std::function<void()> getSomeCallback() override;
     void setSomeCallback(const std::function<void()>& someCallback) override;
+    std::optional<std::function<void()>> getSomeOptionalCallback() override;
+    void setSomeOptionalCallback(const std::optional<std::function<void()>>& someOptionalCallback) override;
     std::optional<double> getNativeDefaultValue() override;
     void setNativeDefaultValue(std::optional<double> nativeDefaultValue) override;
 
@@ -67,6 +69,7 @@ namespace margelo::nitro::test {
     double getIsBlueSetterCallCount() override;
     double getNativeDefaultValueSetterCallCount() override;
     void someMethod() override;
+    bool hasSomeOptionalCallback() override;
 
   private:
     jni::global_ref<JHybridTestViewSpec::JavaPart> _javaPart;

packages/react-native-nitro-test/nitrogen/generated/android/c++/views/JHybridTestViewStateUpdater.cpp

@@ -73,6 +73,11 @@ void JHybridTestViewStateUpdater::updateViewProps(jni::alias_ref<jni::JClass> /*
         : !newProps->someCallback.hasSameValue(oldProps->someCallback)) {
     hybridView->setSomeCallback(newProps->someCallback.get());
   }
+  if (oldProps == nullptr
+        ? newProps->someOptionalCallback.isProvided()
+        : !newProps->someOptionalCallback.hasSameValue(oldProps->someOptionalCallback)) {
+    hybridView->setSomeOptionalCallback(newProps->someOptionalCallback.get());
+  }
   if (oldProps == nullptr
         ? newProps->nativeDefaultValue.isProvided()
         : !newProps->nativeDefaultValue.hasSameValue(oldProps->nativeDefaultValue)) {

packages/react-native-nitro-test/nitrogen/generated/android/kotlin/com/margelo/nitro/test/HybridTestViewSpec.kt

@@ -59,6 +59,20 @@ abstract class HybridTestViewSpec: HybridView() {
       someCallback = value
     }
 
+  abstract var someOptionalCallback: (() -> Unit)?
+
+  private var someOptionalCallback_cxx: Func_void?
+    @Keep
+    @DoNotStrip
+    get() {
+      return someOptionalCallback?.let { Func_void_java(it) }
+    }
+    @Keep
+    @DoNotStrip
+    set(value) {
+      someOptionalCallback = value?.let { it }
+    }
+
   @get:DoNotStrip
   @get:Keep
   @set:DoNotStrip
@@ -81,6 +95,10 @@ abstract class HybridTestViewSpec: HybridView() {
   @DoNotStrip
   @Keep
   abstract fun someMethod(): Unit
+
+  @DoNotStrip
+  @Keep
+  abstract fun hasSomeOptionalCallback(): Boolean
 
   // Default implementation of `HybridObject.toString()`
   override fun toString(): String {

packages/react-native-nitro-test/nitrogen/generated/ios/NitroTest-Swift-Cxx-Bridge.hpp

@@ -2371,5 +2371,20 @@ namespace margelo::nitro::test::bridge::swift {
   inline Result_std__variant_std__shared_ptr_margelo__nitro__test__external__HybridSomeExternalObjectSpec___std__string__ create_Result_std__variant_std__shared_ptr_margelo__nitro__test__external__HybridSomeExternalObjectSpec___std__string__(const std::exception_ptr& error) noexcept {
     return Result<std::variant<std::shared_ptr<margelo::nitro::test::external::HybridSomeExternalObjectSpec>, std::string>>::withError(error);
   }
+
+  // pragma MARK: std::optional<std::function<void()>>
+  /**
+   * Specialized version of `std::optional<std::function<void()>>`.
+   */
+  using std__optional_std__function_void____ = std::optional<std::function<void()>>;
+  inline std::optional<std::function<void()>> create_std__optional_std__function_void____(const std::function<void()>& value) noexcept {
+    return std::optional<std::function<void()>>(value);
+  }
+  inline bool has_value_std__optional_std__function_void____(const std::optional<std::function<void()>>& optional) noexcept {
+    return optional.has_value();
+  }
+  inline std::function<void()> get_std__optional_std__function_void____(const std::optional<std::function<void()>>& optional) noexcept {
+    return optional.value();
+  }
 
 } // namespace margelo::nitro::test::bridge::swift

packages/react-native-nitro-test/nitrogen/generated/ios/c++/HybridTestViewSpecSwift.hpp

@@ -91,6 +91,13 @@ namespace margelo::nitro::test {
     inline void setSomeCallback(const std::function<void()>& someCallback) noexcept override {
       _swiftPart.setSomeCallback(someCallback);
     }
+    inline std::optional<std::function<void()>> getSomeOptionalCallback() noexcept override {
+      auto __result = _swiftPart.getSomeOptionalCallback();
+      return __result;
+    }
+    inline void setSomeOptionalCallback(const std::optional<std::function<void()>>& someOptionalCallback) noexcept override {
+      _swiftPart.setSomeOptionalCallback(someOptionalCallback);
+    }
     inline std::optional<double> getNativeDefaultValue() noexcept override {
       auto __result = _swiftPart.getNativeDefaultValue();
       return __result;
@@ -131,6 +138,14 @@ namespace margelo::nitro::test {
         std::rethrow_exception(__result.error());
       }
     }
+    inline bool hasSomeOptionalCallback() override {
+      auto __result = _swiftPart.hasSomeOptionalCallback();
+      if (__result.hasError()) [[unlikely]] {
+        std::rethrow_exception(__result.error());
+      }
+      auto __value = std::move(__result.value());
+      return __value;
+    }
 
   private:
     NitroTest::HybridTestViewSpec_cxx _swiftPart;

packages/react-native-nitro-test/nitrogen/generated/ios/c++/views/HybridTestViewComponent.mm

@@ -122,6 +122,12 @@ - (void) updateProps:(const std::shared_ptr<const react::Props>&)props
           : !newViewProps.someCallback.hasSameValue(oldViewProps->someCallback)) {
       swiftPart.setSomeCallback(newViewProps.someCallback.get());
     }
+    // someOptionalCallback: optional
+    if (oldViewProps == nullptr
+          ? newViewProps.someOptionalCallback.isProvided()
+          : !newViewProps.someOptionalCallback.hasSameValue(oldViewProps->someOptionalCallback)) {
+      swiftPart.setSomeOptionalCallback(newViewProps.someOptionalCallback.get());
+    }
     // nativeDefaultValue: optional
     if (oldViewProps == nullptr
           ? newViewProps.nativeDefaultValue.isProvided()

packages/react-native-nitro-test/nitrogen/generated/ios/swift/HybridTestViewSpec.swift

@@ -14,13 +14,15 @@ public protocol HybridTestViewSpec_protocol: HybridObject, HybridView {
   var hasBeenCalled: Bool { get set }
   var colorScheme: ColorScheme { get set }
   var someCallback: () -> Void { get set }
+  var someOptionalCallback: (() -> Void)? { get set }
   var nativeDefaultValue: Double? { get set }
 
   // Methods
   func getOnDropViewCount() throws -> Double
   func getIsBlueSetterCallCount() throws -> Double
   func getNativeDefaultValueSetterCallCount() throws -> Double
   func someMethod() throws -> Void
+  func hasSomeOptionalCallback() throws -> Bool
 }
 
 public extension HybridTestViewSpec_protocol {

packages/react-native-nitro-test/nitrogen/generated/ios/swift/HybridTestViewSpec_cxx.swift

@@ -173,6 +173,38 @@ open class HybridTestViewSpec_cxx {
     }
   }
 
+  public final var someOptionalCallback: bridge.std__optional_std__function_void____ {
+    @inline(__always)
+    get {
+      return { () -> bridge.std__optional_std__function_void____ in
+        if let __unwrappedValue = self.__implementation.someOptionalCallback {
+          return bridge.create_std__optional_std__function_void____({ () -> bridge.Func_void in
+            let __closureWrapper = Func_void(__unwrappedValue)
+            return bridge.create_Func_void(__closureWrapper.toUnsafe())
+          }())
+        } else {
+          return .init()
+        }
+      }()
+    }
+    @inline(__always)
+    set {
+      self.__implementation.someOptionalCallback = { () -> (() -> Void)? in
+        if bridge.has_value_std__optional_std__function_void____(newValue) {
+          let __unwrapped = bridge.get_std__optional_std__function_void____(newValue)
+          return { () -> () -> Void in
+            let __wrappedFunction = bridge.wrap_Func_void(__unwrapped)
+            return { () -> Void in
+              __wrappedFunction.call()
+            }
+          }()
+        } else {
+          return nil
+        }
+      }()
+    }
+  }
+
   public final var nativeDefaultValue: bridge.std__optional_double_ {
     @inline(__always)
     get {
@@ -245,6 +277,18 @@ open class HybridTestViewSpec_cxx {
     }
   }
 
+  @inline(__always)
+  public final func hasSomeOptionalCallback() -> bridge.Result_bool_ {
+    do {
+      let __result = try self.__implementation.hasSomeOptionalCallback()
+      let __resultCpp = __result
+      return bridge.create_Result_bool_(__resultCpp)
+    } catch (let __error) {
+      let __exceptionPtr = __error.toCpp()
+      return bridge.create_Result_bool_(__exceptionPtr)
+    }
+  }
+
   public final func getView() -> UnsafeMutableRawPointer {
     return Unmanaged.passRetained(__implementation.view).toOpaque()
   }

packages/react-native-nitro-test/nitrogen/generated/shared/c++/HybridTestViewSpec.cpp

@@ -22,12 +22,15 @@ namespace margelo::nitro::test {
       prototype.registerHybridSetter("colorScheme", &HybridTestViewSpec::setColorScheme);
       prototype.registerHybridGetter("someCallback", &HybridTestViewSpec::getSomeCallback);
       prototype.registerHybridSetter("someCallback", &HybridTestViewSpec::setSomeCallback);
+      prototype.registerHybridGetter("someOptionalCallback", &HybridTestViewSpec::getSomeOptionalCallback);
+      prototype.registerHybridSetter("someOptionalCallback", &HybridTestViewSpec::setSomeOptionalCallback);
       prototype.registerHybridGetter("nativeDefaultValue", &HybridTestViewSpec::getNativeDefaultValue);
       prototype.registerHybridSetter("nativeDefaultValue", &HybridTestViewSpec::setNativeDefaultValue);
       prototype.registerHybridMethod("getOnDropViewCount", &HybridTestViewSpec::getOnDropViewCount);
       prototype.registerHybridMethod("getIsBlueSetterCallCount", &HybridTestViewSpec::getIsBlueSetterCallCount);
       prototype.registerHybridMethod("getNativeDefaultValueSetterCallCount", &HybridTestViewSpec::getNativeDefaultValueSetterCallCount);
       prototype.registerHybridMethod("someMethod", &HybridTestViewSpec::someMethod);
+      prototype.registerHybridMethod("hasSomeOptionalCallback", &HybridTestViewSpec::hasSomeOptionalCallback);
     });
   }
 

packages/react-native-nitro-test/nitrogen/generated/shared/c++/HybridTestViewSpec.hpp

@@ -55,6 +55,8 @@ namespace margelo::nitro::test {
       virtual void setColorScheme(ColorScheme colorScheme) = 0;
       virtual std::function<void()> getSomeCallback() = 0;
       virtual void setSomeCallback(const std::function<void()>& someCallback) = 0;
+      virtual std::optional<std::function<void()>> getSomeOptionalCallback() = 0;
+      virtual void setSomeOptionalCallback(const std::optional<std::function<void()>>& someOptionalCallback) = 0;
       virtual std::optional<double> getNativeDefaultValue() = 0;
       virtual void setNativeDefaultValue(std::optional<double> nativeDefaultValue) = 0;
 
@@ -64,6 +66,7 @@ namespace margelo::nitro::test {
       virtual double getIsBlueSetterCallCount() = 0;
       virtual double getNativeDefaultValueSetterCallCount() = 0;
       virtual void someMethod() = 0;
+      virtual bool hasSomeOptionalCallback() = 0;
 
     protected:
       // Hybrid Setup