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

CasePathable.modify: Change XCTFail to runtimeWarn as per documentation #155

Conversation

djangovanderheijden
Copy link
Contributor

As per the documentation:

If the enum’s case does not match the given case key path, the mutation will not be applied, and a runtime warning will be logged.

In reality, calling modify with a non-matching case will always call XCTFail. This PR adds ComposableArchitecture's RuntimeWarnings.swift file and uses runtimeWarn instead of XCTFail.

Alternatively, we could consider changing the documentation to clarify that the use of a non-matching case causes a runtime crash. I'm open to either solution, but a warning log seems a bit more graceful.

@mbrandonw
Copy link
Member

Hi @djangovanderheijden, our XCTFail from xctest-dynamic-overlay does a runtime warning under the hood when not in tests.

So this does currently behave as described in the docs. Are you seeing otherwise? Also, this definitely should not crash the app. Are you seeing that?

@djangovanderheijden
Copy link
Contributor Author

Hey @mbrandonw, interesting. I did see a crash in my application (with the debugger attached, but not while testing). I'll take another look and get back to you

@djangovanderheijden
Copy link
Contributor Author

Alright, the issue seems unrelated to CasePaths (sorry about that, I should've read the error on my colleague's screen better) but I am definitely seeing crashes:
Screenshot 2024-04-05 at 20 08 24

This happens in a clean, newly created project that's ran for the first time. Here's a small repro:

@CasePathable
enum Foo {
    case bar(String)
    case baz
}

struct ContentView: View {
    var body: some View {
        Text("Foo")
            .onAppear {
                var foo = Foo.baz
                foo.modify(\.bar) {
                    $0 = "bar"
                }
            }
    }
}

@mbrandonw
Copy link
Member

Ah I see, this will be fixed with pointfreeco/swift-issue-reporting#81. Thanks for the heads up!

@djangovanderheijden
Copy link
Contributor Author

Nice! I'll close this.

@djangovanderheijden djangovanderheijden deleted the modify-runtimewarn branch April 9, 2024 06:34
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

Successfully merging this pull request may close these issues.

2 participants