wayfair / vsm-ios

An iOS framework for the VSM Architecture
MIT License
9 stars 2 forks source link

Proposal: RenderedViewState Manual State Subscription #29

Closed albertbori closed 1 year ago

albertbori commented 1 year ago

Description

Some potential problems were identified post-release in the @RenderedViewState property wrapper. This PR attempts to address those problems with a minor tweak to the property wrapper's API.

Problem

@RenderedViewState’s automatic rendering starts when the state property is first accessed by the UIViewController (or UIView). In some cases, there is no immediate need to interact with the state during a view’s early life-cycle. In that scenario, the automatic rendering will never start.

In another case, it's possible to have duplicate state progression:

If you called render() in viewDidLoad as a way of kicking off the state observation, it would execute render(), which would access the state property, which would subscribe statePublisher to render(), which would immediately execute render(). So render would get called twice immediately.

In addition: If you have the following within your render() function, your loaderModel.load() will be called and observed twice instantly, potentially causing duplicate data operations on each usage.

func render {
  switch state {
  case .initialized(let loaderModel):
    $state.observe(loaderModel.load())
  ...
}
...
struct LoaderModel {
  func load() -> AnyPublisher<MyViewState, Never> {
    Just(.someState)
      .subscribe(on: DispatchQueue.global()) // <- first state returned NOT on main thread
      .eraseToAnyPublisher()
  }
}

The current workaround for this issue is to avoid using your render() function for progression state. Only use it for configuring views. (As seen in the VSM examples and documentation.)

These issues, combined with the inherent risk that Apple “could break” the magic subscript for accessing the wrapper’s containing object (thereby breaking the auto-rendering), inspires us to consider adding an explicit render subscription option for the @RenderedViewState property wrapper for these edge cases and for more cautious engineers/orgs.

Solution

To address the problems above, this PR introduces a new function on the projected value of the @RenderedViewState property wrapper which enables engineers to (optionally) more directly control when the state subscription/auto-rendering begins. It can be used by simply calling $state.startRendering(on: self) in any view or view controller lifecycle event.

For example:

init(...) {
  _state = .init(wrappedValue: .initialized(LoaderModel()), render: Self.render)
  super.init(...)
}

override viewDidLoad() {
  super.viewDidLoad()
  $state.startRendering(on: self)
}

This is a non-breaking change which introduces additional capabilities. No migration is necessary.

The main drawback is that this new function may be misunderstood in its purpose and use. Clear documentation may be required to help encourage engineers to use this function properly.

Considered Alternatives

The Source architecture does something similar for UIKit views. However their approach differs. Instead of relying on fully automatic subscription to the state changes for rendering, the solution relies on an implicitly enforced pattern of always kicking off the automatic rendering with a manual call to render() in your init, viewDidLoad or view[Will|Did]Appear. To prevent the double-rendering problem, the internal workings of Source always drops the first value when the state subscription starts.

If we were to adopt this approach, our code would always look like this, regardless of the situation:

init(...) {
  _state = .init(wrappedValue: .initialized(LoaderModel()), render: Self.render)
  super.init(...)
}

override viewDidLoad() {
  super.viewDidLoad()
  render()
}

The reason that this solution was not chosen was because the double-render problem is a circumstantial edge case. The "80% case" is assumed to be that the render will automatically subscribe to state changes when the state is accessed/progressed in the init, viewDidLoad or view[Will|Did]Appear, like so:

init(...) {
  _state = .init(wrappedValue: .initialized(LoaderModel()), render: Self.render)
  super.init(...)
}

override viewDidLoad() {
  super.viewDidLoad()
  if case .initialized(let loaderModel) = state {
    $state.observe(loaderModel.load())
  }
}

Not to mention, the proposed solution allows for both of these paradigms to coexist, providing a "no-thought 80% solution" and a simple workaround for the "20% solution".

Type of Change

Checklist