Skip to content

Commit 3b1e4d4

Browse files
leonsenftleonsenft
committed
fix(compiler): support foreign components defined outside top-level scope
Currently, the template pipeline directly emits the raw expression for foreign component definitions (such as `frameworkImport(MyComponent)`) directly into the body of the generated template function. If a foreign component is defined inside a local scope or is non-exported (e.g. nested inside a test block), the emitted template function may not have access to that variable because `ɵɵdefineComponent` and its template functions are emitted at the top-level module scope. This previously caused reference errors during template compilation. This commit updates the compilation pipeline to instead ingest foreign component references into the component's `consts` pool. The `ɵɵforeignComponent` runtime instruction is updated to accept an index into the constant pool rather than a raw expression. By routing the references through the `consts` pool, block-scoped classes and variables are appropriately captured by `ngtsc` without scoping errors, properly supporting nested/local foreign component usage.
1 parent a6cdad6 commit 3b1e4d4

7 files changed

Lines changed: 64 additions & 43 deletions

File tree

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,10 @@ export class TestCmp {
4545
selectors: [["main"]],
4646
decls: 1,
4747
vars: 0,
48+
consts: [frameworkImport(FancyButton)],
4849
template: function TestCmp_Template(rf, ctx) {
4950
if (rf & 1) {
50-
i0.ɵɵforeignComponent(0, frameworkImport(FancyButton), { class: "btn-cls", "unsafe-attr": "value", label: ctx.title, "unsafe-input": ctx.title });
51+
i0.ɵɵforeignComponent(0, 0, { class: "btn-cls", "unsafe-attr": "value", label: ctx.title, "unsafe-input": ctx.title });
5152
}
5253
},
5354
encapsulation: 2
@@ -63,10 +64,11 @@ export class TestCmpChildren {
6364
selectors: [["main-children"]],
6465
decls: 4,
6566
vars: 0,
67+
consts: [frameworkImport(FancyButton)],
6668
template: function TestCmpChildren_Template(rf, ctx) {
6769
if (rf & 1) {
6870
i0.ɵɵdomTemplate(0, TestCmpChildren_Icon_0_Template, 2, 0)(1, TestCmpChildren_Description_1_Template, 2, 0)(2, TestCmpChildren_Children_2_Template, 2, 0);
69-
i0.ɵɵforeignComponent(3, frameworkImport(FancyButton), { label: ctx.title, icon: i0.ɵɵforeignContent(0), description: i0.ɵɵforeignContent(1), children: i0.ɵɵforeignContent(2) });
71+
i0.ɵɵforeignComponent(3, 0, { label: ctx.title, icon: i0.ɵɵforeignContent(0), description: i0.ɵɵforeignContent(1), children: i0.ɵɵforeignContent(2) });
7072
}
7173
},
7274
encapsulation: 2
@@ -82,10 +84,11 @@ export class TestCmpRenderProps {
8284
selectors: [["main-render-props"]],
8385
decls: 2,
8486
vars: 0,
87+
consts: [frameworkImport(FancyButton)],
8588
template: function TestCmpRenderProps_Template(rf, ctx) {
8689
if (rf & 1) {
8790
i0.ɵɵdomTemplate(0, TestCmpRenderProps_Items_0_Template, 2, 2);
88-
i0.ɵɵforeignComponent(1, frameworkImport(FancyButton), { label: ctx.title, items: i0.ɵɵforeignContentFn(0) });
91+
i0.ɵɵforeignComponent(1, 0, { label: ctx.title, items: i0.ɵɵforeignContentFn(0) });
8992
}
9093
},
9194
encapsulation: 2

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

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,9 +45,10 @@ export class TestCmp {
4545
selectors: [["main"]],
4646
decls: 1,
4747
vars: 0,
48+
consts: [frameworkImport(FancyButton)],
4849
template: function TestCmp_Template(rf, ctx) {
4950
if (rf & 1) {
50-
i0.ɵɵforeignComponent(0, frameworkImport(FancyButton), { class: "btn-cls", "unsafe-attr": "value", label: ctx.title, "unsafe-input": ctx.title });
51+
i0.ɵɵforeignComponent(0, 0, { class: "btn-cls", "unsafe-attr": "value", label: ctx.title, "unsafe-input": ctx.title });
5152
}
5253
},
5354
encapsulation: 2
@@ -63,10 +64,11 @@ export class TestCmpChildren {
6364
selectors: [["main-children"]],
6465
decls: 4,
6566
vars: 0,
67+
consts: [frameworkImport(FancyButton)],
6668
template: function TestCmpChildren_Template(rf, ctx) {
6769
if (rf & 1) {
6870
i0.ɵɵtemplate(0, TestCmpChildren_Icon_0_Template, 2, 0)(1, TestCmpChildren_Description_1_Template, 2, 0)(2, TestCmpChildren_Children_2_Template, 2, 0);
69-
i0.ɵɵforeignComponent(3, frameworkImport(FancyButton), { label: ctx.title, icon: i0.ɵɵforeignContent(0), description: i0.ɵɵforeignContent(1), children: i0.ɵɵforeignContent(2) });
71+
i0.ɵɵforeignComponent(3, 0, { label: ctx.title, icon: i0.ɵɵforeignContent(0), description: i0.ɵɵforeignContent(1), children: i0.ɵɵforeignContent(2) });
7072
}
7173
},
7274
encapsulation: 2
@@ -82,10 +84,11 @@ export class TestCmpRenderProps {
8284
selectors: [["main-render-props"]],
8385
decls: 2,
8486
vars: 0,
87+
consts: [frameworkImport(FancyButton)],
8588
template: function TestCmpRenderProps_Template(rf, ctx) {
8689
if (rf & 1) {
8790
i0.ɵɵtemplate(0, TestCmpRenderProps_Items_0_Template, 2, 2);
88-
i0.ɵɵforeignComponent(1, frameworkImport(FancyButton), { label: ctx.title, items: i0.ɵɵforeignContentFn(0) });
91+
i0.ɵɵforeignComponent(1, 0, { label: ctx.title, items: i0.ɵɵforeignContentFn(0) });
8992
}
9093
},
9194
encapsulation: 2

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

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -380,8 +380,9 @@ function ingestElement(unit: ViewCompilationUnit, element: t.Element): void {
380380
// Foreign components are created in the creation block. Updates are triggered reactively
381381
// through directly passed signal properties, alleviating the need for any explicit update
382382
// operations.
383+
const constIndex = unit.job.addConst(foreignComp.component);
383384
unit.create.push(
384-
ir.createForeignComponentOp(id, foreignComp.component, props, element.startSourceSpan),
385+
ir.createForeignComponentOp(id, o.literal(constIndex), props, element.startSourceSpan),
385386
);
386387
return;
387388
}

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -25,23 +25,25 @@ import {createAndRenderEmbeddedLView} from '../view_manipulation';
2525
import {collectNativeNodes} from '../collect_native_nodes';
2626
import {assertLContainer} from '../assert';
2727
import {CONTAINER_HEADER_OFFSET, LContainer, LContainerFlags} from '../interfaces/container';
28+
import {getConstant} from '../util/view_utils';
2829

2930
/**
3031
* Creation phase instruction to render a foreign component.
3132
*
3233
* @param index The index of the container in the data array.
33-
* @param foreignComponent The matched foreign component.
34+
* @param foreignComponentIndex The index of the matched foreign component in the constant pool.
3435
* @param props Aggregate properties and static attributes.
3536
* @codeGenApi
3637
*/
3738
export function ɵɵforeignComponent(
3839
index: number,
39-
foreignComponent: ForeignComponent<any>,
40+
foreignComponentIndex: number,
4041
props?: any,
4142
): void {
4243
const lView = getLView();
4344
const tView = getTView();
4445
const adjustedIndex = index + HEADER_OFFSET;
46+
const foreignComponent = getConstant<ForeignComponent<any>>(tView.consts, foreignComponentIndex)!;
4547

4648
// 1. Get or create TNode for this container slot
4749
let tNode: TContainerNode;

‎packages/core/src/render3/interfaces/node.ts‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import {Type} from '../../interface/type';
99
import {KeyValueArray} from '../../util/array_utils';
1010
import {TStylingRange} from '../interfaces/styling';
1111
import {AttributeMarker} from './attribute_marker';
12+
import {ForeignComponent} from '../../interface/foreign_component';
1213

1314
import {TIcu} from './i18n';
1415
import {CssSelector} from './projection';
@@ -226,8 +227,9 @@ export type TAttributes = (string | AttributeMarker | CssSelector)[];
226227
* - Attribute arrays.
227228
* - Local definition arrays.
228229
* - Translated messages (i18n).
230+
* - Foreign components.
229231
*/
230-
export type TConstants = (TAttributes | string)[];
232+
export type TConstants = (TAttributes | string | ForeignComponent<any>)[];
231233

232234
/**
233235
* Factory function that returns an array of consts. Consts can be represented as a function in

‎packages/core/test/acceptance/foreign_component/foreign_component_spec.ts‎

Lines changed: 25 additions & 23 deletions
Original file line numberDiff line numberDiff line change
@@ -23,29 +23,6 @@ function FancyButton(props: {children: Node[]}): Node[] {
2323
return [button];
2424
}
2525

26-
// TODO: support this inline.
27-
function InnerComp(props: {renderHeader: (innerMsg: string) => Node[]; children: Node[]}): Node[] {
28-
const inner = document.createElement('div');
29-
inner.className = 'inner';
30-
31-
const headerDiv = document.createElement('div');
32-
headerDiv.className = 'inner-header';
33-
const headerNodes = props.renderHeader('Inner Msg');
34-
for (const child of headerNodes) {
35-
headerDiv.appendChild(child);
36-
}
37-
inner.appendChild(headerDiv);
38-
39-
const bodyDiv = document.createElement('div');
40-
bodyDiv.className = 'inner-body';
41-
for (const child of props.children) {
42-
bodyDiv.appendChild(child);
43-
}
44-
inner.appendChild(bodyDiv);
45-
46-
return [inner];
47-
}
48-
4926
describe('foreign components', () => {
5027
describe('content projection', () => {
5128
it('should update foreign content', async () => {
@@ -483,6 +460,31 @@ describe('foreign components', () => {
483460
return [outer];
484461
}
485462

463+
function InnerComp(props: {
464+
renderHeader: (innerMsg: string) => Node[];
465+
children: Node[];
466+
}): Node[] {
467+
const inner = document.createElement('div');
468+
inner.className = 'inner';
469+
470+
const headerDiv = document.createElement('div');
471+
headerDiv.className = 'inner-header';
472+
const headerNodes = props.renderHeader('Inner Msg');
473+
for (const child of headerNodes) {
474+
headerDiv.appendChild(child);
475+
}
476+
inner.appendChild(headerDiv);
477+
478+
const bodyDiv = document.createElement('div');
479+
bodyDiv.className = 'inner-body';
480+
for (const child of props.children) {
481+
bodyDiv.appendChild(child);
482+
}
483+
inner.appendChild(bodyDiv);
484+
485+
return [inner];
486+
}
487+
486488
@Component({
487489
selector: 'test-cmp',
488490
template: `

‎packages/core/test/render3/foreign_component_spec.ts‎

Lines changed: 18 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -40,8 +40,9 @@ describe('ɵɵforeignComponent', () => {
4040
const fixture = new ViewFixture({
4141
decls: 1,
4242
vars: 0,
43+
consts: [foreignComp],
4344
create: () => {
44-
ɵɵforeignComponent(0, foreignComp);
45+
ɵɵforeignComponent(0, 0);
4546
},
4647
});
4748

@@ -58,8 +59,9 @@ describe('ɵɵforeignComponent', () => {
5859
new ViewFixture({
5960
decls: 1,
6061
vars: 0,
62+
consts: [foreignComp],
6163
create: () => {
62-
ɵɵforeignComponent(0, foreignComp, {name: 'Angular'});
64+
ɵɵforeignComponent(0, 0, {name: 'Angular'});
6365
},
6466
});
6567

@@ -80,8 +82,9 @@ describe('ɵɵforeignComponent', () => {
8082
const fixture = new ViewFixture({
8183
decls: 1,
8284
vars: 0,
85+
consts: [foreignComp],
8386
create: () => {
84-
ɵɵforeignComponent(0, foreignComp);
87+
ɵɵforeignComponent(0, 0);
8588
},
8689
});
8790

@@ -102,9 +105,10 @@ describe('ɵɵforeignComponent', () => {
102105
const fixture = new ViewFixture({
103106
decls: 3,
104107
vars: 0,
108+
consts: [foreignComp],
105109
create: () => {
106110
ɵɵelement(0, 'p');
107-
ɵɵforeignComponent(1, foreignComp);
111+
ɵɵforeignComponent(1, 0);
108112
ɵɵelement(2, 'span');
109113
},
110114
});
@@ -130,9 +134,10 @@ describe('ɵɵforeignComponent', () => {
130134
const fixture = new ViewFixture({
131135
decls: 2,
132136
vars: 0,
137+
consts: [foreignComp],
133138
create: () => {
134139
ɵɵelementStart(0, 'div');
135-
ɵɵforeignComponent(1, foreignComp);
140+
ɵɵforeignComponent(1, 0);
136141
ɵɵelementEnd();
137142
},
138143
});
@@ -171,11 +176,11 @@ describe('ɵɵforeignComponent', () => {
171176
const fixture = new ViewFixture({
172177
decls: 2,
173178
vars: 0,
174-
consts: [['provider-dir', '']],
179+
consts: [['provider-dir', ''], foreignComp],
175180
directives: [ProviderDirective],
176181
create: () => {
177182
ɵɵelementStart(0, 'div', 0);
178-
ɵɵforeignComponent(1, foreignComp);
183+
ɵɵforeignComponent(1, 1);
179184
ɵɵelementEnd();
180185
},
181186
});
@@ -190,7 +195,7 @@ describe('ɵɵforeignComponent', () => {
190195

191196
const createFn = () => {
192197
ɵɵelementStart(0, 'div');
193-
ɵɵforeignComponent(1, foreignComp1);
198+
ɵɵforeignComponent(1, 0);
194199
ɵɵelementEnd();
195200
};
196201
const expectedHtml =
@@ -203,6 +208,7 @@ describe('ɵɵforeignComponent', () => {
203208
const fixture = new ViewFixture({
204209
decls: 2,
205210
vars: 0,
211+
consts: [foreignComp1],
206212
create: createFn,
207213
});
208214
expect(fixture.host.innerHTML).toContain(expectedHtml);
@@ -273,11 +279,12 @@ describe('ɵɵforeignComponent', () => {
273279
const fixture = new ViewFixture({
274280
decls: 4,
275281
vars: 0,
282+
consts: [foreignComp],
276283
create: () => {
277284
ɵɵdomTemplate(0, iconTemplate, 2, 0);
278285
ɵɵdomTemplate(1, descriptionTemplate, 2, 0);
279286
ɵɵdomTemplate(2, childrenTemplate, 2, 0);
280-
ɵɵforeignComponent(3, foreignComp, {
287+
ɵɵforeignComponent(3, 0, {
281288
icon: ɵɵforeignContent(0),
282289
description: ɵɵforeignContent(1),
283290
children: ɵɵforeignContent(2),
@@ -332,9 +339,10 @@ describe('ɵɵforeignComponent', () => {
332339
const fixture = new ViewFixture({
333340
decls: 2,
334341
vars: 0,
342+
consts: [foreignComp],
335343
create: () => {
336344
ɵɵdomTemplate(0, itemTemplate, 2, 2);
337-
ɵɵforeignComponent(1, foreignComp, {
345+
ɵɵforeignComponent(1, 0, {
338346
renderItem: ɵɵforeignContentFn(0),
339347
});
340348
},

0 commit comments

Comments
 (0)