Skip to content

Commit 11b206b

Browse files
leonsenftatscott
leonsenft
authored and
atscott
committed
fix(core): introduce disposal mechanism for Angular views in foreign @content
Coordinate template lifecycle events between Angular and foreign components to allow clean teardown of nested Angular views inside a foreign container. Previously, when Angular content was projected into a foreign component (for instance, via render props), Angular had no way to receive destruction notifications from the foreign component. If the foreign component unmounted or conditionally removed its children, the nested Angular views remained active, leading to memory leaks and incomplete lifecycle teardowns. This change introduces the `ON_DESTROY` symbol and a new registration mechanism (`ForeignOnDestroyFn`) on the `ForeignComponent` interface. The `foreignImport` helper now takes an additional `onDestroy` callback function where the foreign component can register to receive Angular's view-destruction callback. During the creation phase, `ɵɵforeignContentFn` resolves the foreign component from the constant pool using a new constant pool index and invokes the `onDestroy` function. This registers a callback that destroys the corresponding embedded view from the container. In the compiler, `ForeignComponentOp` is modified to track the target constant pool index, and `ForeignContentExpr` reification is updated to pass this index to `ɵɵforeignContentFn`.
1 parent 25c744c commit 11b206b

12 files changed

Lines changed: 290 additions & 109 deletions

File tree

‎packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/foreign_component.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ export class TestCmpRenderProps {
8888
template: function TestCmpRenderProps_Template(rf, ctx) {
8989
if (rf & 1) {
9090
i0.ɵɵdomTemplate(0, TestCmpRenderProps_Items_0_Template, 2, 2);
91-
i0.ɵɵforeignComponent(1, 0, { label: ctx.title, items: i0.ɵɵforeignContentFn(0) });
91+
i0.ɵɵforeignComponent(1, 0, { label: ctx.title, items: i0.ɵɵforeignContentFn(0, 0) });
9292
}
9393
},
9494
encapsulation: 2

‎packages/compiler-cli/test/compliance/test_cases/r3_compiler_compliance/components_and_directives/standalone/foreign_component.local.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ export class TestCmpRenderProps {
8888
template: function TestCmpRenderProps_Template(rf, ctx) {
8989
if (rf & 1) {
9090
i0.ɵɵtemplate(0, TestCmpRenderProps_Items_0_Template, 2, 2);
91-
i0.ɵɵforeignComponent(1, 0, { label: ctx.title, items: i0.ɵɵforeignContentFn(0) });
91+
i0.ɵɵforeignComponent(1, 0, { label: ctx.title, items: i0.ɵɵforeignContentFn(0, 0) });
9292
}
9393
},
9494
encapsulation: 2

‎packages/compiler/src/template/pipeline/ir/src/expression.ts‎

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {CONTEXT_NAME} from '../../../../render3/view/util';
1414
import {ExpressionKind, OpKind} from './enums';
1515
import {SlotHandle} from './handle';
1616
import {OpList, type XrefId} from './operations';
17-
import type {CreateOp} from './ops/create';
17+
import type {ConstIndex, CreateOp} from './ops/create';
1818
import {createStatementOp} from './ops/shared';
1919
import {Interpolation, type UpdateOp} from './ops/update';
2020
import {
@@ -161,14 +161,19 @@ export class ForeignContentExpr extends ExpressionBase {
161161
constructor(
162162
readonly childrenViewXref: XrefId,
163163
readonly childrenViewHandle: SlotHandle,
164+
readonly foreignComponentConstIndex: ConstIndex,
164165
) {
165166
super();
166167
}
167168

168169
override visitExpression(): void {}
169170

170171
override isEquivalent(e: o.Expression): boolean {
171-
return e instanceof ForeignContentExpr && e.childrenViewXref === this.childrenViewXref;
172+
return (
173+
e instanceof ForeignContentExpr &&
174+
e.childrenViewXref === this.childrenViewXref &&
175+
e.foreignComponentConstIndex === this.foreignComponentConstIndex
176+
);
172177
}
173178

174179
override isConstant(): boolean {
@@ -178,7 +183,11 @@ export class ForeignContentExpr extends ExpressionBase {
178183
override transformInternalExpressions(): void {}
179184

180185
override clone(): ForeignContentExpr {
181-
return new ForeignContentExpr(this.childrenViewXref, this.childrenViewHandle);
186+
return new ForeignContentExpr(
187+
this.childrenViewXref,
188+
this.childrenViewHandle,
189+
this.foreignComponentConstIndex,
190+
);
182191
}
183192
}
184193

‎packages/compiler/src/template/pipeline/ir/src/ops/create.ts‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -251,9 +251,9 @@ export interface ForeignComponentOp extends Op<CreateOp>, ConsumesSlotOpTrait {
251251
xref: XrefId;
252252

253253
/**
254-
* Reference to the foreign component class/function itself as an output AST expression.
254+
* Index of the foreign component class/function in the constant pool.
255255
*/
256-
foreignComponentRef: o.Expression;
256+
constIndex: ConstIndex;
257257

258258
/**
259259
* Static attributes and property bindings.
@@ -268,15 +268,15 @@ export interface ForeignComponentOp extends Op<CreateOp>, ConsumesSlotOpTrait {
268268
*/
269269
export function createForeignComponentOp(
270270
xref: XrefId,
271-
foreignComponentRef: o.Expression,
271+
constIndex: ConstIndex,
272272
props: Map<string, o.Expression>,
273273
sourceSpan: ParseSourceSpan | null,
274274
): ForeignComponentOp {
275275
return {
276276
kind: OpKind.ForeignComponent,
277277
xref,
278278
handle: new SlotHandle(),
279-
foreignComponentRef,
279+
constIndex,
280280
props,
281281
sourceSpan,
282282
...TRAIT_CONSUMES_SLOT,

‎packages/compiler/src/template/pipeline/src/ingest.ts‎

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -401,9 +401,7 @@ function ingestForeignComponent(
401401
// through directly passed signal properties, alleviating the need for any explicit update
402402
// operations.
403403
const constIndex = unit.job.addConst(foreignComp.component);
404-
unit.create.push(
405-
ir.createForeignComponentOp(id, o.literal(constIndex), props, element.startSourceSpan),
406-
);
404+
unit.create.push(ir.createForeignComponentOp(id, constIndex, props, element.startSourceSpan));
407405
}
408406

409407
/**

‎packages/compiler/src/template/pipeline/src/phases/reify.ts‎

Lines changed: 7 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -156,7 +156,7 @@ function reifyCreateOperations(unit: CompilationUnit, ops: ir.OpList<ir.CreateOp
156156
: null;
157157
ir.OpList.replace(
158158
op,
159-
ng.foreignComponent(op.handle.slot!, op.foreignComponentRef, propsExpr, op.sourceSpan),
159+
ng.foreignComponent(op.handle.slot!, o.literal(op.constIndex), propsExpr, op.sourceSpan),
160160
);
161161
break;
162162
case ir.OpKind.ElementEnd:
@@ -804,8 +804,12 @@ function reifyIrExpression(unit: CompilationUnit, expr: o.Expression): o.Express
804804
throw new Error(`AssertionError: must be compiling a component`);
805805
}
806806
const isFn = unit.job.views.get(expr.childrenViewXref)!.contextVariables.size > 0;
807-
const identifier = isFn ? Identifiers.foreignContentFn : Identifiers.foreignContent;
808-
return o.importExpr(identifier).callFn([o.literal(expr.childrenViewHandle.slot!)]);
807+
const slot = o.literal(expr.childrenViewHandle.slot!);
808+
return isFn
809+
? o
810+
.importExpr(Identifiers.foreignContentFn)
811+
.callFn([slot, o.literal(expr.foreignComponentConstIndex)])
812+
: o.importExpr(Identifiers.foreignContent).callFn([slot]);
809813
case ir.ExpressionKind.LexicalRead:
810814
throw new Error(`AssertionError: unresolved LexicalRead of ${expr.name}`);
811815
case ir.ExpressionKind.TwoWayBindingSet:

‎packages/compiler/src/template/pipeline/src/phases/resolve_foreign_content.ts‎

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

9-
import * as o from '../../../../output/output_ast';
109
import * as ir from '../../ir';
1110
import type {CompilationJob} from '../compilation';
1211

@@ -48,7 +47,11 @@ export function resolveForeignContent(job: CompilationJob): void {
4847

4948
ir.OpList.replace<ir.CreateOp>(op, templateOp);
5049

51-
const foreignContent = new ir.ForeignContentExpr(templateOp.xref, templateOp.handle);
50+
const foreignContent = new ir.ForeignContentExpr(
51+
templateOp.xref,
52+
templateOp.handle,
53+
target.constIndex,
54+
);
5255
target.props.set(op.propertyName, foreignContent);
5356
}
5457
}

‎packages/core/src/interface/foreign_component.ts‎

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,9 @@
99
/** Symbol used to store and retrieve the render function for a foreign component. */
1010
export const RENDER: unique symbol = Symbol('RENDER');
1111

12+
/** Symbol used to store and retrieve the disposal registration function for a foreign component. */
13+
export const ON_DESTROY: unique symbol = Symbol('ON_DESTROY');
14+
1215
/**
1316
* A function used to render a foreign component in an Angular template.
1417
*
@@ -20,11 +23,22 @@ export const RENDER: unique symbol = Symbol('RENDER');
2023
*/
2124
export type ForeignRenderFn<TProps> = (props: TProps) => [Node[], VoidFunction?];
2225

26+
/**
27+
* A function that allows a foreign component to register a destroy callback.
28+
*
29+
* Angular will invoke this function during the creation phase of projected content
30+
* to provide a cleanup callback. The foreign component is responsible for calling
31+
* this callback when the container slot is removed or when the foreign component itself
32+
* is destroyed. This triggers the destruction and lifecycle cleanup of the nested Angular views.
33+
*/
34+
export type ForeignOnDestroyFn = (destroy: VoidFunction) => void;
35+
2336
/**
2437
* Represents a component from another framework that Angular can import and render.
2538
*
2639
* @template TProps The properties of the foreign component.
2740
*/
2841
export interface ForeignComponent<TProps> {
2942
readonly [RENDER]: ForeignRenderFn<TProps>;
43+
readonly [ON_DESTROY]: ForeignOnDestroyFn;
3044
}

‎packages/core/src/render3/foreign_import.ts‎

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -6,14 +6,27 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {ForeignComponent, ForeignRenderFn, RENDER} from '../interface/foreign_component';
9+
import {
10+
ForeignComponent,
11+
ForeignRenderFn,
12+
ForeignOnDestroyFn,
13+
RENDER,
14+
ON_DESTROY,
15+
} from '../interface/foreign_component';
1016

1117
/**
1218
* Returns a {@link ForeignComponent} for use in Angular components.
1319
*
1420
* @template TProps The properties of the foreign component.
1521
* @param render A function that renders a foreign component.
22+
* @param onDestroy A function for foreign content to register a destroy callback.
1623
*/
17-
export function foreignImport<TProps>(render: ForeignRenderFn<TProps>): ForeignComponent<TProps> {
18-
return {[RENDER]: render};
24+
export function foreignImport<TProps>(
25+
render: ForeignRenderFn<TProps>,
26+
onDestroy: ForeignOnDestroyFn,
27+
): ForeignComponent<TProps> {
28+
return {
29+
[RENDER]: render,
30+
[ON_DESTROY]: onDestroy,
31+
};
1932
}

‎packages/core/src/render3/instructions/foreign_component.ts‎

Lines changed: 25 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,17 +6,17 @@
66
* found in the LICENSE file at https://angular.dev/license
77
*/
88

9-
import {ForeignComponent, RENDER} from '../../interface/foreign_component';
9+
import {ForeignComponent, RENDER, ON_DESTROY} from '../../interface/foreign_component';
1010
import {attachPatchData} from '../context_discovery';
1111
import {nativeInsertBefore} from '../dom_node_manipulation';
1212
import {createForeignView} from '../foreign_view';
1313
import {TContainerNode, TNodeType} from '../interfaces/node';
14-
import {HEADER_OFFSET, RENDERER, TVIEW, FLAGS} from '../interfaces/view';
15-
import {appendChild} from '../node_manipulation';
14+
import {HEADER_OFFSET, RENDERER, TVIEW, FLAGS, LViewFlags} from '../interfaces/view';
15+
import {appendChild, destroyLView} from '../node_manipulation';
1616
import {getLView, getTView, setCurrentTNode, setCurrentTNodeAsNotParent} from '../state';
1717
import {getOrCreateTNode} from '../tnode_manipulation';
1818
import {addToEndOfViewTree} from '../view/construction';
19-
import {createLContainer, addLViewToLContainer} from '../view/container';
19+
import {createLContainer, addLViewToLContainer, removeLViewFromLContainer} from '../view/container';
2020
import {NodeInjector} from '../di';
2121
import {runInInjectionContext} from '../../di';
2222
import {Renderer} from '../interfaces/renderer';
@@ -26,6 +26,8 @@ import {collectNativeNodes} from '../collect_native_nodes';
2626
import {assertLContainer} from '../assert';
2727
import {CONTAINER_HEADER_OFFSET, LContainer, LContainerFlags} from '../interfaces/container';
2828
import {getConstant} from '../util/view_utils';
29+
import {isDestroyed} from '../interfaces/type_checks';
30+
import {assertNotEqual, assertNotSame} from '../../util/assert';
2931

3032
/**
3133
* Creation phase instruction to render a foreign component.
@@ -123,9 +125,13 @@ export function ɵɵforeignContent(index: number): any[] {
123125
* with arguments.
124126
*
125127
* @param index The index of the container in the data array.
128+
* @param foreignComponentConstIndex The index of the matched foreign component in the constant pool.
126129
* @codeGenApi
127130
*/
128-
export function ɵɵforeignContentFn(index: number): (...args: any[]) => any[] {
131+
export function ɵɵforeignContentFn(
132+
index: number,
133+
foreignComponentConstIndex: number,
134+
): (...args: any[]) => any[] {
129135
const lView = getLView();
130136
const adjustedIndex = index + HEADER_OFFSET;
131137

@@ -136,18 +142,32 @@ export function ɵɵforeignContentFn(index: number): (...args: any[]) => any[] {
136142

137143
const tView = getTView();
138144
const tNode = tView.data[adjustedIndex] as TContainerNode;
145+
const foreignComponent = getConstant<ForeignComponent<any>>(
146+
tView.consts,
147+
foreignComponentConstIndex,
148+
)!;
149+
const onDestroy = foreignComponent[ON_DESTROY];
139150

140151
return (...args: any[]) => {
141152
// When the function is called, instantiate and render a new embedded view inside the container.
142153
// The arguments are passed directly as the context of the view.
143154
const embeddedLView = createAndRenderEmbeddedLView(lView, tNode, args);
155+
144156
addLViewToLContainer(
145157
lContainer,
146158
embeddedLView,
147159
lContainer.length - CONTAINER_HEADER_OFFSET,
148160
/* addToDOM */ false,
149161
);
150162

163+
onDestroy(() => {
164+
if (!isDestroyed(embeddedLView)) {
165+
const embeddedLViewIndex = lContainer.indexOf(embeddedLView, CONTAINER_HEADER_OFFSET);
166+
ngDevMode && assertNotSame(embeddedLViewIndex, -1, 'Embedded view not found in container');
167+
removeLViewFromLContainer(lContainer, embeddedLViewIndex - CONTAINER_HEADER_OFFSET);
168+
}
169+
});
170+
151171
// Extract and return the root nodes of the created view
152172
const embeddedTView = embeddedLView[TVIEW];
153173
return collectNativeNodes(embeddedTView, embeddedLView, embeddedTView.firstChild, []);

0 commit comments

Comments
 (0)