fix: Re-apply all props when Fabric re-creates a HybridView's native View

example/__tests__/views.harness.tsx

@@ -0,0 +1,51 @@
+import { Activity } from 'react'
+import { Platform } from 'react-native'
+import { describe, expect, it, render, waitUntil } from 'react-native-harness'
+import { callback } from 'react-native-nitro-modules'
+import { TestView, type TestViewRef } from 'react-native-nitro-test'
+
+const refs: TestViewRef[] = []
+const hybridRef = callback((ref: TestViewRef) => {
+  refs.push(ref)
+})
+
+// Stable element - React does not re-render it, so its ShadowNode never changes.
+const testView = (
+  <TestView
+    isBlue={true}
+    hasBeenCalled={false}
+    colorScheme="light"
+    someCallback={callback(() => {})}
+    hybridRef={hybridRef}
+    style={{ width: 20, height: 20 }}
+  />
+)
+const renderTestView = (visible: boolean) => (
+  <Activity mode={visible ? 'visible' : 'hidden'}>{testView}</Activity>
+)
+
+// On iOS, hiding a subtree drops its native Views and showing it again creates new ones - for
+// ShadowNodes that did not change. This is what react-native-screens does when a screen is
+// detached and re-attached. On Android the native View survives being hidden, so there is
+// nothing to re-create there. See https://github.com/mrousavy/nitro/issues/1380
+const itOnIos = Platform.OS === 'ios' ? it : it.skip
+
+describe('HybridView', () => {
+  itOnIos('applies all props to a native View that Fabric re-created', async () => {
+    refs.length = 0
+
+    const { rerender, unmount } = await render(renderTestView(true))
+    await waitUntil(() => refs.length === 1)
+    expect(refs[0]!.isBlue).toBe(true)
+
+    await rerender(renderTestView(false))
+    await rerender(renderTestView(true))
+
+    // A new native View was created, so `hybridRef` fires again with the new HybridView...
+    await waitUntil(() => refs.length === 2)
+    // ...and that new HybridView received `isBlue` again, instead of keeping its native default.
+    expect(refs[1]!.isBlue).toBe(true)
+
+    unmount()
+  })
+})

packages/nitrogen/src/views/kotlin/KotlinHybridViewManager.ts

@@ -49,6 +49,7 @@ import com.facebook.react.uimanager.SimpleViewManager
 import com.facebook.react.uimanager.StateWrapper
 import com.facebook.react.uimanager.ThemedReactContext
 import com.margelo.nitro.R.id.associated_hybrid_view_tag
+import com.margelo.nitro.R.id.needs_full_props_update_tag
 import com.margelo.nitro.views.RecyclableView
 import ${javaNamespace}.*
 
@@ -71,19 +72,27 @@ public class ${manager}: SimpleViewManager<View>() {
     val hybridView = ${viewImplementation}(reactContext)
     val view = hybridView.view
     view.setTag(associated_hybrid_view_tag, hybridView)
+    view.setTag(needs_full_props_update_tag, true)
     return view
   }
 
   override fun updateState(view: View, props: ReactStylesDiffMap, stateWrapper: StateWrapper): Any? {
     val hybridView = getHybridView(view)
       ?: throw Error("Couldn't find view $view in local views table!")
 
-    // 1. Update each prop individually
+    // 1. \`isDirty\` only tells us whether a prop changed in the ShadowTree - not whether it has
+    //    ever been applied to this View. Fabric can create a new View for a ShadowNode that did
+    //    not change (e.g. when a subtree is hidden and shown again), in which case no prop would
+    //    be dirty at all. A newly created (or recycled) View therefore applies all props once.
+    val forceUpdate = view.getTag(needs_full_props_update_tag) as? Boolean ?: true
+    view.setTag(needs_full_props_update_tag, false)
+
+    // 2. Update each prop individually
     hybridView.beforeUpdate()
-    ${stateUpdaterName}.updateViewProps(hybridView, stateWrapper)
+    ${stateUpdaterName}.updateViewProps(hybridView, stateWrapper, forceUpdate)
     hybridView.afterUpdate()
 
-    // 2. Continue in base View props
+    // 3. Continue in base View props
     return super.updateState(view, props, stateWrapper)
   }
 
@@ -103,6 +112,9 @@ public class ${manager}: SimpleViewManager<View>() {
       // Recycle in it's implementation
       hybridView.prepareForRecycle()
 
+      // This View will be re-used for a different ShadowNode later on, so it needs all props again.
+      hybridView.view.setTag(needs_full_props_update_tag, true)
+
       // Maybe update the view if it changed
       return hybridView.view
     } else {
@@ -132,7 +144,7 @@ internal class ${stateUpdaterName} {
      */
     @Suppress("KotlinJniMissingFunction")
     @JvmStatic
-    external fun updateViewProps(view: ${HybridTSpec}, state: StateWrapper)
+    external fun updateViewProps(view: ${HybridTSpec}, state: StateWrapper, forceUpdate: Boolean)
   }
 }
   `.trim()
@@ -171,7 +183,8 @@ public:
 public:
   static void updateViewProps(jni::alias_ref<jni::JClass> /* class */,
                               jni::alias_ref<${JHybridTSpec}::JavaPart> view,
-                              jni::alias_ref<JStateWrapper::javaobject> stateWrapperInterface);
+                              jni::alias_ref<JStateWrapper::javaobject> stateWrapperInterface,
+                              jboolean forceUpdate);
 
 public:
   static void registerNatives() {
@@ -193,7 +206,7 @@ public:
     const name = escapeCppName(p.name)
     const setter = p.getSetterName('other')
     return `
-if (props->${name}.isDirty) {
+if ((forceUpdate && props->${name}.hasValue()) || props->${name}.isDirty) {
   hybridView->${setter}(props->${name}.value);
   props->${name}.isDirty = false;
 }
@@ -214,7 +227,8 @@ using ConcreteStateData = react::ConcreteState<${stateClassName}>;
 
 void J${stateUpdaterName}::updateViewProps(jni::alias_ref<jni::JClass> /* class */,
                                            jni::alias_ref<${JHybridTSpec}::JavaPart> javaView,
-                                           jni::alias_ref<JStateWrapper::javaobject> stateWrapperInterface) {
+                                           jni::alias_ref<JStateWrapper::javaobject> stateWrapperInterface,
+                                           jboolean forceUpdate) {
   std::shared_ptr<${JHybridTSpec}> hybridView = javaView->get${JHybridTSpec}();
 
   // Get concrete StateWrapperImpl from passed StateWrapper interface object
@@ -237,7 +251,7 @@ void J${stateUpdaterName}::updateViewProps(jni::alias_ref<jni::JClass> /* class
   ${indent(propsUpdaterCalls.join('\n'), '  ')}
 
   // Update hybridRef if it changed
-  if (props->hybridRef.isDirty) {
+  if ((forceUpdate && props->hybridRef.hasValue()) || props->hybridRef.isDirty) {
     // hybridRef changed - call it with new this
     const auto& maybeFunc = props->hybridRef.value;
     if (maybeFunc.has_value()) {

packages/nitrogen/src/views/swift/SwiftHybridViewManager.ts

@@ -45,7 +45,7 @@ export function createSwiftHybridViewManager(
     )
     return `
 // ${p.jsSignature}
-if (newViewProps.${name}.isDirty) {
+if ((forceUpdate && newViewProps.${name}.hasValue()) || newViewProps.${name}.isDirty) {
   swiftPart.${setter}(${indent(parse, '  ')});
   newViewProps.${name}.isDirty = false;
 }
@@ -87,6 +87,7 @@ using namespace ${namespace}::views;
 
 @implementation ${component} {
   std::shared_ptr<${HybridTSpecSwift}> _hybridView;
+  BOOL _didUpdateProps;
 }
 
 + (void) load {
@@ -126,15 +127,22 @@ using namespace ${namespace}::views;
   auto& newViewProps = const_cast<${propsClassName}&>(newViewPropsConst);
   ${swiftNamespace}::${HybridTSpecCxx}& swiftPart = _hybridView->getSwiftPart();
 
-  // 2. Update each prop individually
+  // 2. \`isDirty\` only tells us whether a prop changed in the ShadowTree - not whether it has
+  //    ever been applied to *this* View. Fabric can create a new View for a ShadowNode that
+  //    did not change (e.g. when a subtree is hidden and shown again), in which case no prop
+  //    would be dirty at all. A newly created (or recycled) View therefore applies all props once.
+  const bool forceUpdate = !_didUpdateProps;
+  _didUpdateProps = YES;
+
+  // 3. Update each prop individually
   swiftPart.beforeUpdate();
 
   ${indent(propAssignments.join('\n'), '  ')}
 
   swiftPart.afterUpdate();
 
-  // 3. Update hybridRef if it changed
-  if (newViewProps.hybridRef.isDirty) {
+  // 4. Update hybridRef if it changed
+  if ((forceUpdate && newViewProps.hybridRef.hasValue()) || newViewProps.hybridRef.isDirty) {
     // hybridRef changed - call it with new this
     const auto& maybeFunc = newViewProps.hybridRef.value;
     if (maybeFunc.has_value()) {
@@ -143,7 +151,7 @@ using namespace ${namespace}::views;
     newViewProps.hybridRef.isDirty = false;
   }
 
-  // 4. Continue in base class
+  // 5. Continue in base class
   [super updateProps:props oldProps:oldProps];
 }
 
@@ -153,6 +161,8 @@ using namespace ${namespace}::views;
 
 - (void)prepareForRecycle {
   [super prepareForRecycle];
+  // This View will be re-used for a different ShadowNode later on, so it needs all props again.
+  _didUpdateProps = NO;
   ${swiftNamespace}::${HybridTSpecCxx}& swiftPart = _hybridView->getSwiftPart();
   swiftPart.maybePrepareForRecycle();
 }

packages/react-native-nitro-modules/android/src/main/res/values/ids.xml

@@ -1,4 +1,5 @@
 <?xml version="1.0" encoding="utf-8"?>
 <resources>
     <item type="id" name="associated_hybrid_view_tag"/>
+    <item type="id" name="needs_full_props_update_tag"/>
 </resources>

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

@@ -33,6 +33,15 @@ struct CachedProp {
   BorrowingReference<jsi::Value> jsiValue;
 
 public:
+  /**
+   * Whether this prop ever received a value from JS.
+   * A prop that was never set by JS still holds a default-constructed `value`,
+   * which must not be applied to the View.
+   */
+  bool hasValue() const noexcept {
+    return jsiValue != nullptr;
+  }
+
   bool equals(jsi::Runtime& runtime, const jsi::Value& other) const {
     if (jsiValue == nullptr) {
       return false;

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

@@ -17,7 +17,8 @@ using ConcreteStateData = react::ConcreteState<HybridRecyclableTestViewState>;
 
 void JHybridRecyclableTestViewStateUpdater::updateViewProps(jni::alias_ref<jni::JClass> /* class */,
                                            jni::alias_ref<JHybridRecyclableTestViewSpec::JavaPart> javaView,
-                                           jni::alias_ref<JStateWrapper::javaobject> stateWrapperInterface) {
+                                           jni::alias_ref<JStateWrapper::javaobject> stateWrapperInterface,
+                                           jboolean forceUpdate) {
   std::shared_ptr<JHybridRecyclableTestViewSpec> hybridView = javaView->getJHybridRecyclableTestViewSpec();
 
   // Get concrete StateWrapperImpl from passed StateWrapper interface object
@@ -37,13 +38,13 @@ void JHybridRecyclableTestViewStateUpdater::updateViewProps(jni::alias_ref<jni::
   }
 
   // Update all props if they are dirty
-  if (props->isBlue.isDirty) {
+  if ((forceUpdate && props->isBlue.hasValue()) || props->isBlue.isDirty) {
     hybridView->setIsBlue(props->isBlue.value);
     props->isBlue.isDirty = false;
   }
 
   // Update hybridRef if it changed
-  if (props->hybridRef.isDirty) {
+  if ((forceUpdate && props->hybridRef.hasValue()) || props->hybridRef.isDirty) {
     // hybridRef changed - call it with new this
     const auto& maybeFunc = props->hybridRef.value;
     if (maybeFunc.has_value()) {

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

@@ -31,7 +31,8 @@ class JHybridRecyclableTestViewStateUpdater final: public jni::JavaClass<JHybrid
 public:
   static void updateViewProps(jni::alias_ref<jni::JClass> /* class */,
                               jni::alias_ref<JHybridRecyclableTestViewSpec::JavaPart> view,
-                              jni::alias_ref<JStateWrapper::javaobject> stateWrapperInterface);
+                              jni::alias_ref<JStateWrapper::javaobject> stateWrapperInterface,
+                              jboolean forceUpdate);
 
 public:
   static void registerNatives() {

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

@@ -17,7 +17,8 @@ using ConcreteStateData = react::ConcreteState<HybridTestViewState>;
 
 void JHybridTestViewStateUpdater::updateViewProps(jni::alias_ref<jni::JClass> /* class */,
                                            jni::alias_ref<JHybridTestViewSpec::JavaPart> javaView,
-                                           jni::alias_ref<JStateWrapper::javaobject> stateWrapperInterface) {
+                                           jni::alias_ref<JStateWrapper::javaobject> stateWrapperInterface,
+                                           jboolean forceUpdate) {
   std::shared_ptr<JHybridTestViewSpec> hybridView = javaView->getJHybridTestViewSpec();
 
   // Get concrete StateWrapperImpl from passed StateWrapper interface object
@@ -37,25 +38,25 @@ void JHybridTestViewStateUpdater::updateViewProps(jni::alias_ref<jni::JClass> /*
   }
 
   // Update all props if they are dirty
-  if (props->isBlue.isDirty) {
+  if ((forceUpdate && props->isBlue.hasValue()) || props->isBlue.isDirty) {
     hybridView->setIsBlue(props->isBlue.value);
     props->isBlue.isDirty = false;
   }
-  if (props->hasBeenCalled.isDirty) {
+  if ((forceUpdate && props->hasBeenCalled.hasValue()) || props->hasBeenCalled.isDirty) {
     hybridView->setHasBeenCalled(props->hasBeenCalled.value);
     props->hasBeenCalled.isDirty = false;
   }
-  if (props->colorScheme.isDirty) {
+  if ((forceUpdate && props->colorScheme.hasValue()) || props->colorScheme.isDirty) {
     hybridView->setColorScheme(props->colorScheme.value);
     props->colorScheme.isDirty = false;
   }
-  if (props->someCallback.isDirty) {
+  if ((forceUpdate && props->someCallback.hasValue()) || props->someCallback.isDirty) {
     hybridView->setSomeCallback(props->someCallback.value);
     props->someCallback.isDirty = false;
   }
 
   // Update hybridRef if it changed
-  if (props->hybridRef.isDirty) {
+  if ((forceUpdate && props->hybridRef.hasValue()) || props->hybridRef.isDirty) {
     // hybridRef changed - call it with new this
     const auto& maybeFunc = props->hybridRef.value;
     if (maybeFunc.has_value()) {

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

@@ -31,7 +31,8 @@ class JHybridTestViewStateUpdater final: public jni::JavaClass<JHybridTestViewSt
 public:
   static void updateViewProps(jni::alias_ref<jni::JClass> /* class */,
                               jni::alias_ref<JHybridTestViewSpec::JavaPart> view,
-                              jni::alias_ref<JStateWrapper::javaobject> stateWrapperInterface);
+                              jni::alias_ref<JStateWrapper::javaobject> stateWrapperInterface,
+                              jboolean forceUpdate);
 
 public:
   static void registerNatives() {

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

@@ -13,6 +13,7 @@ import com.facebook.react.uimanager.SimpleViewManager
 import com.facebook.react.uimanager.StateWrapper
 import com.facebook.react.uimanager.ThemedReactContext
 import com.margelo.nitro.R.id.associated_hybrid_view_tag
+import com.margelo.nitro.R.id.needs_full_props_update_tag
 import com.margelo.nitro.views.RecyclableView
 import com.margelo.nitro.test.*
 
@@ -35,19 +36,27 @@ public class HybridRecyclableTestViewManager: SimpleViewManager<View>() {
     val hybridView = HybridRecyclableTestView(reactContext)
     val view = hybridView.view
     view.setTag(associated_hybrid_view_tag, hybridView)
+    view.setTag(needs_full_props_update_tag, true)
     return view
   }
 
   override fun updateState(view: View, props: ReactStylesDiffMap, stateWrapper: StateWrapper): Any? {
     val hybridView = getHybridView(view)
       ?: throw Error("Couldn't find view $view in local views table!")
 
-    // 1. Update each prop individually
+    // 1. `isDirty` only tells us whether a prop changed in the ShadowTree - not whether it has
+    //    ever been applied to this View. Fabric can create a new View for a ShadowNode that did
+    //    not change (e.g. when a subtree is hidden and shown again), in which case no prop would
+    //    be dirty at all. A newly created (or recycled) View therefore applies all props once.
+    val forceUpdate = view.getTag(needs_full_props_update_tag) as? Boolean ?: true
+    view.setTag(needs_full_props_update_tag, false)
+
+    // 2. Update each prop individually
     hybridView.beforeUpdate()
-    HybridRecyclableTestViewStateUpdater.updateViewProps(hybridView, stateWrapper)
+    HybridRecyclableTestViewStateUpdater.updateViewProps(hybridView, stateWrapper, forceUpdate)
     hybridView.afterUpdate()
 
-    // 2. Continue in base View props
+    // 3. Continue in base View props
     return super.updateState(view, props, stateWrapper)
   }
 
@@ -67,6 +76,9 @@ public class HybridRecyclableTestViewManager: SimpleViewManager<View>() {
       // Recycle in it's implementation
       hybridView.prepareForRecycle()
 
+      // This View will be re-used for a different ShadowNode later on, so it needs all props again.
+      hybridView.view.setTag(needs_full_props_update_tag, true)
+
       // Maybe update the view if it changed
       return hybridView.view
     } else {

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

@@ -18,6 +18,6 @@ internal class HybridRecyclableTestViewStateUpdater {
      */
     @Suppress("KotlinJniMissingFunction")
     @JvmStatic
-    external fun updateViewProps(view: HybridRecyclableTestViewSpec, state: StateWrapper)
+    external fun updateViewProps(view: HybridRecyclableTestViewSpec, state: StateWrapper, forceUpdate: Boolean)
   }
 }