Skip to content

Commit aca339e

Browse files
devversionmhevery
authored andcommitted
fix(ivy): unable to project into multiple slots with default selector (angular#30561)
With View engine it was possible to declare multiple projection definitions and to programmatically project nodes into the slots. e.g. ```html <ng-content></ng-content> <ng-content></ng-content> ``` Using `ViewContainerRef#createComponent` allowed projecting nodes into one of the projection defs (through index) This no longer works with Ivy as the `projectionDef` instruction only retrieves a list of selectors instead of also retrieving entries for reserved projection slots which appear when using the default selector multiple times (as seen above). In order to fix this issue, the Ivy compiler now passes all projection slots to the `projectionDef` instruction. Meaning that there can be multiple projection slots with the same wildcard selector. This allows multi-slot projection as seen in the example above, and it also allows us to match the multi-slot node projection order from View Engine (to avoid breaking changes). It basically ensures that Ivy fully matches the View Engine behavior except of a very small edge case that has already been discussed in FW-886 (with the conclusion of working as intended). Read more here: https://hackmd.io/s/Sy2kQlgTE PR Close angular#30561
1 parent f4cd374 commit aca339e

File tree

13 files changed

+219
-131
lines changed

13 files changed

+219
-131
lines changed

packages/compiler-cli/test/compliance/r3_compiler_compliance_spec.ts

Lines changed: 62 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1159,7 +1159,7 @@ describe('compiler compliance', () => {
11591159
type: SimpleComponent,
11601160
selectors: [["simple"]],
11611161
factory: function SimpleComponent_Factory(t) { return new (t || SimpleComponent)(); },
1162-
ngContentSelectors: _c0,
1162+
ngContentSelectors: $c0$,
11631163
consts: 2,
11641164
vars: 0,
11651165
template: function SimpleComponent_Template(rf, ctx) {
@@ -1189,10 +1189,10 @@ describe('compiler compliance', () => {
11891189
if (rf & 1) {
11901190
$r3$.ɵɵprojectionDef($c1$);
11911191
$r3$.ɵɵelementStart(0, "div", $c3$);
1192-
$r3$.ɵɵprojection(1, 1);
1192+
$r3$.ɵɵprojection(1);
11931193
$r3$.ɵɵelementEnd();
11941194
$r3$.ɵɵelementStart(2, "div", $c4$);
1195-
$r3$.ɵɵprojection(3, 2);
1195+
$r3$.ɵɵprojection(3, 1);
11961196
$r3$.ɵɵelementEnd();
11971197
}
11981198
},
@@ -1209,6 +1209,54 @@ describe('compiler compliance', () => {
12091209
result.source, ComplexComponentDefinition, 'Incorrect ComplexComponent definition');
12101210
});
12111211

1212+
it('should support multi-slot content projection with multiple wildcard slots', () => {
1213+
const files = {
1214+
app: {
1215+
'spec.ts': `
1216+
import {Component, NgModule} from '@angular/core';
1217+
1218+
@Component({
1219+
template: \`
1220+
<ng-content></ng-content>
1221+
<ng-content select="[spacer]"></ng-content>
1222+
<ng-content></ng-content>
1223+
\`,
1224+
})
1225+
class Cmp {}
1226+
1227+
@NgModule({ declarations: [Cmp] })
1228+
class Module {}
1229+
`,
1230+
}
1231+
};
1232+
1233+
const output = `
1234+
const $c0$ = ["*", [["", "spacer", ""]], "*"];
1235+
const $c1$ = ["*", "[spacer]", "*"];
1236+
1237+
Cmp.ngComponentDef = $r3$.ɵɵdefineComponent({
1238+
type: Cmp,
1239+
selectors: [["ng-component"]],
1240+
factory: function Cmp_Factory(t) { return new (t || Cmp)(); },
1241+
ngContentSelectors: $c1$,
1242+
consts: 3,
1243+
vars: 0,
1244+
template: function Cmp_Template(rf, ctx) {
1245+
if (rf & 1) {
1246+
i0.ɵɵprojectionDef($c0$);
1247+
i0.ɵɵprojection(0);
1248+
i0.ɵɵprojection(1, 1);
1249+
i0.ɵɵprojection(2, 2);
1250+
}
1251+
},
1252+
encapsulation: 2
1253+
});
1254+
`;
1255+
1256+
const {source} = compile(files, angularFiles);
1257+
expectEmit(source, output, 'Invalid content projection instructions generated');
1258+
});
1259+
12121260
it('should support content projection in nested templates', () => {
12131261
const files = {
12141262
app: {
@@ -1241,7 +1289,7 @@ describe('compiler compliance', () => {
12411289
const $_c2$ = ["id", "second"];
12421290
function Cmp_div_0_Template(rf, ctx) { if (rf & 1) {
12431291
$r3$.ɵɵelementStart(0, "div", $_c2$);
1244-
$r3$.ɵɵprojection(1, 1);
1292+
$r3$.ɵɵprojection(1);
12451293
$r3$.ɵɵelementEnd();
12461294
} }
12471295
const $_c3$ = ["id", "third"];
@@ -1255,10 +1303,10 @@ describe('compiler compliance', () => {
12551303
function Cmp_ng_template_2_Template(rf, ctx) {
12561304
if (rf & 1) {
12571305
$r3$.ɵɵtext(0, " '*' selector: ");
1258-
$r3$.ɵɵprojection(1);
1306+
$r3$.ɵɵprojection(1, 1);
12591307
}
12601308
}
1261-
const $_c4$ = [[["span", "title", "tofirst"]]];
1309+
const $_c4$ = [[["span", "title", "tofirst"]], "*"];
12621310
12631311
template: function Cmp_Template(rf, ctx) {
12641312
if (rf & 1) {
@@ -1312,31 +1360,31 @@ describe('compiler compliance', () => {
13121360
const output = `
13131361
function Cmp_ng_template_1_ng_template_1_Template(rf, ctx) {
13141362
if (rf & 1) {
1315-
$r3$.ɵɵprojection(0, 4);
1363+
$r3$.ɵɵprojection(0, 3);
13161364
}
13171365
}
13181366
function Cmp_ng_template_1_Template(rf, ctx) {
13191367
if (rf & 1) {
1320-
$r3$.ɵɵprojection(0, 3);
1368+
$r3$.ɵɵprojection(0, 2);
13211369
$r3$.ɵɵtemplate(1, Cmp_ng_template_1_ng_template_1_Template, 1, 0, "ng-template");
13221370
}
13231371
}
13241372
function Cmp_ng_template_2_Template(rf, ctx) {
13251373
if (rf & 1) {
13261374
$r3$.ɵɵtext(0, " '*' selector in a template: ");
1327-
$r3$.ɵɵprojection(1);
1375+
$r3$.ɵɵprojection(1, 4);
13281376
}
13291377
}
1330-
const $_c0$ = [[["", "id", "tomainbefore"]], [["", "id", "tomainafter"]], [["", "id", "totemplate"]], [["", "id", "tonestedtemplate"]]];
1331-
const $_c1$ = ["[id=toMainBefore]", "[id=toMainAfter]", "[id=toTemplate]", "[id=toNestedTemplate]"];
1378+
const $_c0$ = [[["", "id", "tomainbefore"]], [["", "id", "tomainafter"]], [["", "id", "totemplate"]], [["", "id", "tonestedtemplate"]], "*"];
1379+
const $_c1$ = ["[id=toMainBefore]", "[id=toMainAfter]", "[id=toTemplate]", "[id=toNestedTemplate]", "*"];
13321380
13331381
template: function Cmp_Template(rf, ctx) {
13341382
if (rf & 1) {
1335-
$r3$.ɵɵprojectionDef($_c2$);
1336-
$r3$.ɵɵprojection(0, 1);
1383+
$r3$.ɵɵprojectionDef($_c0$);
1384+
$r3$.ɵɵprojection(0);
13371385
$r3$.ɵɵtemplate(1, Cmp_ng_template_1_Template, 2, 0, "ng-template");
13381386
$r3$.ɵɵtemplate(2, Cmp_ng_template_2_Template, 2, 0, "ng-template");
1339-
$r3$.ɵɵprojection(3, 2);
1387+
$r3$.ɵɵprojection(3, 1);
13401388
}
13411389
}
13421390
`;

packages/compiler-cli/test/ngtsc/template_mapping_spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,15 +332,15 @@ describe('template source-mapping', () => {
332332
{source: '<h3>', generated: 'i0.ɵɵelementStart(0, "h3")', sourceUrl: '../test.ts'});
333333
expect(mappings).toContain({
334334
source: '<ng-content select="title">',
335-
generated: 'i0.ɵɵprojection(1, 1)',
335+
generated: 'i0.ɵɵprojection(1)',
336336
sourceUrl: '../test.ts'
337337
});
338338
expect(mappings).toContain(
339339
{source: '</h3>', generated: 'i0.ɵɵelementEnd()', sourceUrl: '../test.ts'});
340340
expect(mappings).toContain(
341341
{source: '<div>', generated: 'i0.ɵɵelementStart(2, "div")', sourceUrl: '../test.ts'});
342342
expect(mappings).toContain(
343-
{source: '<ng-content>', generated: 'i0.ɵɵprojection(3)', sourceUrl: '../test.ts'});
343+
{source: '<ng-content>', generated: 'i0.ɵɵprojection(3, 1)', sourceUrl: '../test.ts'});
344344
expect(mappings).toContain(
345345
{source: '</div>', generated: 'i0.ɵɵelementEnd()', sourceUrl: '../test.ts'});
346346
});

packages/compiler/src/render3/view/template.ts

Lines changed: 27 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,6 @@ import {Instruction, StylingBuilder} from './styling_builder';
4040
import {CONTEXT_NAME, IMPLICIT_REFERENCE, NON_BINDABLE_ATTR, REFERENCE_PREFIX, RENDER_FLAGS, asLiteral, getAttrsForDirectiveMatching, invalid, trimTrailingNulls, unsupported} from './util';
4141

4242

43-
// Default selector used by `<ng-content>` if none specified
44-
const DEFAULT_NG_CONTENT_SELECTOR = '*';
45-
4643
// Selector attribute name of `<ng-content>`
4744
const NG_CONTENT_SELECT_ATTR = 'select';
4845

@@ -146,14 +143,13 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
146143

147144
private fileBasedI18nSuffix: string;
148145

149-
// Whether the template includes <ng-content> tags.
150-
private _hasNgContent: boolean = false;
151-
152-
// Selectors found in the <ng-content> tags in the template.
153-
private _ngContentSelectors: string[] = [];
146+
// Projection slots found in the template. Projection slots can distribute projected
147+
// nodes based on a selector, or can just use the wildcard selector to match
148+
// all nodes which aren't matching any selector.
149+
private _ngContentReservedSlots: (string|'*')[] = [];
154150

155151
// Number of non-default selectors found in all parent templates of this template. We need to
156-
// track it to properly adjust projection bucket index in the `projection` instruction.
152+
// track it to properly adjust projection slot index in the `projection` instruction.
157153
private _ngContentSelectorsOffset = 0;
158154

159155
constructor(
@@ -247,16 +243,19 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
247243
// instructions can be generated with the correct internal const count.
248244
this._nestedTemplateFns.forEach(buildTemplateFn => buildTemplateFn());
249245

250-
// Output the `projectionDef` instruction when some `<ng-content>` are present.
251-
// The `projectionDef` instruction only emitted for the component template and it is skipped for
252-
// nested templates (<ng-template> tags).
253-
if (this.level === 0 && this._hasNgContent) {
246+
// Output the `projectionDef` instruction when some `<ng-content>` tags are present.
247+
// The `projectionDef` instruction is only emitted for the component template and
248+
// is skipped for nested templates (<ng-template> tags).
249+
if (this.level === 0 && this._ngContentReservedSlots.length) {
254250
const parameters: o.Expression[] = [];
255251

256-
// Only selectors with a non-default value are generated
257-
if (this._ngContentSelectors.length) {
258-
const r3Selectors = this._ngContentSelectors.map(s => core.parseSelectorToR3Selector(s));
259-
parameters.push(this.constantPool.getConstLiteral(asLiteral(r3Selectors), true));
252+
// By default the `projectionDef` instructions creates one slot for the wildcard
253+
// selector if no parameters are passed. Therefore we only want to allocate a new
254+
// array for the projection slots if the default projection slot is not sufficient.
255+
if (this._ngContentReservedSlots.length > 1 || this._ngContentReservedSlots[0] !== '*') {
256+
const r3ReservedSlots = this._ngContentReservedSlots.map(
257+
s => s !== '*' ? core.parseSelectorToR3Selector(s) : s);
258+
parameters.push(this.constantPool.getConstLiteral(asLiteral(r3ReservedSlots), true));
260259
}
261260

262261
// Since we accumulate ngContent selectors while processing template elements,
@@ -461,14 +460,13 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
461460
}
462461

463462
visitContent(ngContent: t.Content) {
464-
this._hasNgContent = true;
465463
const slot = this.allocateDataSlot();
466-
let selectorIndex = ngContent.selector === DEFAULT_NG_CONTENT_SELECTOR ?
467-
0 :
468-
this._ngContentSelectors.push(ngContent.selector) + this._ngContentSelectorsOffset;
464+
const projectionSlotIdx = this._ngContentSelectorsOffset + this._ngContentReservedSlots.length;
469465
const parameters: o.Expression[] = [o.literal(slot)];
470466
const attributes: o.Expression[] = [];
471467

468+
this._ngContentReservedSlots.push(ngContent.selector);
469+
472470
ngContent.attributes.forEach((attribute) => {
473471
const {name, value} = attribute;
474472
if (name === NG_PROJECT_AS_ATTR_NAME) {
@@ -479,9 +477,9 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
479477
});
480478

481479
if (attributes.length > 0) {
482-
parameters.push(o.literal(selectorIndex), o.literalArr(attributes));
483-
} else if (selectorIndex !== 0) {
484-
parameters.push(o.literal(selectorIndex));
480+
parameters.push(o.literal(projectionSlotIdx), o.literalArr(attributes));
481+
} else if (projectionSlotIdx !== 0) {
482+
parameters.push(o.literal(projectionSlotIdx));
485483
}
486484

487485
this.creationInstruction(ngContent.sourceSpan, R3.projection, parameters);
@@ -887,11 +885,10 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
887885
this._nestedTemplateFns.push(() => {
888886
const templateFunctionExpr = templateVisitor.buildTemplateFunction(
889887
template.children, template.variables,
890-
this._ngContentSelectors.length + this._ngContentSelectorsOffset, template.i18n);
888+
this._ngContentReservedSlots.length + this._ngContentSelectorsOffset, template.i18n);
891889
this.constantPool.statements.push(templateFunctionExpr.toDeclStmt(templateName, null));
892-
if (templateVisitor._hasNgContent) {
893-
this._hasNgContent = true;
894-
this._ngContentSelectors.push(...templateVisitor._ngContentSelectors);
890+
if (templateVisitor._ngContentReservedSlots.length) {
891+
this._ngContentReservedSlots.push(...templateVisitor._ngContentReservedSlots);
895892
}
896893
});
897894

@@ -1011,8 +1008,8 @@ export class TemplateDefinitionBuilder implements t.Visitor<void>, LocalResolver
10111008
getVarCount() { return this._pureFunctionSlots; }
10121009

10131010
getNgContentSelectors(): o.Expression|null {
1014-
return this._hasNgContent ?
1015-
this.constantPool.getConstLiteral(asLiteral(this._ngContentSelectors), true) :
1011+
return this._ngContentReservedSlots.length ?
1012+
this.constantPool.getConstLiteral(asLiteral(this._ngContentReservedSlots), true) :
10161013
null;
10171014
}
10181015

packages/core/src/render3/component_ref.ts

Lines changed: 1 addition & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -123,10 +123,8 @@ export class ComponentFactory<T> extends viewEngine_ComponentFactory<T> {
123123
super();
124124
this.componentType = componentDef.type;
125125
this.selector = componentDef.selectors[0][0] as string;
126-
// The component definition does not include the wildcard ('*') selector in its list.
127-
// It is implicitly expected as the first item in the projectable nodes array.
128126
this.ngContentSelectors =
129-
componentDef.ngContentSelectors ? ['*', ...componentDef.ngContentSelectors] : [];
127+
componentDef.ngContentSelectors ? componentDef.ngContentSelectors : [];
130128
this.isBoundToModule = !!ngModule;
131129
}
132130

packages/core/src/render3/index.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -121,7 +121,7 @@ export {
121121
ɵɵtextInterpolateV,
122122
} from './instructions/all';
123123
export {RenderFlags} from './interfaces/definition';
124-
export {CssSelectorList} from './interfaces/projection';
124+
export {CssSelectorList, ProjectionSlots} from './interfaces/projection';
125125

126126
export {
127127
ɵɵrestoreView,

packages/core/src/render3/instructions/projection.ts

Lines changed: 51 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -5,18 +5,47 @@
55
* Use of this source code is governed by an MIT-style license that can be
66
* found in the LICENSE file at https://angular.io/license
77
*/
8-
import {assertDataInRange} from '../../util/assert';
98
import {TAttributes, TElementNode, TNode, TNodeType} from '../interfaces/node';
10-
import {CssSelectorList} from '../interfaces/projection';
11-
import {HEADER_OFFSET, TVIEW, T_HOST} from '../interfaces/view';
9+
import {ProjectionSlots} from '../interfaces/projection';
10+
import {TVIEW, T_HOST} from '../interfaces/view';
1211
import {appendProjectedNodes} from '../node_manipulation';
13-
import {matchingProjectionSelectorIndex} from '../node_selector_matcher';
12+
import {getProjectAsAttrValue, isNodeMatchingSelectorList, isSelectorInSelectorList} from '../node_selector_matcher';
1413
import {getLView, setIsNotParent} from '../state';
1514
import {findComponentView} from '../util/view_traversal_utils';
1615

1716
import {getOrCreateTNode} from './shared';
1817

1918

19+
/**
20+
* Checks a given node against matching projection slots and returns the
21+
* determined slot index. Returns "null" if no slot matched the given node.
22+
*
23+
* This function takes into account the parsed ngProjectAs selector from the
24+
* node's attributes. If present, it will check whether the ngProjectAs selector
25+
* matches any of the projection slot selectors.
26+
*/
27+
export function matchingProjectionSlotIndex(tNode: TNode, projectionSlots: ProjectionSlots): number|
28+
null {
29+
let wildcardNgContentIndex = null;
30+
const ngProjectAsAttrVal = getProjectAsAttrValue(tNode);
31+
for (let i = 0; i < projectionSlots.length; i++) {
32+
const slotValue = projectionSlots[i];
33+
// The last wildcard projection slot should match all nodes which aren't matching
34+
// any selector. This is necessary to be backwards compatible with view engine.
35+
if (slotValue === '*') {
36+
wildcardNgContentIndex = i;
37+
continue;
38+
}
39+
// If we ran into an `ngProjectAs` attribute, we should match its parsed selector
40+
// to the list of selectors, otherwise we fall back to matching against the node.
41+
if (ngProjectAsAttrVal === null ?
42+
isNodeMatchingSelectorList(tNode, slotValue, /* isProjectionMode */ true) :
43+
isSelectorInSelectorList(ngProjectAsAttrVal, slotValue)) {
44+
return i; // first matching selector "captures" a given node
45+
}
46+
}
47+
return wildcardNgContentIndex;
48+
}
2049

2150
/**
2251
* Instruction to distribute projectable nodes among <ng-content> occurrences in a given template.
@@ -36,32 +65,38 @@ import {getOrCreateTNode} from './shared';
3665
* - we can't have only a parsed as we can't re-construct textual form from it (as entered by a
3766
* template author).
3867
*
39-
* @param selectors A collection of parsed CSS selectors
40-
* @param rawSelectors A collection of CSS selectors in the raw, un-parsed form
68+
* @param projectionSlots? A collection of projection slots. A projection slot can be based
69+
* on a parsed CSS selectors or set to the wildcard selector ("*") in order to match
70+
* all nodes which do not match any selector. If not specified, a single wildcard
71+
* selector projection slot will be defined.
4172
*
4273
* @codeGenApi
4374
*/
44-
export function ɵɵprojectionDef(selectors?: CssSelectorList[]): void {
75+
export function ɵɵprojectionDef(projectionSlots?: ProjectionSlots): void {
4576
const componentNode = findComponentView(getLView())[T_HOST] as TElementNode;
4677

4778
if (!componentNode.projection) {
48-
const noOfNodeBuckets = selectors ? selectors.length + 1 : 1;
79+
// If no explicit projection slots are defined, fall back to a single
80+
// projection slot with the wildcard selector.
81+
const numProjectionSlots = projectionSlots ? projectionSlots.length : 1;
4982
const projectionHeads: (TNode | null)[] = componentNode.projection =
50-
new Array(noOfNodeBuckets).fill(null);
83+
new Array(numProjectionSlots).fill(null);
5184
const tails: (TNode | null)[] = projectionHeads.slice();
5285

5386
let componentChild: TNode|null = componentNode.child;
5487

5588
while (componentChild !== null) {
56-
const bucketIndex =
57-
selectors ? matchingProjectionSelectorIndex(componentChild, selectors) : 0;
89+
const slotIndex =
90+
projectionSlots ? matchingProjectionSlotIndex(componentChild, projectionSlots) : 0;
5891

59-
if (tails[bucketIndex]) {
60-
tails[bucketIndex] !.projectionNext = componentChild;
61-
} else {
62-
projectionHeads[bucketIndex] = componentChild;
92+
if (slotIndex !== null) {
93+
if (tails[slotIndex]) {
94+
tails[slotIndex] !.projectionNext = componentChild;
95+
} else {
96+
projectionHeads[slotIndex] = componentChild;
97+
}
98+
tails[slotIndex] = componentChild;
6399
}
64-
tails[bucketIndex] = componentChild;
65100

66101
componentChild = componentChild.next;
67102
}

0 commit comments

Comments
 (0)