Avoid unsafe extraction in BindingReducer
#3315
Open
+95
−67
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
We currently have a single case of `AnyCasePath(unsafe:)` in the code base, which uses reflection for extraction, and is something we would like to remove in a future major version of Case Paths. This explores a refactoring of `BindingReducer` that takes an explicit case key path toe the binding action case, instead: ```swift BindingReducer(action: \.binding) ``` `BindableAction` as a protocol is still necessary for bindable stores to know how to embed binding actions in a short syntax, _e.g._ `$store.text`. It's a slight bummer that the reducer gets slightly more verbose, especially when there _is_ something abstractable here, I'm just not sure Swift provides the tools yet to truly abstract over a case path in an ergonomic fashion. The alternative would be to introduce a special protocol (`BindableCasePath`?) that the `@Reducer` macro would need to apply correctly when detecting a `BindableAction`, but this is one-off logic that seems to take things in the wrong direction.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
We currently have a single case of
AnyCasePath(unsafe:)in the code base, which uses reflection for extraction, and is something we would like to remove in a future major version of Case Paths.This explores a refactoring of
BindingReducerthat takes an explicit case key path to the binding action case, instead:BindableActionas a protocol is still necessary for bindable stores to know how to embed binding actions in a short syntax, e.g.$store.text.It's a slight bummer that the reducer gets slightly more verbose, especially when there is something abstractable here, I'm just not sure Swift provides the tools yet to truly abstract over a case path in an ergonomic fashion.
The alternative would be to introduce a special protocol (
BindableCasePath?) that the@Reducermacro would need to apply correctly when detecting aBindableAction, but this is one-off logic that seems to take things in the wrong direction.This is a draft for now that does not clean up all the existing occurrences, as I'm not sure we want to complete and merge it.