Skip to content
This repository has been archived by the owner on Apr 20, 2018. It is now read-only.

Prevents BehaviorSubject from dispatching undefined to subscribers if initialized without a value. #27

Closed
wants to merge 8 commits into from

Conversation

trxcllnt
Copy link
Contributor

No description provided.

@mattpodwysocki
Copy link
Member

Not sure if I want to break compatibility with the current .NET implementation which does not allow for such a thing.

@trxcllnt
Copy link
Contributor Author

Does .NET require a BehaviorSubject is initialized with a value?

undefined is a valid event value. If someone wants a BehaviorSubject to dispatch undefined, they can pass it to the constructor.

If the BehaviorSubject isn't initialized with a value, it shouldn't report that it was initialized with undefined; it wasn't. ReplaySubject doesn't dispatch undefined as the first value upon subscription, so why does BehaviorSubject?

BehaviorSubjects are excellent wrappers for Model values, but filtering the initial undefined value is painful to remember, write, and explain, and is hurting adoption in my present project.

var userModel = new Rx.BehaviorSubject();

var userSubscription = userModel
    .where(_.identity) // even with this shorthand... why? the userModel doesn't have a value yet.
    .subscribe(function(val) {
        $('.username').text(val.name);
    });

// ... later
userModel.onNext(new User('Paul'));

@cwharris
Copy link

.NET's BehaviorSubject cannot be instantiated without a starting value. Currently, JavaScript's behavior matches the .NET implementation. However, since we cannot prevent new Rx.BehaviorSubject(), the implicit value of the BehaviorSubject is undefined.

Strictly speaking, a BehaviorSubject cannot exist without a value.

I believe that by allowing a behavior subject to be instantiated without a value we would be disregarding the functional purity of Rx in favor of a convenient shortcut. :/

@trxcllnt
Copy link
Contributor Author

It's not unreasonable that a BehaviorSubject instance can exist in a "pending" state, like a JSON request would. Other Observables/Subjects don't dispatch undefined to new subscribers, so why should an uninitialized BehaviorSubject?

If you really want purity, throw an error if BehaviorSubject is initialized without a value.

Purity shouldn't be enforced if it provides negative utility. Code is just a tool we have to make the future happen sooner.

@cwharris
Copy link

That being said, this situation matches a different pattern that involves a Subject and an Observable capable of yielding the latest value to new subscribers (this is different than a BehaviorSubject, in that a BehaviorSubject yields the current value instead of the latest value). I just looked it up, and once again, this already exists in the form of a replay subject! :)

var replay = new Rx.ReplaySubject(1);

replay.subject(console.log.bind(console));
replay.onNext('a'); // logs 'a'
replay.onNext('b'); // logs 'b'
replay.subject(console.log.bind(console)); // logs 'b' again

console.log(replay.q[0].value); // like replay.value if it were a behavior subject

I would like to suggest adding a function to a ReplaySubject to retrieve the latest value, as I believe this would cover the identified use case.

replay.latest();

Idea for implementation:

Rx.ReplaySubject.prototype.latest = function () {
  return this.q[0].value;
};

I'm not suggesting this is the best idea, but I don't believe it violates any of the principles current in place in Rx.

@trxcllnt
Copy link
Contributor Author

Look, I get that BehaviorSubject is supposed to wrap a value, but that's traditionally been enforced by the compiler. Short of throwing an error, you can't enforce that BehaviorSubject is initialized with a value in JS.

But throwing an error here isn't user-friendly, so why not duck type the thing and mimic the ReplaySubject if BehaviorSubject is initialized without a value?

undefined is a valid value that I can onNext to a BehaviorSubject. The semantics of instantiating a BehaviorSubject without arguments is different than instantiating a BehaviorSubject constructor with undefined as the argument.

In the first case, we're explicitly initializing without a value, including the undefined value. In the second case we're effectively onNext'ing undefined to the BehaviorSubject at initialization, so future subscribers should receive undefined upon subscription.

It doesn't break backwards compatibility unless people are initializing BehaviorSubjects with no arguments and doing something with the undefined first dispatch, and I doubt many people are doing that. If they are, make 'em change their code to new BehaviorSubject(undefined). They should be more explicit anyway.

Adding a "latest" function to the ReplaySubject seems more problematic than changing BehaviorSubject in this way.

First, why restrict it to getting the latest value? Why can't you get any cached value?
Second, if the ReplaySubject has a getter for values, wouldn't you expect to read out values from a replayed Observable to maintain feature parity? But replay is just multicast with a ReplaySubject, so you'd have to add it to the ConnectableObservable prototype at the very least.

To be clear, I'm not advocating this approach.

@mattpodwysocki
Copy link
Member

Well, if you feel that way, I'm open to creating different Subjects which wrap said behavior, but the one as is should stay as it is.

@trxcllnt
Copy link
Contributor Author

Another Subject type is fine with me. From what I can tell, the thing I want is close to a reactive Maybe, but I'm not really qualified to say. As for names, MaybeSubject, FutureSubject, or PromiseSubject?

@cwharris also proposed adding a "maybe" instance method on BehaviorSubject. It's not a bad idea if there's concern over the impact on lib size, since the new type is only slightly different.

var presentValue = new BehaviorSubject();
var futureValue = new BehaviorSubject().maybe();

bouzuya pushed a commit to bouzuya/RxJS that referenced this pull request Mar 23, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants