ReactiveX / rxjs

A reactive programming library for JavaScript
https://rxjs.dev
Apache License 2.0
30.82k stars 3.01k forks source link

Can the internal _value of BehaviourSubject be nulled out on complete? #5405

Open Wilt opened 4 years ago

Wilt commented 4 years ago

Feature Request

Should BehaviourSubject::_value be nulled out on complete? Or is there a reason that it remains even after completion? This might empty out some used memory and would allow the unused value to be garbage collected?

Maybe something in the line of:

  complete(): void {
    this._value = null;
    super.complete();
  }

At first I thought it is not nulled out because of the public getValue() method, but then it was clear that even this method will not return the value after completion, rendering the value useless even in this case.

Maybe I missed some other case then I would gladly hear of the reason to understand why value should remain even after calling complete.

cartant commented 4 years ago

No. It's not completion that closes a subject; it's calling unsubscribe on the subject itself:

https://github.com/ReactiveX/rxjs/blob/def740988071552e8ff18f4c11387aca97be6843/src/internal/Subject.ts#L92-L96

It would make some sense to null out the value when that's done, but, TBH, calling unsubscribe on subjects is pretty unusual: https://ncjamieson.com/closed-subjects/

Wilt commented 4 years ago

Thank you for your response, a link to a great blog post and enlightening me.
I am no expert on RxJs (yet :) ) and I should have looked better in the parent class before posting. I clearly misunderstood the internals of the Subject class.

It also makes me realize that completing subjects for cleaning up is not enough, I should actually even call unsubscribe on those and then I will get "load and angry errors" in case something still calls next after it was closed. I am learning something new every day...

But it means that this could still be considered, right?

  unsubscribe(): void {
    this._value = null;
    super.unsubscribe();
  }
cartant commented 4 years ago

But it means that this could still be considered, right?

Yeah. It should probably null it if an error occurs, to - as getValue will throw in that scenario, too.

It also makes me realize that completing subjects for cleaning up is not enough ...

I would not recommend calling unsubscribe on subjects. It really has a different meaning in that context; it's really a dispose`.

Rather, I would question why having to call it ought to be necessary in the first place. Did something return a subject to someone else? And said thing wants to guard against next, being called? I'd question whether or not that is the preferred way of doing things.

Also, I'd question why it's necessary for the nulling to occur. IMO, it would be indicative of references being held when they ought not to be. Relying upon the nulling of the subject's value to address that would only fix part of the problem.