fix(iOS, Tabs): remove invalidate call in RNSTabsBottomAccessoryComponentView.didMoveToWindow

[sgaczol]

Description

RNSTabsBottomAccessoryComponentView.didMoveToWindow called [self invalidate] - this approach nilled _helper and _shadowStateProxy and also reset the state while the component was still alive. Removing this line of code is correct because React calls invalidate itself via the RCTComponentViewProtocol callback when the component is actually torn down.

Instead of that, we add [_helper unregisterForAccessoryFrameChanges]:
An object that calls this method must also (...) unregister the observer when participating in KVO.
https://developer.apple.com/documentation/objectivec/nsobject-swift.class/observevalue(forkeypath:of:change:context:)?language=objc

We also update RNSTabsBottomAccessoryContentComponentView.didMoveToWindow so it returns early if self.window == nil.

This PR fixes #3798 (comment):

After [self invalidate] nils _helper (and nothing recreates it), RNSTabsBottomAccessoryContentComponentView.didMoveToWindow ends up calling setContentView:forEnvironment: on a nil helper, so handleContentViewVisibilityForEnvironmentIfNeeded never runs to restore the correct opacities, and both copies stay visible.

NOTE: Test associated with the issue to be added soon

Changes

• RNSTabsBottomAccessoryComponentView.didMoveToWindow updated
• RNSTabsBottomAccessoryContentComponentView.didMoveToWindow updated

Before & after - visual documentation

N/A

Test plan

Make sure TabsBottomAccessory tests still work properly.

Checklist

• Included code example that can be used to test this change.
• For visual changes, included screenshots / GIFs / recordings documenting the change.
• For API changes, updated relevant public types.
• Ensured that CI passes

[Copilot]

Pull request overview

This PR adjusts the lifecycle behavior of the iOS Fabric RNSTabsBottomAccessoryComponentView to avoid calling invalidate during didMoveToWindow, preventing internal state (_helper, _shadowStateProxy, and _state) from being reset while the component may still be alive. This aligns teardown with React Native’s RCTComponentViewProtocol-driven invalidation timing.

Changes:

• Remove [self invalidate] from RNSTabsBottomAccessoryComponentView.didMoveToWindow when the view detaches from the window.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[kligarski]

From what I checked, this shouldn't be a problem. Views are not removed when detached from view hierarchy (see https://developer.apple.com/documentation/UIKit/UIViewController/viewDidUnload). AFAIK the only case when wrapper view will be deallocated is when bottom accessory component view will be destroyed and then the proper cleanup will be performed. But we can keep this in mind if something comes up in the future. I checked this example and it didn't crash:

test

import React, { createContext, useContext, useState } from 'react';
import {
  Button,
  Pressable,
  ScrollView,
  StyleSheet,
  Text,
  View,
} from 'react-native';
import type { TabsBottomAccessoryEnvironment } from 'react-native-screens';
import {
  StackContainer,
  type StackRouteConfig,
  useStackNavigationContext,
} from '@apps/shared/gamma/containers/stack';
import {
  TabsContainer,
  type TabRouteConfig,
  DEFAULT_TAB_ROUTE_OPTIONS,
} from '@apps/shared/gamma/containers/tabs';

// 1. Context stays the same, but will be provided at the root
const AccessoryContext = createContext({
  isAccessoryEnabled: true,
  toggleAccessory: () => {},
});

function AccessoryContent(environment: TabsBottomAccessoryEnvironment) {
  return (
    <View style={styles.accessory}>
      <Text>Bottom Accessory</Text>
      {environment === 'inline' && <Text>Inline</Text>}
    </View>
  );
}

function HomeTab() {
  const { push } = useStackNavigationContext();

  return (
    <View style={styles.centered}>
      <Button title="Go to test" onPress={() => push('Test')} />
    </View>
  );
}

function ExploreTab() {
  const { push } = useStackNavigationContext();

  return (
    <ScrollView
      style={styles.container}
      contentInsetAdjustmentBehavior="automatic">
      {Array.from({ length: 150 }, (_, i) => (
        <Pressable
          key={i}
          onPress={() => push('Test')}
          style={[
            styles.scrollItem,
            { backgroundColor: i % 2 ? 'black' : 'white' },
          ]}
        />
      ))}
    </ScrollView>
  );
}

const TAB_ROUTE_CONFIGS: TabRouteConfig[] = [
  {
    name: 'Home',
    Component: HomeTab,
    options: {
      ...DEFAULT_TAB_ROUTE_OPTIONS,
      title: 'Home',
    },
  },
  {
    name: 'Explore',
    Component: ExploreTab,
    options: {
      ...DEFAULT_TAB_ROUTE_OPTIONS,
      title: 'Explore',
    },
  },
];

function TabsScreen() {
  // 2. Consume the context to know whether to show the accessory
  const { isAccessoryEnabled } = useContext(AccessoryContext);

  return (
    <TabsContainer
      routeConfigs={TAB_ROUTE_CONFIGS}
      ios={{
        bottomAccessory: isAccessoryEnabled ? AccessoryContent : undefined,
        tabBarMinimizeBehavior: 'onScrollDown',
      }}
    />
  );
}

function TestScreen() {
  // 3. Consume the context here to render the toggle button
  const { isAccessoryEnabled, toggleAccessory } = useContext(AccessoryContext);

  return (
    <View style={styles.centered}>
      <Button
        title={isAccessoryEnabled ? 'Disable Accessory' : 'Enable Accessory'}
        onPress={toggleAccessory}
      />
    </View>
  );
}

const STACK_ROUTE_CONFIGS: StackRouteConfig[] = [
  {
    name: 'Tabs',
    Component: TabsScreen,
    options: {},
  },
  {
    name: 'Test',
    Component: TestScreen,
    options: {
      headerConfig: {
        title: 'Test',
      },
    },
  },
];

export default function App() {
  // 4. Lift state to the App level so both Stack routes can access it
  const [isAccessoryEnabled, setIsAccessoryEnabled] = useState(true);
  const toggleAccessory = () => setIsAccessoryEnabled(prev => !prev);

  return (
    <AccessoryContext.Provider value={{ isAccessoryEnabled, toggleAccessory }}>
      <StackContainer routeConfigs={STACK_ROUTE_CONFIGS} />
    </AccessoryContext.Provider>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
  centered: {
    flex: 1,
    alignItems: 'center',
    justifyContent: 'center',
    gap: 16,
  },
  accessory: {
    flex: 1,
    backgroundColor: 'rgba(255,0,0,0.5)',
    alignItems: 'center',
    justifyContent: 'center',
    flexDirection: 'row',
    gap: 16,
  },
  scrollItem: {
    width: '100%',
    height: 50,
  },
});

[kligarski]

On the other hand, maybe it won't hurt to call [_helper unregisterForAccessoryFrameChanges] when we're moved to window nil? Because do we have a guarantee that UIKit will keep the same wrapper view? I can't see a reason why it woudn't but we can't guarantee that. Also, when registering KVO we need to make sure we're unregistering it according to docs: https://developer.apple.com/documentation/objectivec/nsobject-swift.class/addobserver(_:forkeypath:options:context:)?language=objc.

[sgaczol]

makes sense: 57e62c5

[kligarski]

I think that we can add original minimal reproduction example as a test screen to the repo (https://gist.github.com/Ubax/8f786fad64bd0799f614f30c9326cc58).

[kligarski]

What about similar if in RNSTabsBottomAccessoryContentComponentView - we have (self.window != nil ? self : nil) threre. Maybe we should early return there if window is nil?

[kligarski]

- (void)didMoveToWindow
{
  if (self.window == nil) {
    return;
  }
  
  if ([self.superview isKindOfClass:[RNSTabsBottomAccessoryComponentView class]]) {
    RNSTabsBottomAccessoryComponentView *accessoryView =
        static_cast<RNSTabsBottomAccessoryComponentView *>(self.superview);
    _accessoryView = accessoryView;
    [_accessoryView.helper setContentView:self forEnvironment:_environment];
  } else {
    [_accessoryView.helper setContentView:nil forEnvironment:_environment];
    _accessoryView = nil;
  }
}

[sgaczol]

done: 90f9724

[kligarski]

From what I checked, this shouldn't be a problem. Views are not removed when detached from view hierarchy (see https://developer.apple.com/documentation/UIKit/UIViewController/viewDidUnload). AFAIK the only case when wrapper view will be deallocated is when bottom accessory component view will be destroyed and then the proper cleanup will be performed. But we can keep this in mind if something comes up in the future. I checked this example and it didn't crash:

test

import React, { createContext, useContext, useState } from 'react';
import {
  Button,
  Pressable,
  ScrollView,
  StyleSheet,
  Text,
  View,
} from 'react-native';
import type { TabsBottomAccessoryEnvironment } from 'react-native-screens';
import {
  StackContainer,
  type StackRouteConfig,
  useStackNavigationContext,
} from '@apps/shared/gamma/containers/stack';
import {
  TabsContainer,
  type TabRouteConfig,
  DEFAULT_TAB_ROUTE_OPTIONS,
} from '@apps/shared/gamma/containers/tabs';

// 1. Context stays the same, but will be provided at the root
const AccessoryContext = createContext({
  isAccessoryEnabled: true,
  toggleAccessory: () => {},
});

function AccessoryContent(environment: TabsBottomAccessoryEnvironment) {
  return (
    <View style={styles.accessory}>
      <Text>Bottom Accessory</Text>
      {environment === 'inline' && <Text>Inline</Text>}
    </View>
  );
}

function HomeTab() {
  const { push } = useStackNavigationContext();

  return (
    <View style={styles.centered}>
      <Button title="Go to test" onPress={() => push('Test')} />
    </View>
  );
}

function ExploreTab() {
  const { push } = useStackNavigationContext();

  return (
    <ScrollView
      style={styles.container}
      contentInsetAdjustmentBehavior="automatic">
      {Array.from({ length: 150 }, (_, i) => (
        <Pressable
          key={i}
          onPress={() => push('Test')}
          style={[
            styles.scrollItem,
            { backgroundColor: i % 2 ? 'black' : 'white' },
          ]}
        />
      ))}
    </ScrollView>
  );
}

const TAB_ROUTE_CONFIGS: TabRouteConfig[] = [
  {
    name: 'Home',
    Component: HomeTab,
    options: {
      ...DEFAULT_TAB_ROUTE_OPTIONS,
      title: 'Home',
    },
  },
  {
    name: 'Explore',
    Component: ExploreTab,
    options: {
      ...DEFAULT_TAB_ROUTE_OPTIONS,
      title: 'Explore',
    },
  },
];

function TabsScreen() {
  // 2. Consume the context to know whether to show the accessory
  const { isAccessoryEnabled } = useContext(AccessoryContext);

  return (
    <TabsContainer
      routeConfigs={TAB_ROUTE_CONFIGS}
      ios={{
        bottomAccessory: isAccessoryEnabled ? AccessoryContent : undefined,
        tabBarMinimizeBehavior: 'onScrollDown',
      }}
    />
  );
}

function TestScreen() {
  // 3. Consume the context here to render the toggle button
  const { isAccessoryEnabled, toggleAccessory } = useContext(AccessoryContext);

  return (
    <View style={styles.centered}>
      <Button
        title={isAccessoryEnabled ? 'Disable Accessory' : 'Enable Accessory'}
        onPress={toggleAccessory}
      />
    </View>
  );
}

const STACK_ROUTE_CONFIGS: StackRouteConfig[] = [
  {
    name: 'Tabs',
    Component: TabsScreen,
    options: {},
  },
  {
    name: 'Test',
    Component: TestScreen,
    options: {
      headerConfig: {
        title: 'Test',
      },
    },
  },
];

export default function App() {
  // 4. Lift state to the App level so both Stack routes can access it
  const [isAccessoryEnabled, setIsAccessoryEnabled] = useState(true);
  const toggleAccessory = () => setIsAccessoryEnabled(prev => !prev);

  return (
    <AccessoryContext.Provider value={{ isAccessoryEnabled, toggleAccessory }}>
      <StackContainer routeConfigs={STACK_ROUTE_CONFIGS} />
    </AccessoryContext.Provider>
  );
}

const styles = StyleSheet.create({
  container: {
    flex: 1,
  },
  centered: {
    flex: 1,
    alignItems: 'center',
    justifyContent: 'center',
    gap: 16,
  },
  accessory: {
    flex: 1,
    backgroundColor: 'rgba(255,0,0,0.5)',
    alignItems: 'center',
    justifyContent: 'center',
    flexDirection: 'row',
    gap: 16,
  },
  scrollItem: {
    width: '100%',
    height: 50,
  },
});

[kligarski]

On the other hand, maybe it won't hurt to call [_helper unregisterForAccessoryFrameChanges] when we're moved to window nil? Because do we have a guarantee that UIKit will keep the same wrapper view? I can't see a reason why it woudn't but we can't guarantee that. Also, when registering KVO we need to make sure we're unregistering it according to docs: https://developer.apple.com/documentation/objectivec/nsobject-swift.class/addobserver(_:forkeypath:options:context:)?language=objc.