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

Introduce Generalized Event #722

Closed

Conversation

inamiy
Copy link
Contributor

@inamiy inamiy commented Apr 1, 2019

This is an experimental PR that allows to add any event type for GSignal<GEvent: GEventProtocol>, which means Signal<Value, Error> will now be its specialization: GSignal<Event<Value, Error>>:

/// Signal that emits specialized `Event<Value, Error>`.
public typealias Signal<Value, Error: Swift.Error> = GSignal<Event<Value, Error>>

The main idea and overview can be found from my RxSwift gist.

Generalizing events allows to create other interesting observables e.g. Single, Maybe, Completable that are found in RxSwift and other Rx frameworks.
Compared to their approach of wrapping existing Observable<Value, Error> to a new type, this PR's approach is more abstract and robust without relying on concrete Event<Value, Error> type.
For example:

  • We can create "non-terminating errors" without adding extra error type
  • We can create "completable event with generic associated value" (its specialization to Void will become Event<Value, Error>)
  • ... and many more fun stuffs

Lastly, it is worth mentioning that core engine of Signal doesn't need to rely on concrete Event<Value, Error> but GEventProtocol.

Commits

b9295b6, 3f23750, ad48733

Organized code since Signal and Event can be clearly separated.
(IMO this also improves readability of existing codebase which can go to separate PR)

ba42df4

Added new "Generalized Event" concept (GEventProtocol) and refactored Signal.
(Other types are currently commented-out and not supported yet)


Before moving onto refactoring other types, I would like to hear what are your thoughts on this!

@mdiep
Copy link
Contributor

mdiep commented Apr 1, 2019

This applies final tagless encoding to Signal.

You listed some things that we can do. I totally agree that this opens up possibilities. The question is what would we actually use this for? i.e. of the things we can do, which would we want to do?

@inamiy
Copy link
Contributor Author

inamiy commented Apr 2, 2019

@mdiep

Single is one of the useful types to describe the API request which will either succeed or fail with receiving at most 1 response.
It is almost the same as Promise, so discussions e.g. #664 will be embraced into this PR, and Single can provide only a mimimal amount of public operators to the users which will be easier to understand.

There are also some minor advantages from type-system without us even noticing.
For example, GSignal<Never> won't allow to use many existing operators e.g. take(first:).

Cons are definitely the type-casting problem that we always need conversion e.g. RxSwift's single.asObservable() (Single to Observable conversion).
But as number of types grow, that is inevitable (especially in current Swift).

IMO knowing what kind of GEvent will be transferred is just as same as knowing what kind Error may be included in the stream.

@inamiy
Copy link
Contributor Author

inamiy commented Apr 2, 2019

The importance of Single also arises in ReactiveFeedback and ReactiveAutomaton where feedback side-effect should only trigger 1 new event, but SignalProducer<Event, NoError> doesn't provide enough type information to prohibit multiple emission.

@mdiep
Copy link
Contributor

mdiep commented Apr 17, 2019

Single would really need 3 events:

enum SingleEvent {
  case completed(Value)
  case failed(Error)
  case interrupted
}

I don't think that would enable much reuse of the existing code.

I could see using this internally to implement singles since you'd get some reuse of the internals. But from the perspective of a public API, this doesn't seem ideal. (Although it is interesting.)

@iv-mexx
Copy link
Contributor

iv-mexx commented May 20, 2019

One thing I come across repeatedly is the need to also have a (different type) value for the completed event.
Usually, this arises for long running operation that produce 1 Result (= Single), but where during the operation, progress updates should also be sent.

Say for a download, it is easy to use a custom type as Value

enum Status {
  case progress(Double)
  case done(Data)
}

But with that, there is no guarantee that a signal with that value behaves as one might expect, namely to receive a number of progress events (with the current progress), followed by exactly one completed event (with the result) or an error.

Say there was an Event that has associated values for Value and Completed

enum ResultEvent {
  case value(Value)
  case completed(Completed)
  case failed(Error)
}

With that, you could get the guarantees from the signal semantics.

Obviously this relates to other events:

  • Single is the case, where Value == Never
  • The currently existing Event is the case where Completed == Never

@andersio
Copy link
Member

andersio commented May 22, 2020

@iv-mexx That is an interesting idea that weaves well into the concept of Single. Might worth exploring.

Linking this with #667 #201 so I can have a reference in the future.

Speaking of this PR, generalizing event would be a non-goal IMO, since higher-order or specialized constructs may run their more expanded events in the baselinee event grammar, and transforming them only at the API edges. RAS at its core is streams of values after all. I am also looking to the direction of splitting the event observer sink into two (value + terminiation), so that ReactiveSwift can take advantage of large value passing optimizations, which are currently impossible due to enum packing at every level of transformation.

Closing due to inactivity.

@andersio andersio closed this May 22, 2020
@andersio andersio mentioned this pull request May 26, 2020
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.

4 participants