pointfreeco / swift-composable-architecture

A library for building applications in a consistent and understandable way, with composition, testing, and ergonomics in mind.
https://www.pointfree.co/collections/composable-architecture
MIT License
12.61k stars 1.46k forks source link

`UIViewController.present` doesn't set state to `nil` when dismissed #3451

Open myihsan opened 1 month ago

myihsan commented 1 month ago

Description

if the child state is Identifiable, the dismiss action is not be sent when dismissed. I think #3309 introduced this issue.

Checklist

Expected behavior

Send dismiss action when dismissed whatever the child state is Identifiable.

Actual behavior

https://github.com/pointfreeco/swift-composable-architecture/blob/fc5cbeec88114ff987f6c3cad3a7f3a3713fdb56/Sources/ComposableArchitecture/Observation/Store%2BObservation.swift#L395-L398 The id that determines whether or not to send the dismiss action is set by the following. https://github.com/pointfreeco/swift-composable-architecture/blob/fc5cbeec88114ff987f6c3cad3a7f3a3713fdb56/Sources/ComposableArchitecture/Observation/Store%2BObservation.swift#L338-L348 However, when scoping for present, if the child state is nil which is the most of the time, the id will also be nil, but _identifiableID(childState) will not be. So the dismiss action will not be sent.

This issue doesn't affect SwiftUI implementations because the scoped Binding is created every time when body is called, including the time the child state becomes non-nil.

I can work around this by mimicking SwiftUI's behavior, but it's so weird.

observe { [weak self] in
  guard let self, store.alert != nil else { return }
  var token: ObserveToken!
  token = present(
    item: $store.scope(state: \.alert, action: \.alert),
    onDismiss: { token.cancel() }
  ) { store in
    return UIAlertController(store: store)
  }
}

Reproducing project

TicTacToe https://github.com/pointfreeco/swift-composable-architecture/blob/fc5cbeec88114ff987f6c3cad3a7f3a3713fdb56/Examples/TicTacToe/tic-tac-toe/Sources/LoginUIKit/LoginViewController.swift#L94-L96 The alert is an AlertState which is Identifiable.

The Composable Architecture version information

1.14.0

jshier commented 1 month ago

This is the issue I reported in a discussion where the presentation tools didn't work in UIKit when scoping stores. Nice to have a true root cause and a reduction.

stephencelis commented 1 month ago

Thanks for the repro! We'll look into this soon.