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

DismissEffect does not work on iOS14 #2233

Closed
1 of 3 tasks
sk409 opened this issue Jun 27, 2023 · 6 comments · Fixed by #2283
Closed
1 of 3 tasks

DismissEffect does not work on iOS14 #2233

sk409 opened this issue Jun 27, 2023 · 6 comments · Fixed by #2283
Labels
bug Something isn't working due to a bug in the library.

Comments

@sk409
Copy link
Contributor

sk409 commented Jun 27, 2023

Description

DismissEffect does not work on iOS14.

Nothing happens when I run the code below on iOS14.

struct CoreReducer: ReducerProtocol {
    @Dependency(\.dismiss) private var dismiss

    func reduce(into state: inout State, action: Action) -> EffectTask<Action> {
        switch action {
        ...
            return .fireAndForget {
                // Nothing happens.
                await dismiss()
            }
        }
    }
}

Checklist

  • I have determined whether this bug is also reproducible in a vanilla SwiftUI project.
  • If possible, I've reproduced the issue using the main branch of this package.
  • This issue hasn't been addressed in an existing GitHub issue or discussion.

Expected behavior

Screen closes after executing DismissEffect.

Actual behavior

Screen does not close after executing DismissEffect.

Steps to reproduce

  1. Transition to any screen on iOS14
  2. Execute DismissEffect on the transition destination screen

The Composable Architecture version information

0.54.1

Destination operating system

iOS 14.5

Xcode version information

14.3

Swift Compiler version information

swift-driver version: 1.75.2 Apple Swift version 5.8 (swiftlang-5.8.0.124.2 clang-1403.0.22.11.100)
@sk409 sk409 added the bug Something isn't working due to a bug in the library. label Jun 27, 2023
@stephencelis
Copy link
Member

@sk409 Can you attach a sample project reproducing the problem? The issue doesn't have enough to go by.

Note that the dismiss effect only works with features embedded with ReducerProtocol.ifLet, held in PresentationState/PresentationAction, and presented via an appropriate view helper, like View.sheet(store:).

@sk409
Copy link
Contributor Author

sk409 commented Jun 27, 2023

@stephencelis

This is the sample project to reproduce.

https://github.com/sk409/DismissEffectSample

The code using the DismissEffect is below.

https://github.com/sk409/DismissEffectSample/blob/main/Test/ContentView.swift

@sk409
Copy link
Contributor Author

sk409 commented Jun 27, 2023

@stephencelis

Note that the dismiss effect only works with features embedded with ReducerProtocol.ifLet, held in PresentationState/PresentationAction, and presented via an appropriate view helper, like View.sheet(store:).

I'm using DismissEffect in the correct order, so it's working on iOS15 and iOS16.

@stephencelis
Copy link
Member

stephencelis commented Jun 27, 2023

@sk409 Yup! I took a look at your code but need to download the iOS 14 runtime sometime later today to investigate. It may just have to do with the fact that programmatic navigation in SwiftUI is quite buggy. It's better in iOS 16.4 and 17, but previous iOSes do not perform well when it comes to navigation.

stephencelis added a commit that referenced this issue Jul 13, 2023
Fixes #2233.

On iOS 14, `DismissEffect` is broken because effect cancellation uses
`AnyHashable` throughout, which can lead to nested
`AnyHashable(AnyHashable(...))` IDs, which can hash differently
depending on the level of nesting. By preserving the hashable ID up to
being stored in the internal cancellation token type, we can avoid a
hash lookup failure that prevents a dismissal from happening.
@stephencelis
Copy link
Member

Thanks again for the repro! Finally had a chance to sit down with this today and have a fix here: #2283. Please confirm that it fixes the problem for you!

stephencelis added a commit that referenced this issue Jul 13, 2023
Fixes #2233.

On iOS 14, `DismissEffect` is broken because effect cancellation uses
`AnyHashable` throughout, which can lead to nested
`AnyHashable(AnyHashable(...))` IDs, which can hash differently
depending on the level of nesting. By preserving the hashable ID up to
being stored in the internal cancellation token type, we can avoid a
hash lookup failure that prevents a dismissal from happening.
@sk409
Copy link
Contributor Author

sk409 commented Jul 13, 2023

@stephencelis
I was able to confirm that DismissEffect is working on iOS14.
Thank you for your response.

stephencelis added a commit that referenced this issue Jul 14, 2023
* Fix cancellation ID existentials in iOS 14

Fixes #2233.

On iOS 14, `DismissEffect` is broken because effect cancellation uses
`AnyHashable` throughout, which can lead to nested
`AnyHashable(AnyHashable(...))` IDs, which can hash differently
depending on the level of nesting. By preserving the hashable ID up to
being stored in the internal cancellation token type, we can avoid a
hash lookup failure that prevents a dismissal from happening.

* wip
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working due to a bug in the library.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants