Skip to content
This repository was archived by the owner on Nov 16, 2021. It is now read-only.

Various tweaks#28

Open
isoden wants to merge 4 commits intoarkon:masterfrom
isoden:tweak
Open

Various tweaks#28
isoden wants to merge 4 commits intoarkon:masterfrom
isoden:tweak

Conversation

@isoden
Copy link
Copy Markdown
Contributor

@isoden isoden commented Jan 5, 2018

I changed it to be simpler.
please review :)

Thanks!

private _isPlatformBrowser: boolean = isPlatformBrowser(this.platformId);

private _beforeInit: Subject<void> = new Subject<void>();
private _onDestroy: Subject<void> = new Subject<void>();
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the purpose of _beforeInit and _onDestroy, exactly?

return merge(...sources)
.pipe(
takeUntil(this._beforeInit),
takeUntil(this._onDestroy),
Copy link
Copy Markdown
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm pretty unfamiliar with some rxjs practices. Could you explain what's going on here?

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this link will answer both your questions:
http://brianflove.com/2016/12/11/anguar-2-unsubscribe-observables/
takeUntil() is a practice to make sure a subscribe gets unsubscribed for sure when some action occurs.
the action can be defined with Subject, and then when you call Subject.next() Subject.complete() (isoden added this in lines 63-64 of this pull request) you make the action happens, which in turn makes the unsubscribe happen for the subscribe that was registered to this Subject (with takeUntil).

Copy link
Copy Markdown

@ndcunningham ndcunningham Apr 15, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't you just add a takeUntil(merge(this._beforeInit, this._onDestroy))
instead of double takeUntils ?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

4 participants