Use CXShim instead of hardcoded Combine implementation

[ddddxxx]

See Combine Compatible Package

Advantage

• Current clients remains unaffected.
• Apple's Combine is used by default whenever possible. No additional dependency is introduced.
• OpenCombine is still available.
• The package now support lower version macOS/iOS (use open-source Combine).
• Your clients can still use open-source Combine for macOS Catalina if they want.
• No matter which Combine implementation you client choose, the package adapt to it automatically.

[helje5]

I'm not convinced. This is bringing in a whole lot of dependencies, particularly in the common macOS-std-Combine case.
If switching Combine's is considered worthwhile, I'd rather add an internal "CXShim" like target to deal with it. (which btw has no proper fallbacks, so the user has to specify a Combine).

[ddddxxx]

In the common macOS-std-Combine case, no third party Combine is bundled in. And user is not forced to specify a Combine in any case. That's what I call "Current clients remains unaffected."

The most noticeable change is that the system requirement is now macOS 10.10 / iOS 8.0 (was macOS 10.15 / iOS 10.13).

[helje5]

You are changing the source to do a import CXShim, which surely does make SPM fetch all the dependencies? I.e. the full version of your Combine, Nimble and more.
I don't think that this is desirable. I'd rather have an own small (single file ..) "CXShim" target within the project instead, that's all we need for switching imps (plus an enhanced Package.swift to account for your own Combine).

Also I'm not quite sure how "CXShim" works, looking at the source, it seems like the explicitly does have to specify a Combine version, even for the macOS setup?:

#if USE_COMBINE // where does this come from?
... more checks ...
#else
#error("Must specify a Combine implementation.")
#endif

[helje5]

(don't get me wrong, I'm happy to add optional support for your Combine version as well if some environment variable is set, I'll have a look at that later)

[ddddxxx]

Yes, SPM will fetch CombineX and its dependency, that how SPM works. But they're not bundled into the final product if std-Combine is used.

And yes, you can easily write your own implementation for switching imps via env variable. But imagine every package does it on their own. It would be a chaos. That's why I designed CXShim: to solve it once for all. If one day someone write a new Combine implementation, only CXShim needs to change, You package will adapt to it automatically.

The USE_COMBINE flag is defined in the Package.swift. In fact the error message can never be reached.

[ddddxxx]

I believe a single shim package is desirable for all cross platform package depends on Combine.

This solution is tested by my own LyricsX project. It contains 2 submodule (independent package), LyricsKit and MusicPlayer, which all depends on CXShim. The macOS application support macOS 10.11 and use CombineX. The iOS application requires iOS 13.0 and use Apple's Combine. They share the same submodule.

[ddddxxx]

CXShim is particularly useful for deep nested package. Imagine adding a Combine utility package to SwiftWebUI (or Linux client of SwiftWebUI), like CombineExtensions. It's impossible because of linux build. But now with CXShim, you can easily add CXExtensions.

[helje5]

This really needs to be fixed in SPM (package alternatives, package selection, module aliases, resolution support). Not by hacks like this (no offence, it really is an SPM issue, other 25y old pkg managers can do all that forever).

But in no case should the shim library be delivered with a full Combine implementation, it should live in a separate, tiny package driven by the community. I can set one up and invite everyone, but how would we decide what the default fallback implementation is? :-)

[ddddxxx]

I totally agree! CXShim is the result of my battle with SPM. As you see, my LyricsX project have a strong need of such shim package.

The CXShim was a separate package, until CombineX's test suit start to depend on it 🤣. The test suit run against both Combine and CombineX, so CXShim fit perfectly.

[ddddxxx]

Compare to the problem it solved, I don't think full CombineX repo is a very big deal. The dependencies are cached so it won't increase the build time. And CombineX is driven by community anyway.

[helje5]

Compare to the problem it solved, I don't think full CombineX repo is a very big deal.

As mentioned I disagree.

CombineX is driven by community anyway

U seriouz? 😃

[helje5]

OK, I can see three options to finish up the PR:

• a) leave it as is, this is a toy project anywayz, who cares
• b) I (or someone else who is not a Combine framework author) sets up a new GH org, and you and the OpenCombine people get invited. Those org just hosts the shim package. OpenCombine stays the default on Linux (because it was first, only sensible selection I can see, we can do elections later :-).
• c) we add a small variant of your shim to SwiftWebUI, default stays OpenCombine, but the user can select your's if they want to

Something I won't do is fetch a whole Combine implementation just for the shim, sorry.

[ddddxxx]

CombineX is driven by community anyway

I'm positive. especially compare to OpenCombine which is of course maintained by individual.

I'm not the creator of CombineX, actually I didn't know its creator until I contribute to CombineX. I decide to contribute to CombineX not OpenCombine because it's driven by community (the community was one-man-army then, now it has 2 member. It will become to 3 if you'd like to join).

[ddddxxx]

I'd prefer (c) cause I don't want invent the same wheel twice :-)

[ddddxxx]

@helje5 Done. The package is still a Combine Compatible Package (except default to OpenCombine. so If anyone use this package with other Combine Compatible Package on Linux, they have to specify a Combine implementation).

What I care about most is Combine Compatible Package, not concrete Combine implementation. Because it fill the gap between different Combine and lead the community to the brighter future.

PS: I can't stop thinking what if Apple launched open-source Combine at the beginning. The whole mess would just never exist.

[helje5]

Thanks! Seems to be fine, going to try it tomorrow and then merge.

P.S.: This is not a Combine-as-OpenSource issue at all, it is purely an SPM issue.

[ddddxxx]

Even we have something like import OpenCombine as Combine, the problem is still there.

import Foundation
import Combine // OpenCombine is imported as Combine

[1, 2, 3].publisher
// 🛑 Error on macOS (10.15 and 10.14-). Combine and OpenCombine both extend `Sequence`
// The Combine ship with system is effectively unavailable on macOS 10.14, but their interface collide anyway. Even you didn't `import Combine(system Combine)`

#if USE_COMBINE // compilation conditions is still required
[1, 2, 3].publisher
#elseif USE_OPEN_COMBINE
[1, 2, 3].ocombine.publisher
#endif
// ✅ Okay

@Published var foo = 0
// 🛑 Error on macOS. Foundation re-export Published

#if USE_COMBINE // compilation conditions is still required
@Foundation.Published var foo = 0
#elseif USE_OPEN_COMBINE
@Combine(renamed OpenCombine) var foo = 0
#endif
// ✅ Okay

With CXShim you just need

import Foundation
import CXShim

[1, 2, 3].cx.publisher // uniformed namespace

@CXShim.Published var foo = 0 // uniformed module

This is the real world problem CXShim tries to solve. Luckily SwiftWebUI didn't encounter them. I assume that's why you're not interested in CXShim.

[ddddxxx]

I noticed you consider CombineX repo as a burden of CXShim. I'd like to improve. If I did revive the CXShim repo, would you accept the pr at the beginning?

And please do not consider me "Combine framework author". The honor belongs to @luoxiu and @broadwaylamb. I'm merely an enthusiastic contributor. I'm a member of cx-org because of my contribution (that's how community works, right?).

If there was luoxiu/CombineX and OpenCombine/OpenCombine, The CXShim could be OpenCombine/OpenCombineShim, or ddddxxx/CombineShim. but we got cx-org/CombineX and broadwaylamb/OpenCombine, so that's it.

[helje5]

That would be option b). We can do that.

[ddddxxx]

I see. I just stick with option c).

This pr is ready for review.

[ddddxxx]

@helje5 Anything I need to do to get this pr merged?

[helje5]

I don't think so, have to review the changes, try them out and eventually merge them. Explicitly designated as a toy project, priority is not sky high (means no ETA), srz.