SignalKit icon indicating copy to clipboard operation
SignalKit copied to clipboard

Swift 3.0 compatibility

Open shergin opened this issue 8 years ago • 8 comments

How about Swift 3.0 compatible version or branch?

shergin avatar Aug 23 '16 18:08 shergin

For reference, this fork is an attempt to migrate the project to Swift 3

evgeniyd avatar Nov 23 '16 11:11 evgeniyd

I will work on it in the coming weeks.

I would like to remove some unnecessary parts like the CollectionEvent , UITableView and UICollectionView bindings.

Also the versioning came out of control 😅 if I introduce the API breaking changes by following the semantic versioning the next version should be 5.0. What do you think should I just bump the version to 4.1?

If you have any suggestions, please share them here.

Much appreciated!

yankodimitrov avatar Feb 08 '17 20:02 yankodimitrov

Yay!!1 I believe there is noting bad in bumping major version!

shergin avatar Feb 08 '17 23:02 shergin

Im working on it on the SignalKit-5.0 branch.

I refactored the Signal and SignalValue to be simple non-thread safe classes by default. Usually those are intended to create a binding between ViewModel and the View on the main thread. If the ViewModel is doing some async work on a background thread it will have the option to send the result to the Signal on the main thread.

Of course we can create a thread safe Signal and SignalValue by passing a Lock protocol implementation (there is a default public MutexLock implementation) or we can use the class factory method .atomic() and .atomic(value: T):

// non-thread safe
let age = Signal<Int>()
let name = Signal<String>(value: "John")

// thread safe
let age = Signal<Int>(lock: MutexLock())
let name = SignalValue<String>(value: "John", lock: MutexLock())

// thread safe
let age = Signal<Int>.atomic()
let name = SignalValue<String>.atomic(value: "John")

Hope to hear your feedback 😃

yankodimitrov avatar Mar 05 '17 21:03 yankodimitrov

Yay! 🎉 I always prefer having default implementation simple as possible, so I think this is the right move! I also slightly prefer to have Signal<Int>(atomic: true) instead of Signal<Int>.atomic()... I believe that it is more Swifty style. But, feel free to ignore my advices because... I don't know the difference between Signal and SignalValue. 😄

Awesome work, Yanko! Thank you!

shergin avatar Mar 06 '17 19:03 shergin

You are right its more cleaner as an initializer parameter, so I refactored it to:

let name = Signal<String>(atomic: true)

Probably because SignalValue is a bad name so I renamed it to SignalProperty. 😅 The Idea is that SignalProperty have an initial/current value and sends it to new observers right away, so you don't have to send an initial value as with the basic Signal:

// with SignalProperty
let name = SignalProperty<String>(value: "John", atomic: true)
name.bindTo(textIn: nameLabel).disposeWith(bag) // nameLabel.text is now "John"

// with Signal
let name = Signal<String>(atomic: true)
name.bindTo(textIn: nameLabel).disposeWith(bag)
name.send("John") // we have to send the initial value after we have signal observers

Also I added macOS target 🎉

Thanks!

yankodimitrov avatar Mar 07 '17 08:03 yankodimitrov

Wow! That is great that you designate this functionality (SignalProperty) as a first class citizen of SignalKit! Really, these are different and equally important concepts and we have to have them both!

Personally, I slightly prefer to name SignalProperty as something that extends Signal concept, so it should have a name pattern <Adjective>+"Signal" like... PersistentSignal. But, again, as an author, you have to know better. 😄

shergin avatar Mar 07 '17 19:03 shergin

I like the idea.

Here are some explorations for a signal that reacts to new observers by sending them its current value:

  • ReactiveSignal
  • StoredSignal

StoredSignal sounds good, but yes naming things is hard 😄

yankodimitrov avatar Mar 07 '17 21:03 yankodimitrov