-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add overloads of TestStore.send that accept CaseKeyPaths #2681
Add overloads of TestStore.send that accept CaseKeyPaths #2681
Conversation
@scogeo Thanks for taking the time to PR this! It's actually something we talked about, but we felt that having two ways of doing the same thing was enough reason to avoid the extra API. In the case of store.receive(.response(.success(MyResponse(/* complicated payload */))))
// vs.
store.receive(\.response.success) // no payload Meanwhile, this is not the case with store.send(.textFieldChanged("Hello"))
// vs.
store.send(\.textFieldChanged) // 🛑 can't send Even if we allow payloads as a secondary argument, the // Need to manipulate the `Result`'s error type to be `Equatable` and not `any Error`:
store.receive(.response(.success(MyResponse(/* complicated payload */))))
// Can simply equate on the `MyResponse` type:
store.receive(\.response.success, MyResponse(/* complicated payload */)) The // 2 ways of doing the exact same thing? Is there a difference other than syntax?
store.send(.textFieldChanged("Hello"))
store.send(\.textFieldChanged, "Hello") Does our line of thought make sense? Is there something we're missing? If I really stretch my thinking, losing the nested parentheses is nice, but then would we extend this to syntax to sending actions to normal |
@stephencelis You make some good points about the differences in motivation between My motivation for submitting the PR came from refactoring a large And in some cases, it is not just nested parentheses, it is a completely differently style of writing the code. For example, this snippet also from await store.send(.voiceMemos(.element(id: deadbeefURL, action: .playButtonTapped))) {
$0.voiceMemos[id: deadbeefURL]?.mode = .playing(progress: 0)
}
await store.receive(\.voiceMemos[id:deadbeefURL].delegate.playbackStarted)
await self.clock.run() With the PR, this could then be rewritten as: await store.send(\.voiceMemos[id:deadbeefURL].playButtonTapped) {
$0.voiceMemos[id: deadbeefURL]?.mode = .playing(progress: 0)
}
await store.receive(\.voiceMemos[id:deadbeefURL].delegate.playbackStarted)
await self.clock.run() This looks much more symmetric and is easier for my mind to process. So while this is definitely syntactic sugar, it adds symmetry to With regards to In the end, this may just be a personal coding style preference. But I do find the asymmetry between |
@scogeo We're definitely not opposed to the idea. Let us just think on it a bit longer. |
@stephencelis Sounds good. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry for the hold up, but after some time to think it over we think that it makes sense to have these overloads. Thanks for taking the time to PR!
…ure to from: "1.9.0" (#940) This PR contains the following updates: | Package | Update | Change | |---|---|---| | [pointfreeco/swift-composable-architecture](https://togithub.com/pointfreeco/swift-composable-architecture) | minor | `from: "1.8.2"` -> `from: "1.9.0"` | --- ### Release Notes <details> <summary>pointfreeco/swift-composable-architecture (pointfreeco/swift-composable-architecture)</summary> ### [`v1.9.0`](https://togithub.com/pointfreeco/swift-composable-architecture/releases/tag/1.9.0) [Compare Source](https://togithub.com/pointfreeco/swift-composable-architecture/compare/1.8.2...1.9.0) #### What's Changed See [Migrating to 1.9](https://togithub.com/pointfreeco/swift-composable-architecture/blob/main/Sources/ComposableArchitecture/Documentation.docc/Articles/MigrationGuides/MigratingTo1.9.md) for more details. - Added: New versions of `TestStore.send` that accept case key paths (thanks [@​scogeo](https://togithub.com/scogeo), [https://github.com/pointfreeco/swift-composable-architecture/pull/2681](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2681); [https://github.com/pointfreeco/swift-composable-architecture/pull/2868](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2868)). - Added `Reducer.dependency(value)`, for overriding a reducer's dependency using a singleton value of a type ([https://github.com/pointfreeco/swift-composable-architecture/pull/2863](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2863)). - Fixed: Improve `Store` diagnostics for deriving bindings ([https://github.com/pointfreeco/swift-composable-architecture/pull/2793](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2793)). - Fixed: Avoid erroneous perception checks when `ViewStore`s are initialized in a view that doesn't use `WithPerceptionTracking` ([https://github.com/pointfreeco/swift-composable-architecture/pull/2849](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2849)). - Fixed: Support `#if` branching in `@ObservableState` and enum `@Reducer`s ([https://github.com/pointfreeco/swift-composable-architecture/pull/2800](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2800)). - Infrastructure: Tree navigation documentation fixes (thanks [@​imjn](https://togithub.com/imjn), [https://github.com/pointfreeco/swift-composable-architecture/pull/2837](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2837)); presentation reducer documentation fixes (thanks [@​ozumin](https://togithub.com/ozumin), [https://github.com/pointfreeco/swift-composable-architecture/pull/2853](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2853)). - Infrastructure: Improve tutorial diffing (thanks [@​oka-yuji](https://togithub.com/oka-yuji), [https://github.com/pointfreeco/swift-composable-architecture/pull/2844](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2844)). - Infrastructure: Expand release build test coverage ([https://github.com/pointfreeco/swift-composable-architecture/pull/2856](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2856)). - Infrastructure: Document gotcha with macros and previews ([https://github.com/pointfreeco/swift-composable-architecture/pull/2855](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2855)). #### New Contributors - [@​imjn](https://togithub.com/imjn) made their first contribution in [https://github.com/pointfreeco/swift-composable-architecture/pull/2837](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2837) - [@​oka-yuji](https://togithub.com/oka-yuji) made their first contribution in [https://github.com/pointfreeco/swift-composable-architecture/pull/2844](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2844) - [@​ozumin](https://togithub.com/ozumin) made their first contribution in [https://github.com/pointfreeco/swift-composable-architecture/pull/2853](https://togithub.com/pointfreeco/swift-composable-architecture/pull/2853) **Full Changelog**: pointfreeco/swift-composable-architecture@1.8.2...1.9.0 </details> --- ### Configuration 📅 **Schedule**: Branch creation - At any time (no schedule defined), Automerge - At any time (no schedule defined). 🚦 **Automerge**: Enabled. ♻ **Rebasing**: Whenever PR is behind base branch, or you tick the rebase/retry checkbox. 👻 **Immortal**: This PR will be recreated if closed unmerged. Get [config help](https://togithub.com/renovatebot/renovate/discussions) if that's undesired. --- - [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check this box --- This PR has been generated by [Renovate Bot](https://togithub.com/renovatebot/renovate). <!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNi4xMDAuMCIsInVwZGF0ZWRJblZlciI6IjM2LjEwMC4wIiwidGFyZ2V0QnJhbmNoIjoibWFpbiJ9--> Co-authored-by: Self-hosted Renovate Bot <361546+cgrindel-self-hosted-renovate[bot]@users.noreply.github.enterprise.com>
Recently the ability to receive actions in a
TestStore
was updated to supportCasePaths
. This was a great improvement in ergonomics, but unfortunately the ability to send actions into the store usingCasePaths
is missing. This leads to a bit of a dichotomy when writing test cases when using deeply nested enums in actions.For example, this snippet from a test case in the
VoiceMemoTests
reads a little weird with a mixture ofCasePath
syntax and standard enum construction syntax.This PR allows this now to be written as:
This significantly improves the ergonomics of writing test cases, and also improves the readability of tests cases.
While not shown in this example, the PR also supports the ability to send associated values of enums, and is included in the test case for this feature.