fix(ivy): detach ViewRef from ApplicationRef on destroy (#27276)

Currently we store the `_appRef` when a `ViewRef` is attached, however we don't use it for anything. These changes use it to detach the view from the `ApplicationRef` when it is destroyed. These changes also fix that the `ComponentRef` doesn't remove its `ViewRef` on destroy.

PR Close #27276
This commit is contained in:
Kristiyan Kostadinov 2018-11-26 21:56:29 +01:00 committed by Jason Aden
parent 0487fbe236
commit 4622d0b23a
2 changed files with 265 additions and 231 deletions

View File

@ -59,7 +59,9 @@ export class ViewRef<T> implements viewEngine_EmbeddedViewRef<T>, viewEngine_Int
} }
destroy(): void { destroy(): void {
if (this._viewContainerRef && viewAttached(this._view)) { if (this._appRef) {
this._appRef.detachView(this);
} else if (this._viewContainerRef && viewAttached(this._view)) {
this._viewContainerRef.detach(this._viewContainerRef.indexOf(this)); this._viewContainerRef.detach(this._viewContainerRef.indexOf(this));
this._viewContainerRef = null; this._viewContainerRef = null;
} }

View File

@ -25,7 +25,7 @@ class SomeComponent {
} }
{ {
fixmeIvy('unknown') && describe('bootstrap', () => { describe('bootstrap', () => {
let mockConsole: MockConsole; let mockConsole: MockConsole;
beforeEach(() => { mockConsole = new MockConsole(); }); beforeEach(() => { mockConsole = new MockConsole(); });
@ -74,6 +74,7 @@ class SomeComponent {
return MyModule; return MyModule;
} }
fixmeIvy('unknown') &&
it('should bootstrap a component from a child module', it('should bootstrap a component from a child module',
async(inject([ApplicationRef, Compiler], (app: ApplicationRef, compiler: Compiler) => { async(inject([ApplicationRef, Compiler], (app: ApplicationRef, compiler: Compiler) => {
@Component({ @Component({
@ -102,6 +103,7 @@ class SomeComponent {
expect(component.injector.get('hello')).toEqual('component'); expect(component.injector.get('hello')).toEqual('component');
}))); })));
fixmeIvy('unknown') &&
it('should bootstrap a component with a custom selector', it('should bootstrap a component with a custom selector',
async(inject([ApplicationRef, Compiler], (app: ApplicationRef, compiler: Compiler) => { async(inject([ApplicationRef, Compiler], (app: ApplicationRef, compiler: Compiler) => {
@Component({ @Component({
@ -133,7 +135,7 @@ class SomeComponent {
describe('ApplicationRef', () => { describe('ApplicationRef', () => {
beforeEach(() => { TestBed.configureTestingModule({imports: [createModule()]}); }); beforeEach(() => { TestBed.configureTestingModule({imports: [createModule()]}); });
it('should throw when reentering tick', () => { fixmeIvy('unknown') && it('should throw when reentering tick', () => {
@Component({template: '{{reenter()}}'}) @Component({template: '{{reenter()}}'})
class ReenteringComponent { class ReenteringComponent {
reenterCount = 1; reenterCount = 1;
@ -174,7 +176,7 @@ class SomeComponent {
}); });
}); });
it('should be called when a component is bootstrapped', fixmeIvy('unknown') && it('should be called when a component is bootstrapped',
inject([ApplicationRef], (ref: ApplicationRef) => { inject([ApplicationRef], (ref: ApplicationRef) => {
createRootEl(); createRootEl();
const compRef = ref.bootstrap(SomeComponent); const compRef = ref.bootstrap(SomeComponent);
@ -183,12 +185,15 @@ class SomeComponent {
}); });
describe('bootstrap', () => { describe('bootstrap', () => {
fixmeIvy('unknown') &&
it('should throw if an APP_INITIIALIZER is not yet resolved', it('should throw if an APP_INITIIALIZER is not yet resolved',
withModule( withModule(
{ {
providers: [ providers: [{
{provide: APP_INITIALIZER, useValue: () => new Promise(() => {}), multi: true} provide: APP_INITIALIZER,
] useValue: () => new Promise(() => {}),
multi: true
}]
}, },
inject([ApplicationRef], (ref: ApplicationRef) => { inject([ApplicationRef], (ref: ApplicationRef) => {
createRootEl(); createRootEl();
@ -206,6 +211,7 @@ class SomeComponent {
defaultPlatform = _platform; defaultPlatform = _platform;
})); }));
fixmeIvy('unknown') &&
it('should wait for asynchronous app initializers', async(() => { it('should wait for asynchronous app initializers', async(() => {
let resolve: (result: any) => void; let resolve: (result: any) => void;
const promise: Promise<any> = new Promise((res) => { resolve = res; }); const promise: Promise<any> = new Promise((res) => { resolve = res; });
@ -216,15 +222,18 @@ class SomeComponent {
}, 1); }, 1);
defaultPlatform defaultPlatform
.bootstrapModule( .bootstrapModule(createModule(
createModule([{provide: APP_INITIALIZER, useValue: () => promise, multi: true}])) [{provide: APP_INITIALIZER, useValue: () => promise, multi: true}]))
.then(_ => { expect(initializerDone).toBe(true); }); .then(_ => { expect(initializerDone).toBe(true); });
})); }));
it('should rethrow sync errors even if the exceptionHandler is not rethrowing', async(() => { fixmeIvy('unknown') &&
it('should rethrow sync errors even if the exceptionHandler is not rethrowing',
async(() => {
defaultPlatform defaultPlatform
.bootstrapModule(createModule( .bootstrapModule(createModule([
[{provide: APP_INITIALIZER, useValue: () => { throw 'Test'; }, multi: true}])) {provide: APP_INITIALIZER, useValue: () => { throw 'Test'; }, multi: true}
]))
.then(() => expect(false).toBe(true), (e) => { .then(() => expect(false).toBe(true), (e) => {
expect(e).toBe('Test'); expect(e).toBe('Test');
// Error rethrown will be seen by the exception handler since it's after // Error rethrown will be seen by the exception handler since it's after
@ -233,18 +242,22 @@ class SomeComponent {
}); });
})); }));
fixmeIvy('unknown') &&
it('should rethrow promise errors even if the exceptionHandler is not rethrowing', it('should rethrow promise errors even if the exceptionHandler is not rethrowing',
async(() => { async(() => {
defaultPlatform defaultPlatform
.bootstrapModule(createModule([ .bootstrapModule(createModule([{
{provide: APP_INITIALIZER, useValue: () => Promise.reject('Test'), multi: true} provide: APP_INITIALIZER,
])) useValue: () => Promise.reject('Test'),
multi: true
}]))
.then(() => expect(false).toBe(true), (e) => { .then(() => expect(false).toBe(true), (e) => {
expect(e).toBe('Test'); expect(e).toBe('Test');
expect(mockConsole.res[0].join('#')).toEqual('ERROR#Test'); expect(mockConsole.res[0].join('#')).toEqual('ERROR#Test');
}); });
})); }));
fixmeIvy('unknown') &&
it('should throw useful error when ApplicationRef is not configured', async(() => { it('should throw useful error when ApplicationRef is not configured', async(() => {
@NgModule() @NgModule()
class EmptyModule { class EmptyModule {
@ -257,6 +270,7 @@ class SomeComponent {
}); });
})); }));
fixmeIvy('unknown') &&
it('should call the `ngDoBootstrap` method with `ApplicationRef` on the main module', it('should call the `ngDoBootstrap` method with `ApplicationRef` on the main module',
async(() => { async(() => {
const ngDoBootstrap = jasmine.createSpy('ngDoBootstrap'); const ngDoBootstrap = jasmine.createSpy('ngDoBootstrap');
@ -267,6 +281,7 @@ class SomeComponent {
}); });
})); }));
fixmeIvy('unknown') &&
it('should auto bootstrap components listed in @NgModule.bootstrap', async(() => { it('should auto bootstrap components listed in @NgModule.bootstrap', async(() => {
defaultPlatform.bootstrapModule(createModule({bootstrap: [SomeComponent]})) defaultPlatform.bootstrapModule(createModule({bootstrap: [SomeComponent]}))
.then((moduleRef) => { .then((moduleRef) => {
@ -275,6 +290,7 @@ class SomeComponent {
}); });
})); }));
fixmeIvy('unknown') &&
it('should error if neither `ngDoBootstrap` nor @NgModule.bootstrap was specified', it('should error if neither `ngDoBootstrap` nor @NgModule.bootstrap was specified',
async(() => { async(() => {
defaultPlatform.bootstrapModule(createModule({ngDoBootstrap: false})) defaultPlatform.bootstrapModule(createModule({ngDoBootstrap: false}))
@ -286,11 +302,13 @@ class SomeComponent {
}); });
})); }));
fixmeIvy('unknown') &&
it('should add bootstrapped module into platform modules list', async(() => { it('should add bootstrapped module into platform modules list', async(() => {
defaultPlatform.bootstrapModule(createModule({bootstrap: [SomeComponent]})) defaultPlatform.bootstrapModule(createModule({bootstrap: [SomeComponent]}))
.then(module => expect((<any>defaultPlatform)._modules).toContain(module)); .then(module => expect((<any>defaultPlatform)._modules).toContain(module));
})); }));
fixmeIvy('unknown') &&
it('should bootstrap with NoopNgZone', async(() => { it('should bootstrap with NoopNgZone', async(() => {
defaultPlatform defaultPlatform
.bootstrapModule(createModule({bootstrap: [SomeComponent]}), {ngZone: 'noop'}) .bootstrapModule(createModule({bootstrap: [SomeComponent]}), {ngZone: 'noop'})
@ -307,6 +325,7 @@ class SomeComponent {
createRootEl(); createRootEl();
defaultPlatform = _platform; defaultPlatform = _platform;
})); }));
fixmeIvy('unknown') &&
it('should wait for asynchronous app initializers', async(() => { it('should wait for asynchronous app initializers', async(() => {
let resolve: (result: any) => void; let resolve: (result: any) => void;
const promise: Promise<any> = new Promise((res) => { resolve = res; }); const promise: Promise<any> = new Promise((res) => { resolve = res; });
@ -318,30 +337,40 @@ class SomeComponent {
const compilerFactory: CompilerFactory = const compilerFactory: CompilerFactory =
defaultPlatform.injector.get(CompilerFactory, null); defaultPlatform.injector.get(CompilerFactory, null);
const moduleFactory = compilerFactory.createCompiler().compileModuleSync( const moduleFactory =
createModule([{provide: APP_INITIALIZER, useValue: () => promise, multi: true}])); compilerFactory.createCompiler().compileModuleSync(createModule(
[{provide: APP_INITIALIZER, useValue: () => promise, multi: true}]));
defaultPlatform.bootstrapModuleFactory(moduleFactory).then(_ => { defaultPlatform.bootstrapModuleFactory(moduleFactory).then(_ => {
expect(initializerDone).toBe(true); expect(initializerDone).toBe(true);
}); });
})); }));
it('should rethrow sync errors even if the exceptionHandler is not rethrowing', async(() => { fixmeIvy('unknown') &&
it('should rethrow sync errors even if the exceptionHandler is not rethrowing',
async(() => {
const compilerFactory: CompilerFactory = const compilerFactory: CompilerFactory =
defaultPlatform.injector.get(CompilerFactory, null); defaultPlatform.injector.get(CompilerFactory, null);
const moduleFactory = compilerFactory.createCompiler().compileModuleSync(createModule( const moduleFactory =
[{provide: APP_INITIALIZER, useValue: () => { throw 'Test'; }, multi: true}])); compilerFactory.createCompiler().compileModuleSync(createModule([
{provide: APP_INITIALIZER, useValue: () => { throw 'Test'; }, multi: true}
]));
expect(() => defaultPlatform.bootstrapModuleFactory(moduleFactory)).toThrow('Test'); expect(() => defaultPlatform.bootstrapModuleFactory(moduleFactory)).toThrow('Test');
// Error rethrown will be seen by the exception handler since it's after // Error rethrown will be seen by the exception handler since it's after
// construction. // construction.
expect(mockConsole.res[0].join('#')).toEqual('ERROR#Test'); expect(mockConsole.res[0].join('#')).toEqual('ERROR#Test');
})); }));
fixmeIvy('unknown') &&
it('should rethrow promise errors even if the exceptionHandler is not rethrowing', it('should rethrow promise errors even if the exceptionHandler is not rethrowing',
async(() => { async(() => {
const compilerFactory: CompilerFactory = const compilerFactory: CompilerFactory =
defaultPlatform.injector.get(CompilerFactory, null); defaultPlatform.injector.get(CompilerFactory, null);
const moduleFactory = compilerFactory.createCompiler().compileModuleSync(createModule( const moduleFactory =
[{provide: APP_INITIALIZER, useValue: () => Promise.reject('Test'), multi: true}])); compilerFactory.createCompiler().compileModuleSync(createModule([{
provide: APP_INITIALIZER,
useValue: () => Promise.reject('Test'),
multi: true
}]));
defaultPlatform.bootstrapModuleFactory(moduleFactory) defaultPlatform.bootstrapModuleFactory(moduleFactory)
.then(() => expect(false).toBe(true), (e) => { .then(() => expect(false).toBe(true), (e) => {
expect(e).toBe('Test'); expect(e).toBe('Test');
@ -427,6 +456,7 @@ class SomeComponent {
expect(appRef.viewCount).toBe(0); expect(appRef.viewCount).toBe(0);
}); });
fixmeIvy('unknown') &&
it('should not allow to attach a view to both, a view container and the ApplicationRef', it('should not allow to attach a view to both, a view container and the ApplicationRef',
() => { () => {
const comp = TestBed.createComponent(MyComp); const comp = TestBed.createComponent(MyComp);
@ -448,7 +478,7 @@ class SomeComponent {
}); });
}); });
fixmeIvy('unknown') && describe('AppRef', () => { describe('AppRef', () => {
@Component({selector: 'sync-comp', template: `<span>{{text}}</span>`}) @Component({selector: 'sync-comp', template: `<span>{{text}}</span>`})
class SyncComp { class SyncComp {
text: string = '1'; text: string = '1';
@ -535,18 +565,20 @@ class SomeComponent {
}); });
} }
it('isStable should fire on synchronous component loading', fixmeIvy('unknown') && it('isStable should fire on synchronous component loading',
async(() => { expectStableTexts(SyncComp, ['1']); })); async(() => { expectStableTexts(SyncComp, ['1']); }));
it('isStable should fire after a microtask on init is completed', fixmeIvy('unknown') && it('isStable should fire after a microtask on init is completed',
async(() => { expectStableTexts(MicroTaskComp, ['11']); })); async(() => { expectStableTexts(MicroTaskComp, ['11']); }));
it('isStable should fire after a macrotask on init is completed', fixmeIvy('unknown') && it('isStable should fire after a macrotask on init is completed',
async(() => { expectStableTexts(MacroTaskComp, ['11']); })); async(() => { expectStableTexts(MacroTaskComp, ['11']); }));
fixmeIvy('unknown') &&
it('isStable should fire only after chain of micro and macrotasks on init are completed', it('isStable should fire only after chain of micro and macrotasks on init are completed',
async(() => { expectStableTexts(MicroMacroTaskComp, ['111']); })); async(() => { expectStableTexts(MicroMacroTaskComp, ['111']); }));
fixmeIvy('unknown') &&
it('isStable should fire only after chain of macro and microtasks on init are completed', it('isStable should fire only after chain of macro and microtasks on init are completed',
async(() => { expectStableTexts(MacroMicroTaskComp, ['111']); })); async(() => { expectStableTexts(MacroMicroTaskComp, ['111']); }));
@ -569,7 +601,7 @@ class SomeComponent {
}); });
} }
it('should be fired after app becomes unstable', async(() => { fixmeIvy('unknown') && it('should be fired after app becomes unstable', async(() => {
const fixture = TestBed.createComponent(ClickComp); const fixture = TestBed.createComponent(ClickComp);
const appRef: ApplicationRef = TestBed.get(ApplicationRef); const appRef: ApplicationRef = TestBed.get(ApplicationRef);
const zone: NgZone = TestBed.get(NgZone); const zone: NgZone = TestBed.get(NgZone);