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

[Consideration] Bridge from OpenCombine to Combine publishers #217

Open
matteogazzato opened this issue Aug 13, 2021 · 12 comments
Open

[Consideration] Bridge from OpenCombine to Combine publishers #217

matteogazzato opened this issue Aug 13, 2021 · 12 comments

Comments

@matteogazzato
Copy link

Hi guys,

I know there was already somebody talking about a "interoperability" between OpenCombine and Combine (here the issue) and possibly would be out of scope but here there is my situation and I would understand if the solution I'm thinking can be implementable:
I currently have an SDK written using OpenCombine, the project, which is including the SDK, will support iOS 13+, so I would use Apple Combine on that side.
The main and only nice to have would be a sort of OpenCombine.Publisher extension useful to convert the Publisher to Combine .Publisher. In this scenario, it would be pretty easy to quickly integrate the SDK inside the App and work with Combine.

  • Is this wrong?
  • Would it possible to achieve this kind of solution? If so, should I write down a custom Combine Publisher and Subscriber to correctly convert from OpenCombine.Publisher? (Any suggestion is welcome)

I know something was done for RxSwift with RxCombine.

Thanks in advance for any help.

@maximkrouk
Copy link

maximkrouk commented Aug 13, 2021

I implemented a prototype this evening, maybe I'll make a draft PR tomorrow after cleaning up the codebase (or maybe @broadwaylamb may create a separate repo for the bridge if needed)

There is a chance, that this feature is not in-scope for the framework/community, but anyway it was fun to implement 🌚

@matteogazzato
Copy link
Author

Thanks @maximkrouk for your support, I tried to achieve my goal creating a draft custom Publisher and Subscription.
Here there my implementation

extension OpenCombine.Publisher {
    var combinePublisher: Combine.AnyPublisher<Output, Swift.Error> {
        BridegedPublisher(withOpenCombinePublisher: self)
            .eraseToAnyPublisher()
    }
}

public class BridegedPublisher<OpenCombinePublisher: OpenCombine.Publisher>: Combine.Publisher {
    public typealias Output = OpenCombinePublisher.Output
    public typealias Failure = Swift.Error
    
    private let ocPublisher: OpenCombinePublisher
    
    init(withOpenCombinePublisher publisher: OpenCombinePublisher) {
        self.ocPublisher = publisher
    }
    
    public func receive<S: Combine.Subscriber>(subscriber: S) where Failure == S.Failure, Output == S.Input {
        subscriber.receive(
            subscription: BridgedSubscription(
                withOpenCombinePublisher: ocPublisher,
                combineSubscriber: subscriber
            )
        )
        
    }
}

public class BridgedSubscription<OpenCombinePublisher: OpenCombine.Publisher,
                                 CombineSubscriber: Combine.Subscriber>: Combine.Subscription
where CombineSubscriber.Input == OpenCombinePublisher.Output, CombineSubscriber.Failure == Swift.Error {
    
    private let ocPublisher: OpenCombinePublisher
    private let subject = Combine.PassthroughSubject<OpenCombinePublisher.Output, Swift.Error>()
    private var subscriber: CombineSubscriber?
    
    init(
        withOpenCombinePublisher publisher: OpenCombinePublisher,
        combineSubscriber: CombineSubscriber) {
        self.ocPublisher = publisher
        self.subscriber = combineSubscriber
        subject.receive(subscriber: self.subscriber!)
        
        _ = self.ocPublisher
            .sink { error in
            switch error {
            case .failure(let error):
                self.subject.send(completion: .failure(error))
            case .finished:
                self.subject.send(completion: .finished)
            }
        } receiveValue: { event in
            self.subject.send(event)
        }
    }
    
    public func request(_ demand: Combine.Subscribers.Demand) {
        // Not necessary?
    }
    
    public func cancel() {
        self.subscriber = nil
        
    }
}

Of course, this is only linked to publisher.
Your implementation would be a complete bridge between OpenCombine - Combine and that's great!
I'm following the combine-bridge merge request.

Thanks again 🤙🏻

@maximkrouk
Copy link

My implementation is more low-level and relies on Combine private API, so you can't be sure that it won't break, so depending on what does @broadwaylamb thinks about it it might be rewritten to something higher-level, like your suggestion, so thank u for sharing 🙂

@maximkrouk
Copy link

maximkrouk commented Aug 17, 2021

My implementation is more low-level and relies on Combine private API, so you can't be sure that it won't break, so depending on what does @broadwaylamb think about it, it might be rewritten to something higher-level, like your suggestion, so thank u for sharing 🙂

@designerfuzzi
Copy link

designerfuzzi commented Jun 17, 2022

same idea here, so +1.
the problem i am facing is 'ObservableObject' is ambiguous for type lookup in this context in Xcode 11.3.1 which is the last Version that supports iOS 12 and below. In that particular setup ObservableObjectexists in Beta Framework "Combine" but can't be used and linking with OpenCombine produces the above ambiguous relation. Apple's Combine is not linked in my project and i just dont know a simple way to kick the relation in favour of OpenCombine yet. Maybe someone has some hint or link where i can read on about it. Apart from that "Publisher" would be a nice feature and indeed make development easier with support to lower OS <= OSX10.14

@designerfuzzi
Copy link

designerfuzzi commented Jun 17, 2022

uuh sometime one more cofé helps..
i had to change

import Combine
class SomeCombineUsingClass : ObservableObject  {
    //...
}
//to
import OpenCombine
class SomeCombineUsingClass : OpenCombine.ObservableObject {
   //...
}

now taking on the missing @Publisher feature

@maximkrouk
Copy link

maximkrouk commented Jun 17, 2022

Yep, the issue is not in the bridge, but in Combine&OpenCombine, you have to use Module prefix if both modules are imported, but afaik you won't have to use it if you import OpenCombineBridge and one of the Combine frameworks 🤔

OpenCombine is more like a replacement for the Combine, so in general it's not meant to be used with the Combine and names are kept the same 🙂

Sadly some features are still not available 😔

@designerfuzzi
Copy link

designerfuzzi commented Jun 17, 2022

that leaves me with a simple question.. how to take on code parts that make use of @Publisher feature..
"just" 8 properties that needs to be rewritten .. which i can wrap in precompiler directives to avoid clashing for builds later on. Sorry if that is a little distracting from matteogazzato's issue

Maybe a blunt thought into the dust .. when @Publisher feature is not available in Swift then my thought is could @propertyWrapper help me out declaring same behaviour?

@broadwaylamb
Copy link
Member

I think you can just do this:

typealias Published = OpenOmbine.Published

This way you don't have to rewrite all your properties.

@designerfuzzi
Copy link

designerfuzzi commented Jun 17, 2022

nice one! thank you. but i can not admit done yet.
I linked all 4 OpenCombine libs, doesnt hurt yet, also import OpenCombine & import OpenCombineDispatch
them in code. Outcommented my own propertywrapper, and used your suggestion ^^but! instead i had to write
typealias Published = OpenCombine.Published and voila ! seems to make it work. One step further.

Now thinking.. same has to be done for Scheduler and who knows what will come up after.. but step by step.

someObserveable.$highlighted
    .receive(on: DispatchQueue.main) //<---- Argument type 'DispatchQueue' does not conform to expected type 'Scheduler'
    .sink { [weak self] newValue in
        self?.overlayView.needsDisplay = true
    }
    .store(in: &cancellables)

So i tried additionally
typealias Scheduler = OpenCombine.Scheduler
which as i was bluntly hoping can't fix it the same nice easy way.

@broadwaylamb
Copy link
Member

Instead of DispatchQueue.main use DispatchQueue.main.ocombine.

@designerfuzzi
Copy link

compile success. Resume..

import OpenCombine
import OpenCombineDispatch
typealias Published = OpenCombine.Published


someObserveable.$highlighted
    .receive(on: DispatchQueue.main.ocombine) 
    .sink { [weak self] newValue in
        self?.overlayView.needsDisplay = true
    }
    .store(in: &cancellables)

made it work. Thank you very much

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

4 participants