core(initialized): add mixin for initialized #9609
Conversation
googlebot
added
the
cla: yes
| * an emit immediately. | ||
| * @docs-private | ||
| */ | ||
| export interface OnInitialized { |
| this._isInitialized = true; | ||
|
|
||
| this._pendingSubscribers.forEach(this._notifySubscriber); | ||
| this._pendingSubscribers = []; |
There was a problem hiding this comment.
this._pendingSubscribers = null;?
There was a problem hiding this comment.
Hmm, I think setting it to null makes it clear that it won't be used again. Curious your thoughts on adding the forced-not-null (!) in parts of the code where it should definitely be there. Is it clear as a dev that the array should be expected to be not-null in those cases?
There was a problem hiding this comment.
FYI my reason for setting it to null was so that it didn't take up the extra memory for the empty array
| * of ngOnInit. | ||
| * @docs-private | ||
| */ | ||
| _markInitialized(): void { |
There was a problem hiding this comment.
Should this guard against being called twice?
There was a problem hiding this comment.
The logic in the method is safe if called twice, but to make it more rigid I've added an error that makes it clear when it is called twice. I suspect it'll be raised in cases where a subclass tries to call it even though a base class does so. Might prevent some erroneous calls.
|
|
||
| // Function that completes the test when the number of notifications meets the expectation. | ||
| function onNotified() { | ||
| if (++currentNotificationCount === expectedNotificationCount) { |
There was a problem hiding this comment.
Not a fan of combining these unary operators with comparisons
| @@ -54,6 +54,7 @@ export const rollupGlobals = { | |||
| ...rollupMatEntryPoints, | |||
|
|
|||
| 'rxjs/BehaviorSubject': 'Rx', | |||
| 'rxjs/ReplaySubject': 'Rx', | |||
andrewseguin
force-pushed
the
core-on-initialized
branch
from
bee71da to
742019c
Compare
|
Comments addressed, please review |
jelbourn
added
pr: lgtm
action: merge
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
Adds a mixin that directives and components can use to add an
initializedstream for clients to know when they have entered the ngOnInit stage of their lifecycle. Useful to watch when you have a handle of a component but want to wait until its inputs are set before performing an action (e.g. watching a MatSort's initialization before sorting, since its inputs are not available until it reachesngOnInit)Note: Opt'ed out of using a
ReplaySubjectsince it requires a value is passed for itsemit, which we want to avoid in case later we want to emit something relevant