Skip to content

Commit 0487fbe

Browse files
pkozlowski-opensourcejasonaden
authored andcommitted
fix(ivy): throw a descriptive error when trying to insert / move destroyed view (angular#27289)
PR Close angular#27289
1 parent d0e8020 commit 0487fbe

File tree

3 files changed

+39
-36
lines changed

3 files changed

+39
-36
lines changed

packages/core/src/render3/component_ref.ts

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,7 @@ export class ComponentRef<T> extends viewEngine_ComponentRef<T> {
265265
ngDevMode && assertDefined(this.destroyCbs, 'NgModule already destroyed');
266266
this.destroyCbs !.forEach(fn => fn());
267267
this.destroyCbs = null;
268+
this.hostView.destroy();
268269
}
269270
onDestroy(callback: () => void): void {
270271
ngDevMode && assertDefined(this.destroyCbs, 'NgModule already destroyed');

packages/core/src/render3/view_engine_compatibility.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,9 @@ export function createContainerRef(
265265
}
266266

267267
move(viewRef: viewEngine_ViewRef, newIndex: number): viewEngine_ViewRef {
268+
if (viewRef.destroyed) {
269+
throw new Error('Cannot move a destroyed View in a ViewContainer!');
270+
}
268271
const index = this.indexOf(viewRef);
269272
this.detach(index);
270273
this.insert(viewRef, this._adjustIndex(newIndex));

packages/core/test/linker/integration_spec.ts

Lines changed: 35 additions & 36 deletions
Original file line numberDiff line numberDiff line change
@@ -995,17 +995,18 @@ function declareTests(config?: {useJit: boolean}) {
995995
expect(dir.receivedArgs).toEqual(['one', undefined]);
996996
});
997997

998-
fixmeIvy('unknown') && it('should not allow pipes in hostListeners', () => {
999-
@Directive({selector: '[host-listener]', host: {'(click)': 'doIt() | somePipe'}})
1000-
class DirectiveWithHostListener {
1001-
}
998+
fixmeIvy('FW-742: Pipes in host listeners should throw a descriptive error') &&
999+
it('should not allow pipes in hostListeners', () => {
1000+
@Directive({selector: '[host-listener]', host: {'(click)': 'doIt() | somePipe'}})
1001+
class DirectiveWithHostListener {
1002+
}
10021003

1003-
TestBed.configureTestingModule({declarations: [MyComp, DirectiveWithHostListener]});
1004-
const template = '<div host-listener></div>';
1005-
TestBed.overrideComponent(MyComp, {set: {template}});
1006-
expect(() => TestBed.createComponent(MyComp))
1007-
.toThrowError(/Cannot have a pipe in an action expression/);
1008-
});
1004+
TestBed.configureTestingModule({declarations: [MyComp, DirectiveWithHostListener]});
1005+
const template = '<div host-listener></div>';
1006+
TestBed.overrideComponent(MyComp, {set: {template}});
1007+
expect(() => TestBed.createComponent(MyComp))
1008+
.toThrowError(/Cannot have a pipe in an action expression/);
1009+
});
10091010

10101011

10111012

@@ -1238,37 +1239,35 @@ function declareTests(config?: {useJit: boolean}) {
12381239
});
12391240

12401241
describe('.insert', () => {
1241-
fixmeIvy('unknown') &&
1242-
it('should throw with destroyed views', async(() => {
1243-
const fixture = TestBed.configureTestingModule({schemas: [NO_ERRORS_SCHEMA]})
1244-
.createComponent(MyComp);
1245-
const tc = fixture.debugElement.children[0].children[0];
1246-
const dynamicVp: DynamicViewport = tc.injector.get(DynamicViewport);
1247-
const ref = dynamicVp.create();
1248-
fixture.detectChanges();
1242+
it('should throw with destroyed views', async(() => {
1243+
const fixture = TestBed.configureTestingModule({schemas: [NO_ERRORS_SCHEMA]})
1244+
.createComponent(MyComp);
1245+
const tc = fixture.debugElement.children[0].children[0];
1246+
const dynamicVp: DynamicViewport = tc.injector.get(DynamicViewport);
1247+
const ref = dynamicVp.create();
1248+
fixture.detectChanges();
12491249

1250-
ref.destroy();
1251-
expect(() => {
1252-
dynamicVp.insert(ref.hostView);
1253-
}).toThrowError('Cannot insert a destroyed View in a ViewContainer!');
1254-
}));
1250+
ref.destroy();
1251+
expect(() => {
1252+
dynamicVp.insert(ref.hostView);
1253+
}).toThrowError('Cannot insert a destroyed View in a ViewContainer!');
1254+
}));
12551255
});
12561256

12571257
describe('.move', () => {
1258-
fixmeIvy('unknown') &&
1259-
it('should throw with destroyed views', async(() => {
1260-
const fixture = TestBed.configureTestingModule({schemas: [NO_ERRORS_SCHEMA]})
1261-
.createComponent(MyComp);
1262-
const tc = fixture.debugElement.children[0].children[0];
1263-
const dynamicVp: DynamicViewport = tc.injector.get(DynamicViewport);
1264-
const ref = dynamicVp.create();
1265-
fixture.detectChanges();
1258+
it('should throw with destroyed views', async(() => {
1259+
const fixture = TestBed.configureTestingModule({schemas: [NO_ERRORS_SCHEMA]})
1260+
.createComponent(MyComp);
1261+
const tc = fixture.debugElement.children[0].children[0];
1262+
const dynamicVp: DynamicViewport = tc.injector.get(DynamicViewport);
1263+
const ref = dynamicVp.create();
1264+
fixture.detectChanges();
12661265

1267-
ref.destroy();
1268-
expect(() => {
1269-
dynamicVp.move(ref.hostView, 1);
1270-
}).toThrowError('Cannot move a destroyed View in a ViewContainer!');
1271-
}));
1266+
ref.destroy();
1267+
expect(() => {
1268+
dynamicVp.move(ref.hostView, 1);
1269+
}).toThrowError('Cannot move a destroyed View in a ViewContainer!');
1270+
}));
12721271
});
12731272

12741273
});

0 commit comments

Comments
 (0)