-
Notifications
You must be signed in to change notification settings - Fork 231
API doc comment updates for lifecycle hooks and router #151
Conversation
|
Looks good! @vikerman This collides with one of my internal CLs, so I can manually merge this change internally it if you want. |
|
FYI, angular/angular.io#2664 has been merged into ng.io master. |
| /// Note that a directive should not implement both [DoCheck] and [OnChanges] at | ||
| /// the same time. [ngOnChanges] would not be called when a directive | ||
| /// implements [DoCheck]. Reaction to the changes have to be handled from within | ||
| /// the [ngDoCheck] callback. |
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.
Did you verify this. I vaguely remember discussions in TS that this is not actually the case.
Seems I even started it myself :D angular/angular#6810
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.
Thanks for the cross link to 6810. No I did not double check, but the statements did seem a little strange to me.
I'll let @matanlurey @ferhatb or @thso reply as to the accuracy of the statements.
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.
@matanlurey ??
|
@chalin Could you rebase this on latest? We'll get it pulled in ASAP |
kevmoo
left a comment
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.
Please rebase against latest ASAP – so we can get this in before something else changes. 😄
|
Ok, I'll get on it in about 30 min. |
Contributes to angulardart#116. This is a followup to angulardart#124. This commit has updates for all remaining entries in the spreadsheet except for `CanActivate`, which I think should be handled once the router chapter is created.
3d80b27 to
90a1155
Compare
|
Rebased. |
kevmoo
left a comment
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.
Thanks! 🤘
Author: Patrice Chalin <[email protected]> Closes #151 ------------- Created by MOE: https://github.com/google/moe MOE_MIGRATED_REVID=139099701
Contributes to #116. This is a followup to #124. This commit has updates for all remaining entries in the spreadsheet except for
CanActivate, which I think should be handled once the router chapter is created.Note that this commit depends on angular/angular.io#2664