KMP-ObservableViewModel icon indicating copy to clipboard operation
KMP-ObservableViewModel copied to clipboard

ViewModel is not cleared on iOS 17.4

Open osrl opened this issue 1 year ago • 11 comments

I was testing on iOS 17.0 before and everything was working fine.

I'm using @StateViewModel. If there's anything more I can provide please let me know.

Edit: My code works fine on 17.2 and doesn't work on 17.4. But I couldn't reproduce it on the sample app. Looks like it's a bug on my code. It might be related to koin but I'm not sure.

Do you have any idea @rickclephas

Edit: It's not related to koin either. I've copied your TimeTravelViewModel and inited it just like you did

struct ChatScreen: View {
    @StateViewModel var timeViewModel = TimeTravelViewModel()
   
    let printer = DeallocPrinter()
}
class DeallocPrinter {
    deinit {
        print("ChatScreen deallocated")
    }
}

Even if the ChatScreen is deallocated, timeViewModel is not deallocated and not cleared.

osrl avatar Aug 23 '24 15:08 osrl

Could you possibly provide a minimal reproducible sample?

rickclephas avatar Aug 23 '24 17:08 rickclephas

I tried to do that with your sample but it worked as expected. I will probably fix the issue when I can reproduce it 😅 All I know is it works fine on 17.2, not on 17.4

osrl avatar Aug 23 '24 17:08 osrl

Hi again @rickclephas . I've found out very interesting results. Here is my reproducible sample:

I've added some flags:

let useNavigationStack = false
let useScrollView = true
let useTextInput = true
// Pressing dissmiss instead of back

If any of these flags are different then it is, it works fine. Also, pressing Back button on navigation bar works fine too. You should press Dismiss button to see it not clearing. This looks like a very interesting iOS bug but I'm not sure.

By the way, with these combinations, it also doesn't work on iOS 17.2 or below. I'm using NavigationStack on my production app. But I couldn't make it "not work" with NavigationStack on your sample app.

osrl avatar Aug 24 '24 11:08 osrl

Okay i've added one more flag and reproduced version issue with NavigationStack and NavigationView (doesn't matter for this sample) . I think this is a strong reference issue but it works on 17.2 and below. Also back button or dismiss button doesn't make any difference.

let useNavigationStack = true //doesn't matter
let setDeeplinkHandler = true

As you can see in the sample, ContentView doesn't even have the reference to the DeeplinkHandler but it still causes the problem.

osrl avatar Aug 24 '24 13:08 osrl

Thanks for the reproducer! I am not exactly sure what changed in iOS 17.3, but the root cause is in the deeplink logic:

.onAppear {
    if setDeeplinkHandler {
        selectedChat = deeplinkHandler.chatId
    }
}

This logic is run right before the chat screen is closed. If deeplinkHandler.chatId == nil then the ViewModel isn't cleared, if it isn't nil the ViewModel is correctly cleared. So it looks like the indirect change of shouldPresentChat to false during the back navigation is causing this issue.

Another strange thing I noticed is that a non-null chatId in the DeeplinkHandler doesn't open the chat screen. I guess that hides another issue, where you wouldn't be able to navigate to back to the chat list if a deep link was used.

Making sure the deeplink logic in the onAppear only executes once, solves the ViewModel clearing issue, and should also allow you to navigate back.

Not sure why the initial deeplink navigation isn't working though.

rickclephas avatar Aug 25 '24 11:08 rickclephas

Btw using a verify basic ObservableObject results in the same issue:

class TestObject: ObservableObject {
    
    init() {
        print("TestObject init")
    }
    
    deinit {
        print("TestObject deinint")
    }
}

struct ContentView: View {
    @StateObject var testObject = TestObject()
}

It seems something has changed in SwiftUI that for some reason keeps state objects in memory when the isPresented state is being modified during back navigation.

rickclephas avatar Aug 25 '24 11:08 rickclephas

thanks for the comments @rickclephas . the first reproducer commit may need another issue. it also has very interesting findings. using scrollview, textinput or navigation view causes the same issue. did you have a chance to check it out?

osrl avatar Aug 26 '24 07:08 osrl

Hmm I can't seem to reproduce it with that one. Both the back and dismiss button correctly log the init, onAppear, onDisappear and clear messages. Tested on iOS (simulator) 17.0.1 and 17.4 with Xcode 15.4.

rickclephas avatar Aug 26 '24 19:08 rickclephas

are you sure these flags are set:

let useNavigationStack = false
let useScrollView = true
let useTextInput = true

I've tested on the same iOS and Xcode versions but it doesn't log the clear message. other messages are fine.

Screenshot 2024-08-27 at 16 00 40

please be sure to checkout commit 46f11e3

osrl avatar Aug 27 '24 13:08 osrl

Yeah double checked the flags and commit. Clicking on "Next Screen" and "Dismiss" results in: image

rickclephas avatar Aug 27 '24 16:08 rickclephas

well that is very interesting. I'm still getting the same result. Screenshot 2024-08-29 at 10 13 04

osrl avatar Aug 29 '24 07:08 osrl

Any update on this one? I found out that few of my ViewModels aren't cleared as well.

dch09 avatar May 22 '25 08:05 dch09

@dch09 unfortunately not. I haven't been able to reproduce this. If you can provide a reproducable sample I might be able to find the cause of the issue.

rickclephas avatar May 22 '25 17:05 rickclephas