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

Two way data binding #10

Closed
gsipic opened this issue Mar 20, 2024 · 10 comments
Closed

Two way data binding #10

gsipic opened this issue Mar 20, 2024 · 10 comments
Assignees

Comments

@gsipic
Copy link

gsipic commented Mar 20, 2024

Hello,

How can you use Binding(two way data communication) with this library from view model.

@SwiftedMind
Copy link
Owner

Hey @gsipic

that depends. A binding to the underlying ProcessState or LoadableState can be created just like you would with other properties in the view model:

import Processed
import SwiftUI

final class ViewModel: ObservableObject, LoadableSupport {
  @Published var loadable: LoadableState<Int> = .absent
}

struct OuterView: View {
  @StateObject private var viewModel = ViewModel()

  var body: some View {
    InnerView(loadable: $viewModel.loadable)
  }
}

struct InnerView: View {
  @Binding var loadable: LoadableState<Int>

  var body: some View {
    Text("")
  }
}

However, if you need/want to pass down the run/load methods, that's not really possible, by the nature of how this package works in classes. The view model owns these methods, so you'd have to pass down the view model itself (as an @ObservedObject for example).

When used in classes, these loadables and processes can't be complex objects themselves because value observation in SwiftUI would not work anymore (among other problems, like storing and handling the internal Task). That's why it is implemented through these LoadableSupport/ProcessSupport protocols.

With iOS 17 and the new @Observable mechanism, I can probably improve upon this quite a bit in a new version of this package.


In retrospect, I should have probably been more explicit about this current limitation in the README. Sorry about that!

@SwiftedMind SwiftedMind self-assigned this Mar 20, 2024
@gsipic
Copy link
Author

gsipic commented Mar 21, 2024

Ok, thank you for response. Can you make data property in loadableState struct to have setter and getter. Because in current situation you need to pass all Loadable state in InnerView but a want only pass data part of loadableState.

Current data don't have setter

public var data: Value? {
    if case .loaded(let data) = self { return data }
    return nil
  }

Don't want

struct InnerView: View {
  @Binding var loadable: LoadableState<Int>

  var body: some View {
    Text("")
  }
}

a want this

struct InnerView: View {
  @Binding var loadable: Int

  var body: some View {
    Text("")
  }
}

This is how a make workaround a have added setter and getter on data. And with that inner view can only get part of wanted data. I don't need to pass all Loadable to InnerView.

extension LoadableState {
    var dataBinding: Value {
        get {
            if case .loaded(let data) = self { return data }
            fatalError()
        }
        set {
            setValue(newValue)
        }
    }
}

@SwiftedMind
Copy link
Owner

The problem is that there is no way to do this correctly, because there is not a possible, single definition for correct in such a case.

@Binding var loadable: Int

Using a non-optional type here makes the assumption that the loadable state can only ever be .loaded(value), which is not true in the general case. Also, if there always is a value, you don't need a loadable in the first place.

Secondly, what would it mean to set the data property to nil? A state of .absent, .loading or .error? All of these would make sense because for any of them, the getter of data would actually return nil.


That said, you might be facing a specific use case where your InnerView is guaranteed to be only shown when the loadable does have a value and you want it to just be able to manipulate what that value is, while always keeping the .loaded state of the loadable.

This is a use case that would have to rely on proper documentation (because its behavior is not exactly intuitive). There's still no single, correct way of doing this.

Would the following work for you?

extension Loadable.Binding {
  var loadedBinding: Binding<Value>? {
    guard let data = wrappedValue.data else {
      return nil
    }

    return Binding<Value> {
      data
    } set: { newValue in
      guard case .loaded = wrappedValue else {
        return // no-op? Not clear what should happen in this case.
      }
      wrappedValue = .loaded(newValue)
    }
  }
}

struct OuterView: View {
  @Loadable<Int> private var loadable

  var body: some View {
    if let binding = $loadable.loadedBinding {
      InnerView(loadable: binding)
    }
  }
}

struct InnerView: View {
  @Binding var loadable: Int

  var body: some View {
    Text(loadable.formatted())
  }
}

To be honest, I'm not entirely sure this would be a good addition to the package. I think it's just not clear enough in its behavior for people to easily understand and use this. But I need to think about this a bit more, or maybe you have some ideas or arguments on how to make it clearer 🙂

In any case, for now you can just use your existing workaround or copy my extension from above into your project and you will have support for this.

@gsipic
Copy link
Author

gsipic commented Mar 21, 2024

Ok, thank you for support about this issue. This is edge case in my opinion. I need also to do some thinking 🙂.

P.S The library is very good thank you for open sourcing it.

@SwiftedMind
Copy link
Owner

Thank you 🙂

What might be doable is adding a second product to the package that users can import separately 🤔. That product could contain helper extensions for these "rarer" scenarios. This would keep the core functionality/package clean.

Would you be fine with that? Like, I could add a product called ProcessedUtility or so that would contain this.

@gsipic
Copy link
Author

gsipic commented Mar 22, 2024

Hey,

That is a good approach 🙂. I'm fine with that design. Making clean separation between core and extensions.

Thank you!

@SwiftedMind
Copy link
Owner

Awesome!
I'll prepare a new release this weekend then, hopefully 🙂

@SwiftedMind
Copy link
Owner

I might need a few more days, sorry 😅 Got lost in some other projects over the weekend

@SwiftedMind
Copy link
Owner

@gsipic It's released 🎉 🙂 2.2.0
Hope it works for you, I ended up keeping the name. I think it's quite concise

@gsipic
Copy link
Author

gsipic commented Apr 3, 2024

@SwiftedMind Yes that better naming convention.

@gsipic gsipic closed this as completed Apr 3, 2024
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

No branches or pull requests

2 participants