Aspect's Around

[yuyuUI]

Hi, I would like to bring up a discussion about Around. Previously, we had _PrintChangesReducer, which might be the same as Aspect's after.
I was new to this concept of aspect oriented programming and found the need for Around was much less than I needed. One case can be tracking the time a reducer takes. Such as mxSignpost(_:dso:log:name:signpostID:_:_:). However I can not tell if this is a good way. 🙏

struct Around<Base: ReducerProtocol>: ReducerProtocol {
  typealias State = Base.State
  typealias Action = Base.Action
  var base: Base
  enum Advice {
    case before(Action)
    case after(Action, time: CFAbsoluteTime)
  }

  var aop: (Advice) -> Void

  @Dependency(\.absoluteTime) var absoluteTime

  func reduce(into state: inout Base.State, action: Base.Action) -> EffectTask<Base.Action> {
    var before: CFAbsoluteTime!
    let effects = base.reduce(into: &state, action: action)
    return .merge(
      .fireAndForget {
        before = absoluteTime
        aop(.before(action))
      },
      effects,
      .fireAndForget {
        let after = absoluteTime
        aop(.after(action, time: after - before))
        before = nil
      }
    )
  }
}

[tgrapperon]

Hey @yuyuUI!
Without dwelling into the feature's idea itself, you probably need to protect your before variable, as you're touching it from two async contexts. You can use LockIsolated from Dependencies for example. Otherwise, the compiler will show warnings if you enable sendability checks, even if it's de facto OK given the implementation. There is no real need to set the before to nil at the end too.
You may also want to concatenate instead of merge, as effects are here sent "simultaneously", and you have no guarantee over individual effect's ordering in a real-life app.
That being said, I'm not familiar with the topic, and I'm only commenting the TCA bits.

Please note that the library also ships with a .signpost() reducer "modifier" that posts actions and effects events to Instruments.

[yuyuUI]

Thank you! @tgrapperon
I missed .signpost(). It looks like the thing I was looking for. I think it could be a good example to study.
You are right. I notice the .merge is wrong because the let effects = base.reduce(...) is the context. And I should run the aspect before it. Therefore, it should also solve other concerns.