-
Notifications
You must be signed in to change notification settings - Fork 521
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
Invert subscription transforms in a reactive way (attempt No. 2) #420
base: master
Are you sure you want to change the base?
Conversation
hasher.combine(self.objectIdentifier) | ||
} | ||
#else | ||
public var hashValue: Int { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Legacy Hashing Violation: Prefer using the hash(into:)
function instead of overriding hashValue
(legacy_hashing)
self.observer.on(state) | ||
} | ||
|
||
// TODO: Use NSLock/atomic values for thread safety |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Todo Violation: TODOs should be resolved (Use NSLock/atomic values for t...). (todo)
_ observer: Observer, | ||
cancel: Cancelable | ||
) -> (sink: Disposable, subscription: Disposable) | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening Brace Spacing Violation: Opening braces should be preceded by a single space and on the same line as the declaration. (opening_brace)
internal class Producer<Substate>: Observable<Substate> { | ||
internal override func subscribe<Observer: ObserverType>(_ observer: Observer) | ||
-> Disposable where Observer.Substate == Substate | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening Brace Spacing Violation: Opening braces should be preceded by a single space and on the same line as the declaration. (opening_brace)
cancel: Cancelable) | ||
-> (sink: Disposable, subscription: Disposable) | ||
where Observer.Substate == Substate | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening Brace Spacing Violation: Opening braces should be preceded by a single space and on the same line as the declaration. (opening_brace)
cancel: Cancelable | ||
) -> (sink: Disposable, subscription: Disposable) | ||
where Observer.Substate == Substate | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening Brace Spacing Violation: Opening braces should be preceded by a single space and on the same line as the declaration. (opening_brace)
internal func composeSelect<SelectedSubstate>( | ||
_ transform: @escaping (Substate) -> SelectedSubstate | ||
) -> Observable<SelectedSubstate> | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening Brace Spacing Violation: Opening braces should be preceded by a single space and on the same line as the declaration. (opening_brace)
cancel: Cancelable | ||
) -> (sink: Disposable, subscription: Disposable) | ||
where Observer.Substate == Substate | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening Brace Spacing Violation: Opening braces should be preceded by a single space and on the same line as the declaration. (opening_brace)
public func subscribe<Subscriber: StoreSubscriber>(_ subscriber: Subscriber) | ||
-> SubscriptionToken | ||
where Subscriber.StoreSubscriberStateType == Substate | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening Brace Spacing Violation: Opening braces should be preceded by a single space and on the same line as the declaration. (opening_brace)
public func subscribe<Subscriber: StoreSubscriber>(_ subscriber: Subscriber) | ||
-> SubscriptionToken | ||
where Subscriber.StoreSubscriberStateType == Substate | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening Brace Spacing Violation: Opening braces should be preceded by a single space and on the same line as the declaration. (opening_brace)
middleware: [], | ||
automaticallySkipsRepeats: true) | ||
|
||
var receivedValue: TestStringAppState? = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Redundant Optional Initialization Violation: Initializing an optional variable with nil is redundant. (redundant_optional_initialization)
|
||
init<Subscriber: StoreSubscriber>(subscriber: Subscriber) | ||
where Subscriber.StoreSubscriberStateType == Substate | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening Brace Spacing Violation: Opening braces should be preceded by a single space and on the same line as the declaration. (opening_brace)
subscriber: Subscriber | ||
) -> SubscriptionToken | ||
where Subscriber.StoreSubscriberStateType == State | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening Brace Spacing Violation: Opening braces should be preceded by a single space and on the same line as the declaration. (opening_brace)
subscriber: Subscriber | ||
) -> SubscriptionToken | ||
where Subscriber.StoreSubscriberStateType == Substate | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening Brace Spacing Violation: Opening braces should be preceded by a single space and on the same line as the declaration. (opening_brace)
subscriber: Subscriber | ||
) -> SubscriptionToken | ||
where Subscriber.StoreSubscriberStateType == Substate | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening Brace Spacing Violation: Opening braces should be preceded by a single space and on the same line as the declaration. (opening_brace)
subscriber: Subscriber | ||
) -> SubscriptionToken | ||
where Subscriber.StoreSubscriberStateType == Substate | ||
{ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Opening Brace Spacing Violation: Opening braces should be preceded by a single space and on the same line as the declaration. (opening_brace)
@mjarvis @danielmartinprieto Could you have a look at the approach some time? |
Backstory: @mjarvis started this in #307 about 2 years ago. The way the ReSwift.Store is set up, his initial approach created a subscription right away. I suggested and tried to help with an approach where apps would have a
PendingSubscription
that only at thesubscribe()
call would be added to the Store.But this didn't work out well in practice and both Malcolm and I have practically given up for the time being :)
With my recent adventures into the realm of making Middleware calls simpler, I experimented with another way to dispatch actions, and found it would fit ReSwift quite well.
This is a sample call:
This code heavily borrows concepts and implementation from RxSwift. I had quite some trouble wrapping my had around the concepts to be able to implement them properly.
A couple of notes regarding the reactive nature, aka how the inversion is done:
Observable<State>
BlockSubscriber
: it forwardsnewState
calls to a block which is set from the outside as a callback to pass events down the Observable stream usingObservable.create
Observable.create
sounds like you'd create the event stream itself, similar to creating arrays; but it ain't so. Instead, you define the connection/subscription from Observable to its Observer counterpart.Observable.create
.StoreSubscriberType
in an app is not subscribed to the store anymore, just theBlockSubscriber
is; the app's subscribers receive updates via the observable/observer chain.BlockSubscriber
is prepared inObservable.create
to be subscribed to the store itself -- but the subscription is only executed when a connection happens. This is fun, and also weird, because I think about the creation first, like a "hot" sequence that just begins pumping out events, just but it is actually executed very late and, in some sense, lazily.By using
Observable
and its observers, you can model transformations with operators without actually having a functional store subscription.I left extensive comments in the code to make sense of the subleties of memory management. I still want to get rid of most parts of it. The dual of an
Observable
operator and itsSink
, which is just an observer that applies the transformations and conditionally forwards the result to the next in the chain, will probably stick. But theProducer
type, wrapping each step's objects in a cancelable objects with strongly references to its parts, maybe we can ditch that kind of stuff.Important types
IncompleteSubscription
I went with @mjarvis's API, starting with
Store.subscription() -> IncompleteSubscription<Store.State, Store.State>
. TheIncompleteSubscription<RootStoreState, Substate>
tracks the root state type of the store so you cannot create anIncompleteSubscription
in one store and stuff it into another, unless both use the same state type..select
operations modify theSubstate
associated type accordingly whileRootStoreState
stays the same.IncompleteSubscription
is the user-facing API. Observables and such are internal implementation details. Each operation replaces its underlyingObservable<Substate>
.SubscriptionToken
Problem with hiding the actual subscription to the store in a hidden
BlockSubscriber
is that users cannot unsubscribe the real subscriber. They only know the object they passed in. (Let's call the one users own the "user-facing subscriber".)I created
SubscriptionToken
to introduce a different state update mechanism in addition to notifiying all subscribers. You can unsubscribe each token individually, like regular subscribers. You can also still callStore.unsubscribe
with the user-facing subscriber and all corresponding tokens will be removed. The tokens keep a strong reference to the observable connection, represented by theDisposable
type; it receives thedispose()
command when the token is deallocated. This breaks the connection and eventually frees the underlyingBlockSubscriber
as well.Things I know are still missing:
Filter
, if we need that operator at all, that isSubscriptionBox
types etc. that introduced the "old" transformations, and revert this to the most basic ReSwift v2Subscription
s. (We still need these for theBlockSubscriber
to receive events, unless we remove the whole object-based subscription thing completely from the public API)skipRepeats
: ifautomaticallySkipRepeats
is enabled, figure out if the latest operator is askipRepeats
as well (a simple boolean forIncompleteSubscription
could do, or an type association to theSkipRepeats
operator type itself)I'd love to have your feedback on this! PRs to the branch of this PR are also very welcome if you want to play around with it! :)