From 778c8ee8d9b835a43d23395a6468257e739c7f07 Mon Sep 17 00:00:00 2001 From: Kacper Kafara Date: Tue, 15 Oct 2024 11:57:10 +0200 Subject: [PATCH] fix(iOS): `activityState` regression check false-positive (#2404) ## Description We had an issue introduced in #2389 that added the preload. We temporarily have an assumption in native stack, that the `activityState` can only progress when `Screen` component is rendered (directly) inside native stack navigator. This invariant was enforced with appropriate checks on native side. It turns out that we were wrong in our way of checking whether the `Screen` belongs to native stack or not - we have forgotten about `RNSContainerNavigationController` class that is used when rendering `ScreenContainer` with `hasTwoStates: true` and which inherits from `RNSNavigationController`. ## Changes ~Added a `isNativeStackViewController` method to `RNSNavigationController` and overrode it in subclass.~ Added `RNSScreenView#isNativeStackScreen` method, which checks whether our direct react superview is a `RNSScreenStackView`. I've found checking for `_controller.navigationController` not reliable, as `navigationController` can be found arbitrarily high in the view hierarchy. ## Test code and steps to reproduce Reported by Expo. If you want to reproduce this: create a JS stack navigator with bottom tabs inside (with `hasTwoStates: true`), and then a regular JS stack inside & try to navigate to any screen. We have not caught that earlier, because we're not testing regular JS stack at all. ## Checklist - [ ] Included code example that can be used to test this change - [x] Ensured that CI passes --- ios/RNSScreen.mm | 18 ++++++++++++++---- ios/RNSScreenStack.mm | 2 +- 2 files changed, 15 insertions(+), 5 deletions(-) diff --git a/ios/RNSScreen.mm b/ios/RNSScreen.mm index 309a338f6..4c4619422 100644 --- a/ios/RNSScreen.mm +++ b/ios/RNSScreen.mm @@ -307,15 +307,25 @@ - (void)setActivityStateOrNil:(NSNumber *)activityStateOrNil { int activityState = [activityStateOrNil intValue]; if (activityStateOrNil != nil && activityState != -1 && activityState != _activityState) { - if ([_controller.navigationController isKindOfClass:RNSNavigationController.class] && - _activityState < activityState) { - RCTLogError(@"[RNScreens] activityState can only progress in NativeStack"); - } + [self maybeAssertActivityStateProgressionOldValue:_activityState newValue:activityState]; _activityState = activityState; [_reactSuperview markChildUpdated]; } } +- (void)maybeAssertActivityStateProgressionOldValue:(int)oldValue newValue:(int)newValue +{ + if (self.isNativeStackScreen && newValue < oldValue) { + RCTLogError(@"[RNScreens] activityState can only progress in NativeStack"); + } +} + +/// Note that this method works only after the screen is actually mounted under a screen stack view. +- (BOOL)isNativeStackScreen +{ + return [_reactSuperview isKindOfClass:RNSScreenStackView.class]; +} + #if !TARGET_OS_TV && !TARGET_OS_VISION - (void)setStatusBarStyle:(RNSStatusBarStyle)statusBarStyle { diff --git a/ios/RNSScreenStack.mm b/ios/RNSScreenStack.mm index 98903d176..8fa4fbb7b 100644 --- a/ios/RNSScreenStack.mm +++ b/ios/RNSScreenStack.mm @@ -953,7 +953,7 @@ - (void)navigationController:(UINavigationController *)navigationController - (void)markChildUpdated { - // do nothing + // In native stack this should be called only for `preload` purposes. [self updateContainer]; }