Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ViewModel is not cleared on iOS 17.4 #80

Open
osrl opened this issue Aug 23, 2024 · 11 comments
Open

ViewModel is not cleared on iOS 17.4 #80

osrl opened this issue Aug 23, 2024 · 11 comments

Comments

@osrl
Copy link

osrl commented Aug 23, 2024

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 osrl changed the title ViewModel is not cleared on iOS 17.5 ViewModel is not cleared on iOS 17.4 Aug 23, 2024
@rickclephas
Copy link
Owner

Could you possibly provide a minimal reproducible sample?

@osrl
Copy link
Author

osrl commented Aug 23, 2024

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 added a commit to osrl/KMP-ObservableViewModel that referenced this issue Aug 24, 2024
@osrl
Copy link
Author

osrl commented Aug 24, 2024

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 added a commit to osrl/KMP-ObservableViewModel that referenced this issue Aug 24, 2024
@osrl
Copy link
Author

osrl commented Aug 24, 2024

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.

@rickclephas
Copy link
Owner

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
Copy link
Owner

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.

@osrl
Copy link
Author

osrl commented Aug 26, 2024

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?

@rickclephas
Copy link
Owner

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.

@osrl
Copy link
Author

osrl commented Aug 27, 2024

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

@rickclephas
Copy link
Owner

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

@osrl
Copy link
Author

osrl commented Aug 29, 2024

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants