pointfreeco/isowords

Settings.Action.binding actions are not being handled as expected

acosmicflamingo opened this issue · 3 comments

Describe the bug
It seems like many of the viewStore actions sent within views of the Settings module (e.g. SoundsSettingsView) do not actually do anything within the Settings reducer as I'd expect. Instead, I see in the debug console a message like this:

2022-12-09 17:31:27.590658-0600 isowords[11473:1931144] [ComposableArchitecture] A binding action sent from a view store 
at "SettingsFeature/SoundsSettingsView.swift:22" was not handled. …

  Action:
    Settings.Action.binding(.set(_, 0.6017382))

To fix this, invoke "BindingReducer()" from your feature reducer's "body".

This is very strange because BindingReducer() is indeed within the Settings reducer body.

To reproduce
If I compile isowords in Xcode, run the app, click on the settings button, select the sound button and try and change the value of any of the sound sliders, the sliders actually do not change value.

Expected behavior
I'd expect that the Settings reducer would actually stop at any of the breakpoints I set within it.

Screenshots/video
https://user-images.githubusercontent.com/67525430/206812842-45fda284-78ec-4908-8177-1ab66a1a5015.mov

Environment

  • Device: iPhone 14 Simulator
  • OS: iOS 16.1
  • Xcode: 14.1

Additional context
What is strange is that if I run "SettingsPreview", I do not see this problem.

Hey @acosmicflamingo! The other common error is forgetting to embed the child Reducer that hosts the BindingReducer into its parent. You can indeed check that the Home feature is not scoping any Setting reducer, which explains why it doesn't run at all.

If we add:

Scope(state: \.settings, action: /Action.settings) {
  Settings()
}

to Home.body, the message goes away and the sliders are updating.

The SettingPreview doesn't have any issue because it runs the Settings reducer by construction.

This is very likely an omission from the ReducerProtocol conversion, albeit I'm not familiar enough with the codebase to decide if this is the correct way to embed this feature here. I'll check how it looked like before, and I'll eventually PR.

Hello there @tgrapperon ! Nice to meet you :) Thanks for the helpful info AND even creating a PR :D definitely makes sense. Since you say it's common, perhaps it'd be beneficial for this to be documented somewhere. So I created this PR: pointfreeco/swift-composable-architecture#1729

This was fixed awhile back, and we finally converted our old style of shared settings state to use a proper dependency here: #186. Thanks for filing the issue though!