RFC: Deprecate Effect.fireAndForget and Effect.task in favor of unified Effect.run interface

[stephencelis]

The Composable Architecture currently leads folks to three main interfaces for performing effects:

• Effect.run
• Effect.task
• Effect.fireAndForget

After a recent discussion and some thought, we'd like to propose deprecating task and fireAndForget and instead rely on a single run interface, and we would like to outline our reasoning.

How we got here

First, some background: when we worked on better support for Swift concurrency, we focussed on distancing the library primitives from Combine, and introducing new primitives for working with structured concurrency. These new primitives were actually largely based off existing primitives that had proven themselves useful, pre-concurrency beta:

• Effect.run feeds zero or more actions back into the store. We had a Combine-friendly version a lot time ago, but finessed the interface when building support for async-await.

• Effect.fireAndForget feeds no actions back into the store. It just fires some effectful work off into the void and the store can forget all about it. Prior to the concurrency beta, we had a version that took a simple, synchronous () -> Void closure, but we beefed this up to take an () async throws -> Void closure with the beta.

• Effect.task can feed a single action back into the store. It was introduced to the library before our more holistic Concurrency beta, and was our first experiment with allowing effects to perform async-await work, but we only recommended its use in "live" dependencies because the test store was not equipped for handling this kind of asynchronous work at the time. With the concurrency beta, we beefed it up to behave more like the new and improved run and fireAndForget helpers.

Where we'd like to go

We think it's time to reconsider things. We don't think separate interfaces for fireAndForget and task are carrying their weight these days, and community members liked @davdroman are coming to the same conclusion. So we'd like to propose deprecating these interfaces, first softly, then loudly on a 1.0 release branch, before fully removing them when 1.0 is out.

• Effect.fireAndForget would be more simply handled by an Effect.run that ignores the send argument, and thus cannot feed actions back into the store:

  -return .fireAndForget { ... }
  +return .run { _ in ... }

  It's fewer characters that just as loudly signal intent.

  "Fire-and-forget" is also jargon-y language that can cause confusion, not only across language barriers. @alexito4 points out that this separate interface calls its very semantics into question. It can evoke Task.detached but does not share any of the same behavior. We think collapsing things to .run { _ in … } avoids all of these questions and mess.

• Effect.task is sugar, but we wonder if this sugar is worth it. Instead we propose that one can simply use run here, as well:

  -return .task { /*...*/; return .action }
  +return .run { send in /*...*/; await send(.action) }

  While .task signals up front that the effect will feed a single action back into the store...we're not sure this is meaningful. It's also not really true. If the effect is cancelled or throws cancellation, it will not feed that single action back into the store.

  Beyond that, run can animate effects, but task cannot. To animate a task you must either upgrade it to a run, or you must reach for the .animation() effect modifier. The former shows how fussy a refactoring can be for a change that is simple. The latter is just more surface area bogging the library down.

  One argument for .task is it’s a clean interface for feeding values back into the store:

  return .task { .action }

  We think such “synchronous” actions are better handled by a non-async interface. While we have Effect(value: .action), it was designed to be paired with Effect.init(error:) and is part of the Combine APIs we have deprecated. Instead, we propose introducing Effect.send as a more precise helper:

  return .send(.action)

---

If we had designed effects from scratch with structured concurrency in mind, I'm not sure we would have even come up with these three interfaces. Taking the time to remove two of them should make the library easier to learn and less fussy to work with over time.

So this is the direction we’d like to take things. We understand that it’s more churn on the road to 1.0, but we think it will make the library better and more approachable in the long run. How does that sound to everyone?

[jshier]

Would this be an opportune moment to introduce other direction action conveniences? .send is useful, but I occasionally have actions which call multiple other actions. Sometimes these should be ordered, which is easy enough in run but might makes sense as a composition of the new .send, but sometimes they should occur in parallel (or at least concurrently). For instance, when reloading the content of a screen, multiple network requests might be necessary, so I'd like to just kick off their individual actions and allow them to each call back into the reducer. Do you think there's any need to provide such abstractions (multiple sync, async actions)?

[tgrapperon]

The deprecation of Effect(value:) made .task { .action } more difficult to reach, which was probably a positive side-effect of the change. I'm slightly concerned that .send will make it even more accessible than before, leading to this sub-optimal goto pattern. Maybe other names would be more appropriate, like .delegate or .post?

[mbrandonw]

We can also add some docs to Effect.send to warn against the pattern and explain what situations we think it is good for.

[tgrapperon]

As I'm thinking about it, it would also force the library to take a stance and formalize the delegate pattern, which is probably not required.
It also seems that a Reducer graph would allow to spot misuses at runtime in DEBUG, as we would be able to check if a given Reducer that emits a .send is producing in reaction something else than .none without touching its State.

[tgrapperon]

Such a process should be done with a synchronous action and not incur a thread hop.

@mbrandonw Is this achievable with the current non-deprecated tools? AFAIK, only Effect(value:) behaves like this, as this gist from the Concurrency beta shows.
Is .send(.action) intended to be implemented in a way that doesn't hop, and not as some sugar to .run { await $0(.action) } like I initially thought. If so, I have no reservation about the name.

[mbrandonw]

Is .send(.action) intended to be implemented in a way that doesn't hop,

Yes exactly. If we provide a dedicated .send, then we would make the guarantee that it is definitely performed synchronously. It would probably mean a new case in the internal Effect.Operation enum.

[rcarver]

Throwing out another naming idea to... "return an effect"

return .effect { send in await send(.action) }
return .effect { _ in ... }
return .send(.action) // this still looks right to "send an action" 

[davdroman]

What about renaming all to .task { send in ... }?

I think there's an appealing argument for this too. A couple of reasons:

1. "Task" is a state-of-the-art term with an established meaning. "Run" is a rather generic term that may or may not imply asynchrony to the reader.
2. return .task reads a lot better and is more declarative than return .run as a double imperative.

[mbrandonw]

The main reason we shied away from .task is that it will of course be compared to Task, but Task is a tool for unstructured concurrency, yet .task is very much structured. It participates in cooperative cancellation and its lifetime is tracked by the store (if you do await viewStore.send(...).finish()).

To us that makes a little dangerous to conflate the two ideas.

[larryonoff]

What about migrating to direct Effect naming (like suggested above), i.e.

return Effect  { send in ... }
...
return .effect  { send in ... }

// the same naming is for reducers

var body: some ReducerProtocol<State, Action> {
  CombineReducers {
    Reduce {
    }
   
   Scope(...)
  }
}

with both options available.

I agree that Effect.effect feels redundant, but it looks like ButtonStyles implementations in SwiftUI.

[davdroman]

Actually joked with @mbrandonw today that we may have just gone with Effect { send in … }

What about migrating to direct Effect naming (like suggested above), i.e.

I'm starting to see this as more and more suitable. Effect { send in } would be my preferred API, but even if we go with .effect { send in } for static's sake I think it's worth the redundancy in exchange for the clarity at the point of use.

[jshier]

To me it seems like the static naming should facilitate the distinction between different types of effects, even if they're all Effect underneath.

• .run seems okay, if a bit succinct, in that it can run arbitrary actions.
• .action also seems okay, as it's a direct send of an action.
• .effect is both redundant and non-descriptive.
• .task is also rather non-descriptive, and its shadowing of Task is misleading.

In the end, it seems like .run can serve both the current .run and .task roles, leaving .action and .run as the separate between sync and async effects. That's likely enough to start.

[junebash]

I'm also in favor of Effect { send in /*...*/ }. Looking at the current state of the library and the roadmap, it looks like this is intended to be the primary entry point for side effects, and making this a full initializer rather than a static factory method makes that clearer.

[Jeehut]

I suggested to unify Effect APIs in this thread, not knowing about this RFC. While I suggested something different, I'm also very happy with what was suggested here. 👍

But why stop there? I think there's also a chance of streamlining Effect.merge and Effect.concatenate. See my comment here.

[kakhaberikiknadze]

So, I'm getting non-sendable warning for this one since action.send is @MainActor (Action) -> Void.

Non-sendable type '@mainactor (Reducer.Action) -> Void' in implicitly asynchronous access to main actor-isolated property 'send' cannot cross actor boundary

struct Reducer: ReducerProtocol {
    struct State: Equatable {}
    
    enum Action {
        case foo
        case bar(TaskResult<String>)
    }
    
    var body: some ReducerProtocol<State, Action> {
        Reduce { state, action in
            switch action {
            case .foo:
                return .run { action in
                    await action.send(
                        await .bar(
                            TaskResult {
                                try await Task.sleep(for: .seconds(2))
                                return "result"
                            }
                        )
                    )
                }
            case let .bar(result):
                print(result)
                return .none
            }
        }
    }
}

It would work if I mark run closure with @MainActor like so

// ...
            switch action {
            case .foo:
                return .run { @MainActor action in
                    action.send(
                        await .bar(
                            TaskResult {
                                try await Task.sleep(for: .seconds(2))
                                return "result"
                            }
                        )
                    )
                }

Is the 2nd approach a correct practice? (Asking this question as I couldn't find a note about main actor in the description.

[mbrandonw]

This seems like a Swift bug to me. Somehow there is a difference between action() and action.send(), even though both are @MainActor.

This simple repro also shows the problem:

@MainActor
struct S {
  var call: @MainActor () -> Void
  func callAsFunction() { self.call() }
}
func run(_ operation: @Sendable @escaping (S) async -> Void) {}
func foo() {
  run { send in
    await send()
    await send.call()  // ⚠️ Non-sendable type '@MainActor () -> Void' in implicitly asynchronous access to main actor-isolated property 'call' cannot cross actor boundary
  }
}

This should probably be filed as a Swift bug, but in the meantime is there a reason you aren't constructing your effect like so:

return .run { send in 
  await send(…)
}

The Send argument implements callAsFunction which allows you to simply invoke send like a function.

[mbrandonw]

Oh I see, the send property in Send is not @Sendable but it does seem like it should be. We will investigate.

[kakhaberikiknadze]

Thanks for the answer 🙌🏻 Yeah I could construct it as send(...). Tried that and it works indeed. I just didn't notice I could call it like so.

[VilemKurz]

The simplicity of .task will be missed.

return .task { .delegate(.reorderButtonToggled) }
return .run { await $0(.delegate(.reorderButtonToggled)) }

[mbrandonw]

@VilemKurz This can be done today in an even simpler syntax that will not be deprecated:

return .send(.delegate(.reorderButtonToggled))