Skip to content
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

Provider #60

Closed
wants to merge 5 commits into from
Closed

Provider #60

wants to merge 5 commits into from

Conversation

ohitsdaniel
Copy link
Contributor

Resolves #58.

Problem

Currently, ComposableNavigator is very TCA focused and assumes that people "hand-down" their view models into the NavigationTree. Flutter solves this by wrapping Widgets in ProviderWidgets that take care of initialising the Widget's dependencies and handing it down the WidgetTree through the context.

As NavigationTrees and View Hierarchies are regenerated whenever the path changes, NavigationTree/Views cannot retain ViewModels / ObservableObjects themselves as they would be reinitialised on every redraw.

Solution

Provider takes care of initialising and retaining screen dependencies. Dependencies are stored in a screen scope in a global DependencyStore. When a path builder builds its content, the Provider view is built and initialises the dependency once. Whenever a screen is removed from the navigation path, the dependency is removed from the store.

Draft

This PR is currently marked as draft as I want to add an example of provider usage to the Readme and Example app.

@github-actions
Copy link

Current coverage for ComposableDeeplinking.framework is 100.00%

No files affecting coverage found


Current coverage for ComposableNavigator.framework is 96.93%

Files changed - -
Provider.swift 96.88%
NavigationNode.swift 98.96%
DependencyStore.swift 99.03%

Current coverage for ComposableNavigatorTCA.framework is 98.46%

No files affecting coverage found


Powered by xcov

Generated by 🚫 Danger

@ohitsdaniel
Copy link
Contributor Author

ohitsdaniel commented Mar 11, 2021

I tried this out and unfortunately, the observing provider does not lead to updates in the content as the reference does not update. One way to solve this is to wrap all values in a value-type view model inside the observed object that is a computed property on the reference-type vm

...
          Provider(
            observing: { navigator, currentScreenID in
              DetailViewModel(
                train: screen.train,
                navigator: navigator,
                currentScreenID: currentScreenID
              )
            },
            content: { viewModel in
              DetailView(viewModel: viewModel.vm)
            }
          )
...

class DetailViewModel: ObservableObject {
  let train: Train
  let navigator: Navigator
  let currentScreenID: ScreenID

  @Published var counter: Int = 0

  init(train: Train, navigator: Navigator, currentScreenID: ScreenID) {
    self.train = train
    self.navigator = navigator
    self.currentScreenID = currentScreenID
  }

  func increaseCounter() {
    counter += 1
  }

  func decreaseCounter() {
    counter -= 1
  }

  func showCapacity() {
    navigator.go(
      to: CapacityScreen(capacity: train.capacity),
      on: currentScreenID
    )
  }

 struct ValueTypeVM {
    let counter: Int
    let train: Train
    let increaseCounter: () -> ()
    let decreaseCounter: () -> ()
    let showCapacity: () -> ()
  }

  var vm: ValueTypeVM {
    ValueTypeVM(
      counter: counter,
      train: train,
      increaseCounter: self.increaseCounter,
      decreaseCounter: self.decreaseCounter,
      showCapacity: self.showCapacity
    )
  }
}

Whenever the observed reference-type VM updates, it updates its content, passing a new value-type VM into the view, causing that one to update. I'll put some more thought into this and I think it can be solved in an easier way.

@omichde
Copy link
Contributor

omichde commented Mar 18, 2021

Although the origin may be TCA driven, I think the value/data/vm driven approach is a foundational and common concept in SwiftUI and the composability of this navigator as well. I'm not sure wether opening it up for different approaches is distracting from the strengths of the current implementation.
Just my 2c

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add Providers
2 participants