Skip to content

Commit eef7d8a

Browse files
gkalpakalxhub
gkalpak
authored and
alxhub
committed
fix(upgrade): call ngOnInit() after ngOnChanges() (on components with inputs)
Fixes #18913
1 parent 617b3d2 commit eef7d8a

File tree

2 files changed

+178
-16
lines changed

2 files changed

+178
-16
lines changed

‎packages/upgrade/src/common/downgrade_component_adapter.ts

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ export class DowngradeComponentAdapter {
9999
})(input.prop);
100100
attrs.$observe(input.attr, observeFn);
101101

102-
// Use `$watch()` (in addition to `$observe()`) in order to initialize the input in time
102+
// Use `$watch()` (in addition to `$observe()`) in order to initialize the input in time
103103
// for `ngOnChanges()`. This is necessary if we are already in a `$digest`, which means that
104104
// `ngOnChanges()` (which is called by a watcher) will run before the `$observe()` callback.
105105
let unwatch: Function|null = this.componentScope.$watch(() => {
@@ -138,8 +138,7 @@ export class DowngradeComponentAdapter {
138138
(<OnChanges>this.component).ngOnChanges(inputChanges !);
139139
}
140140

141-
// If opted out of propagating digests, invoke change detection
142-
// when inputs change
141+
// If opted out of propagating digests, invoke change detection when inputs change.
143142
if (!propagateDigest) {
144143
detectChanges();
145144
}
@@ -150,10 +149,16 @@ export class DowngradeComponentAdapter {
150149
this.componentScope.$watch(this.wrapCallback(detectChanges));
151150
}
152151

153-
// Attach the view so that it will be dirty-checked.
152+
// If necessary, attach the view so that it will be dirty-checked.
153+
// (Allow time for the initial input values to be set and `ngOnChanges()` to be called.)
154154
if (needsNgZone || !propagateDigest) {
155-
const appRef = this.parentInjector.get<ApplicationRef>(ApplicationRef);
156-
appRef.attachView(this.componentRef.hostView);
155+
let unwatch: Function|null = this.componentScope.$watch(() => {
156+
unwatch !();
157+
unwatch = null;
158+
159+
const appRef = this.parentInjector.get<ApplicationRef>(ApplicationRef);
160+
appRef.attachView(this.componentRef.hostView);
161+
});
157162
}
158163
}
159164

‎packages/upgrade/test/static/integration/downgrade_module_spec.ts

Lines changed: 167 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* found in the LICENSE file at https://angular.io/license
77
*/
88

9-
import {ApplicationRef, Component, Inject, Injector, Input, NgModule, NgZone, OnChanges, OnDestroy, StaticProvider, ViewRef, destroyPlatform} from '@angular/core';
9+
import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, ApplicationRef, Component, DoCheck, Inject, Injector, Input, NgModule, NgZone, OnChanges, OnDestroy, OnInit, StaticProvider, ViewRef, destroyPlatform} from '@angular/core';
1010
import {async, fakeAsync, tick} from '@angular/core/testing';
1111
import {BrowserModule} from '@angular/platform-browser';
1212
import {platformBrowserDynamic} from '@angular/platform-browser-dynamic';
@@ -16,7 +16,7 @@ import {$ROOT_SCOPE, INJECTOR_KEY, LAZY_MODULE_REF} from '@angular/upgrade/src/c
1616
import {LazyModuleRef} from '@angular/upgrade/src/common/util';
1717
import {downgradeComponent, downgradeModule} from '@angular/upgrade/static';
1818

19-
import {html} from '../test_helpers';
19+
import {html, multiTrim} from '../test_helpers';
2020

2121

2222
export function main() {
@@ -306,6 +306,162 @@ export function main() {
306306
});
307307
}));
308308

309+
it('should run the lifecycle hooks in the correct order', async(() => {
310+
const logs: string[] = [];
311+
let rootScope: angular.IRootScopeService;
312+
313+
@Component({
314+
selector: 'ng2',
315+
template: `
316+
{{ value }}
317+
<button (click)="value = 'qux'"></button>
318+
<ng-content></ng-content>
319+
`
320+
})
321+
class Ng2Component implements AfterContentChecked,
322+
AfterContentInit, AfterViewChecked, AfterViewInit, DoCheck, OnChanges, OnDestroy,
323+
OnInit {
324+
@Input() value = 'foo';
325+
326+
ngAfterContentChecked() { this.log('AfterContentChecked'); }
327+
ngAfterContentInit() { this.log('AfterContentInit'); }
328+
ngAfterViewChecked() { this.log('AfterViewChecked'); }
329+
ngAfterViewInit() { this.log('AfterViewInit'); }
330+
ngDoCheck() { this.log('DoCheck'); }
331+
ngOnChanges() { this.log('OnChanges'); }
332+
ngOnDestroy() { this.log('OnDestroy'); }
333+
ngOnInit() { this.log('OnInit'); }
334+
335+
private log(hook: string) { logs.push(`${hook}(${this.value})`); }
336+
}
337+
338+
@NgModule({
339+
declarations: [Ng2Component],
340+
entryComponents: [Ng2Component],
341+
imports: [BrowserModule],
342+
})
343+
class Ng2Module {
344+
ngDoBootstrap() {}
345+
}
346+
347+
const bootstrapFn = (extraProviders: StaticProvider[]) =>
348+
platformBrowserDynamic(extraProviders).bootstrapModule(Ng2Module);
349+
const lazyModuleName = downgradeModule<Ng2Module>(bootstrapFn);
350+
const ng1Module =
351+
angular.module('ng1', [lazyModuleName])
352+
.directive('ng2', downgradeComponent({component: Ng2Component, propagateDigest}))
353+
.run(($rootScope: angular.IRootScopeService) => {
354+
rootScope = $rootScope;
355+
rootScope.value = 'bar';
356+
});
357+
358+
const element =
359+
html('<div><ng2 value="{{ value }}" ng-if="!hideNg2">Content</ng2></div>');
360+
angular.bootstrap(element, [ng1Module.name]);
361+
362+
setTimeout(() => { // Wait for the module to be bootstrapped.
363+
setTimeout(() => { // Wait for `$evalAsync()` to propagate inputs.
364+
const button = element.querySelector('button') !;
365+
366+
// Once initialized.
367+
expect(multiTrim(element.textContent)).toBe('bar Content');
368+
expect(logs).toEqual([
369+
// `ngOnChanges()` call triggered directly through the `inputChanges` $watcher.
370+
'OnChanges(bar)',
371+
// Initial CD triggered directly through the `detectChanges()` or `inputChanges`
372+
// $watcher (for `propagateDigest` true/false respectively).
373+
'OnInit(bar)',
374+
'DoCheck(bar)',
375+
'AfterContentInit(bar)',
376+
'AfterContentChecked(bar)',
377+
'AfterViewInit(bar)',
378+
'AfterViewChecked(bar)',
379+
...(propagateDigest ?
380+
[
381+
// CD triggered directly through the `detectChanges()` $watcher (2nd
382+
// $digest).
383+
'DoCheck(bar)',
384+
'AfterContentChecked(bar)',
385+
'AfterViewChecked(bar)',
386+
] :
387+
[]),
388+
// CD triggered due to entering/leaving the NgZone (in `downgradeFn()`).
389+
'DoCheck(bar)',
390+
'AfterContentChecked(bar)',
391+
'AfterViewChecked(bar)',
392+
]);
393+
logs.length = 0;
394+
395+
// Change inputs and run `$digest`.
396+
rootScope.$apply('value = "baz"');
397+
expect(multiTrim(element.textContent)).toBe('baz Content');
398+
expect(logs).toEqual([
399+
// `ngOnChanges()` call triggered directly through the `inputChanges` $watcher.
400+
'OnChanges(baz)',
401+
// `propagateDigest: true` (3 CD runs):
402+
// - CD triggered due to entering/leaving the NgZone (in `inputChanges`
403+
// $watcher).
404+
// - CD triggered directly through the `detectChanges()` $watcher.
405+
// - CD triggered due to entering/leaving the NgZone (in `detectChanges`
406+
// $watcher).
407+
// `propagateDigest: false` (2 CD runs):
408+
// - CD triggered directly through the `inputChanges` $watcher.
409+
// - CD triggered due to entering/leaving the NgZone (in `inputChanges`
410+
// $watcher).
411+
'DoCheck(baz)',
412+
'AfterContentChecked(baz)',
413+
'AfterViewChecked(baz)',
414+
'DoCheck(baz)',
415+
'AfterContentChecked(baz)',
416+
'AfterViewChecked(baz)',
417+
...(propagateDigest ?
418+
[
419+
'DoCheck(baz)',
420+
'AfterContentChecked(baz)',
421+
'AfterViewChecked(baz)',
422+
] :
423+
[]),
424+
]);
425+
logs.length = 0;
426+
427+
// Run `$digest` (without changing inputs).
428+
rootScope.$digest();
429+
expect(multiTrim(element.textContent)).toBe('baz Content');
430+
expect(logs).toEqual(
431+
propagateDigest ?
432+
[
433+
// CD triggered directly through the `detectChanges()` $watcher.
434+
'DoCheck(baz)',
435+
'AfterContentChecked(baz)',
436+
'AfterViewChecked(baz)',
437+
// CD triggered due to entering/leaving the NgZone (in the above $watcher).
438+
'DoCheck(baz)',
439+
'AfterContentChecked(baz)',
440+
'AfterViewChecked(baz)',
441+
] :
442+
[]);
443+
logs.length = 0;
444+
445+
// Trigger change detection (without changing inputs).
446+
button.click();
447+
expect(multiTrim(element.textContent)).toBe('qux Content');
448+
expect(logs).toEqual([
449+
'DoCheck(qux)',
450+
'AfterContentChecked(qux)',
451+
'AfterViewChecked(qux)',
452+
]);
453+
logs.length = 0;
454+
455+
// Destroy the component.
456+
rootScope.$apply('hideNg2 = true');
457+
expect(logs).toEqual([
458+
'OnDestroy(qux)',
459+
]);
460+
logs.length = 0;
461+
});
462+
});
463+
}));
464+
309465
it('should detach hostViews from the ApplicationRef once destroyed', async(() => {
310466
let ng2Component: Ng2Component;
311467

@@ -339,17 +495,18 @@ export function main() {
339495
const $injector = angular.bootstrap(element, [ng1Module.name]);
340496
const $rootScope = $injector.get($ROOT_SCOPE) as angular.IRootScopeService;
341497

342-
// Wait for the module to be bootstrapped.
343-
setTimeout(() => {
344-
const hostView: ViewRef =
345-
(ng2Component.appRef.attachView as jasmine.Spy).calls.mostRecent().args[0];
498+
setTimeout(() => { // Wait for the module to be bootstrapped.
499+
setTimeout(() => { // Wait for the hostView to be attached (during the `$digest`).
500+
const hostView: ViewRef =
501+
(ng2Component.appRef.attachView as jasmine.Spy).calls.mostRecent().args[0];
346502

347-
expect(hostView.destroyed).toBe(false);
503+
expect(hostView.destroyed).toBe(false);
348504

349-
$rootScope.$apply('hideNg2 = true');
505+
$rootScope.$apply('hideNg2 = true');
350506

351-
expect(hostView.destroyed).toBe(true);
352-
expect(ng2Component.appRef.detachView).toHaveBeenCalledWith(hostView);
507+
expect(hostView.destroyed).toBe(true);
508+
expect(ng2Component.appRef.detachView).toHaveBeenCalledWith(hostView);
509+
});
353510
});
354511
}));
355512

0 commit comments

Comments
 (0)