Skip to content

Commit

Permalink
fix(iOS): activityState regression check false-positive (#2404)
Browse files Browse the repository at this point in the history
## 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
  • Loading branch information
kkafar authored Oct 15, 2024
1 parent e4333a1 commit 778c8ee
Show file tree
Hide file tree
Showing 2 changed files with 15 additions and 5 deletions.
18 changes: 14 additions & 4 deletions ios/RNSScreen.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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
{
Expand Down
2 changes: 1 addition & 1 deletion ios/RNSScreenStack.mm
Original file line number Diff line number Diff line change
Expand Up @@ -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];
}

Expand Down

0 comments on commit 778c8ee

Please sign in to comment.