Skip to content

Commit

Permalink
fix(compiler): remove container index from conditional instruction
Browse files Browse the repository at this point in the history 
Stops passing in the `containerIndex` argument to the `conditional` instruction since it isn't being used anymore.
  • Loading branch information
@crisbeto
crisbeto committed Apr 3, 2024
1 parent f577319 commit 5fc8aa2
Show file tree
Hide file tree
Showing 21 changed files with 42 additions and 55 deletions.
8 changes: 6 additions & 2 deletions adev/BUILD.bazel
View file
Expand Up @@ -35,9 +35,10 @@ APPLICATION_DEPS = link_packages([
"@npm//@angular/animations",
"@npm//@angular/cdk",
"@npm//@angular/common",
"@npm//@angular/compiler",
# TODO: temporarily disabled to land #55190.
# "@npm//@angular/compiler",
# "@npm//@angular/core",
"@npm//@angular/compiler-cli",
"@npm//@angular/core",
"@npm//@angular/docs",
"@npm//@angular/forms",
"@npm//@angular/material",
Expand Down Expand Up @@ -79,6 +80,9 @@ APPLICATION_DEPS = link_packages([
])

TEST_DEPS = APPLICATION_DEPS + link_packages([
# TODO: temporarily moved here to land #55190.
"@npm//@angular/compiler",
"@npm//@angular/core",
"@npm//@angular/platform-browser-dynamic",
"@npm//@angular/build-tooling/bazel/browsers/chromium",
"@npm//@types/jasmine",
Expand Down
2 changes: 1 addition & 1 deletion ...3_compiler_compliance/components_and_directives/content_projection/ng_content_fallback.js
View file
Expand Up @@ -69,7 +69,7 @@ $r3$.ɵɵdefineComponent({
}
if (rf & 2) {
$r3$.ɵɵadvance(5);
$r3$.ɵɵconditional(5, ctx.hasFooter ? 5 : -1);
$r3$.ɵɵconditional(ctx.hasFooter ? 5 : -1);
$r3$.ɵɵadvance();
$r3$.ɵɵproperty("ngIf", ctx.hasStructural);
}
Expand Down
2 changes: 1 addition & 1 deletion ...cli/test/compliance/test_cases/r3_view_compiler_control_flow/basic_if_else_if_template.js
View file
Expand Up @@ -33,6 +33,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ctx.value() === 1 ? 2 : ctx.otherValue() === 2 ? 3 : ctx.message ? 4 : 5);
$r3$.ɵɵconditional(ctx.value() === 1 ? 2 : ctx.otherValue() === 2 ? 3 : ctx.message ? 4 : 5);
}
}
2 changes: 1 addition & 1 deletion ...er-cli/test/compliance/test_cases/r3_view_compiler_control_flow/basic_if_else_template.js
View file
Expand Up @@ -21,6 +21,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ctx.value() ? 2 : 3);
$r3$.ɵɵconditional(ctx.value() ? 2 : 3);
}
}
2 changes: 1 addition & 1 deletion ...ompiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/basic_if_template.js
View file
Expand Up @@ -15,6 +15,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ctx.value() ? 2 : -1);
$r3$.ɵɵconditional(ctx.value() ? 2 : -1);
}
}
2 changes: 1 addition & 1 deletion ...ler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/basic_switch_template.js
View file
Expand Up @@ -34,6 +34,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ($MyApp_contFlowTmp$ = ctx.value()) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : $MyApp_contFlowTmp$ === 2 ? 4 : 5);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = ctx.value()) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : $MyApp_contFlowTmp$ === 2 ? 4 : 5);
}
}
6 changes: 3 additions & 3 deletions ...e/test_cases/r3_view_compiler_control_flow/if_nested_alias_listeners_template.pipeline.js
View file
Expand Up @@ -30,7 +30,7 @@ function MyApp_Conditional_0_Conditional_1_Conditional_1_Template(rf, ctx) {
let $MyApp_Conditional_0_Conditional_1_contFlowTmp$;
const $ctx_r2$ = $r3$.ɵɵnextContext(2);
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(1, ($MyApp_Conditional_0_Conditional_1_contFlowTmp$ = $ctx_r2$.value()) ? 1 : -1, $MyApp_Conditional_0_Conditional_1_contFlowTmp$);
$r3$.ɵɵconditional(($MyApp_Conditional_0_Conditional_1_contFlowTmp$ = $ctx_r2$.value()) ? 1 : -1, $MyApp_Conditional_0_Conditional_1_contFlowTmp$);
}
}

Expand All @@ -50,7 +50,7 @@ function MyApp_Conditional_0_Conditional_1_Conditional_1_Template(rf, ctx) {
let $MyApp_Conditional_0_contFlowTmp$;
const $ctx_r0$ = $r3$.ɵɵnextContext();
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(1, ($MyApp_Conditional_0_contFlowTmp$ = $ctx_r0$.value()) ? 1 : -1, $MyApp_Conditional_0_contFlowTmp$);
$r3$.ɵɵconditional(($MyApp_Conditional_0_contFlowTmp$ = $ctx_r0$.value()) ? 1 : -1, $MyApp_Conditional_0_contFlowTmp$);
}
}
…
Expand All @@ -60,6 +60,6 @@ function MyApp_Conditional_0_Conditional_1_Conditional_1_Template(rf, ctx) {
}
if (rf & 2) {
let $MyApp_contFlowTmp$;
$r3$.ɵɵconditional(0, ($MyApp_contFlowTmp$ = ctx.value()) ? 0 : -1, $MyApp_contFlowTmp$);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = ctx.value()) ? 0 : -1, $MyApp_contFlowTmp$);
}
}
6 changes: 3 additions & 3 deletions ...-cli/test/compliance/test_cases/r3_view_compiler_control_flow/if_nested_alias_template.js
View file
Expand Up @@ -25,7 +25,7 @@ function MyApp_Conditional_0_Conditional_1_Template(rf, ctx) {
…
$r3$.ɵɵtextInterpolate3(" Inner: ", $ctx_r2$.value(), "/", $root_r1$, "/", ctx, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(1, ($MyApp_Conditional_0_Conditional_1_contFlowTmp$ = $ctx_r2$.value()) ? 1 : -1, $MyApp_Conditional_0_Conditional_1_contFlowTmp$);
$r3$.ɵɵconditional(($MyApp_Conditional_0_Conditional_1_contFlowTmp$ = $ctx_r2$.value()) ? 1 : -1, $MyApp_Conditional_0_Conditional_1_contFlowTmp$);
}
}

Expand All @@ -40,7 +40,7 @@ function MyApp_Conditional_0_Template(rf, ctx) {
…
$r3$.ɵɵtextInterpolate2(" Root: ", $ctx_r0$.value(), "/", ctx, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(1, ($MyApp_Conditional_0_contFlowTmp$ = $ctx_r0$.value()) ? 1 : -1, $MyApp_Conditional_0_contFlowTmp$);
$r3$.ɵɵconditional(($MyApp_Conditional_0_contFlowTmp$ = $ctx_r0$.value()) ? 1 : -1, $MyApp_Conditional_0_contFlowTmp$);
}
}
…
Expand All @@ -50,6 +50,6 @@ function MyApp_Template(rf, ctx) {
}
if (rf & 2) {
let $MyApp_contFlowTmp$;
$r3$.ɵɵconditional(0, ($MyApp_contFlowTmp$ = ctx.value()) ? 0 : -1, $MyApp_contFlowTmp$);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = ctx.value()) ? 0 : -1, $MyApp_contFlowTmp$);
}
}
2 changes: 1 addition & 1 deletion ...er-cli/test/compliance/test_cases/r3_view_compiler_control_flow/if_with_alias_template.js
View file
Expand Up @@ -20,6 +20,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ($MyApp_contFlowTmp$ = ctx.value()) ? 2 : -1, $MyApp_contFlowTmp$);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = ctx.value()) ? 2 : -1, $MyApp_contFlowTmp$);
}
}
2 changes: 1 addition & 1 deletion ...est/compliance/test_cases/r3_view_compiler_control_flow/if_with_pipe_template.pipeline.js
View file
Expand Up @@ -30,6 +30,6 @@ function $MyApp_Conditional_3_Template$(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, $r3$.ɵɵpipeBind1(3, 2, ctx.val) === 1 ? 2 : $r3$.ɵɵpipeBind1(4, 4, ctx.val) === 2 ? 5 : 6);
$r3$.ɵɵconditional($r3$.ɵɵpipeBind1(3, 2, ctx.val) === 1 ? 2 : $r3$.ɵɵpipeBind1(4, 4, ctx.val) === 2 ? 5 : 6);
}
}
4 changes: 2 additions & 2 deletions ...mpiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_if_template.js
View file
Expand Up @@ -40,7 +40,7 @@ function MyApp_Conditional_4_Template(rf, ctx) {
}
if (rf & 2) {
const $ctx_r2$ = $r3$.ɵɵnextContext();
$r3$.ɵɵconditional(0, $ctx_r2$.innerVal === 0 ? 0 : $ctx_r2$.innerVal === 1 ? 1 : $ctx_r2$.innerVal === 2 ? 2 : 3);
$r3$.ɵɵconditional($ctx_r2$.innerVal === 0 ? 0 : $ctx_r2$.innerVal === 1 ? 1 : $ctx_r2$.innerVal === 2 ? 2 : 3);
}
}

Expand All @@ -61,6 +61,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ctx.val === 0 ? 2 : ctx.val === 1 ? 3 : ctx.val === 2 ? 4 : 5);
$r3$.ɵɵconditional(ctx.val === 0 ? 2 : ctx.val === 1 ? 3 : ctx.val === 2 ? 4 : 5);
}
}
4 changes: 2 additions & 2 deletions ...er-cli/test/compliance/test_cases/r3_view_compiler_control_flow/nested_switch_template.js
View file
Expand Up @@ -30,7 +30,7 @@ function MyApp_Case_3_Template(rf, ctx) {
…
const $ctx_r1$ = $r3$.ɵɵnextContext();
…
$r3$.ɵɵconditional(0, ($MyApp_Case_3_contFlowTmp$ = $ctx_r1$.nestedValue()) === 0 ? 0 : $MyApp_Case_3_contFlowTmp$ === 1 ? 1 : $MyApp_Case_3_contFlowTmp$ === 2 ? 2 : -1);
$r3$.ɵɵconditional(($MyApp_Case_3_contFlowTmp$ = $ctx_r1$.nestedValue()) === 0 ? 0 : $MyApp_Case_3_contFlowTmp$ === 1 ? 1 : $MyApp_Case_3_contFlowTmp$ === 2 ? 2 : -1);
}
}

Expand All @@ -52,6 +52,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ($MyApp_contFlowTmp$ = ctx.value()) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : $MyApp_contFlowTmp$ === 2 ? 4 : -1);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = ctx.value()) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : $MyApp_contFlowTmp$ === 2 ? 4 : -1);
}
}
2 changes: 1 addition & 1 deletion ...compliance/test_cases/r3_view_compiler_control_flow/switch_with_pipe_template.pipeline.js
View file
Expand Up @@ -14,6 +14,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ($MyApp_contFlowTmp$ = $r3$.ɵɵpipeBind1(3, 2, ctx.value())) === $r3$.ɵɵpipeBind1(4, 4, 0) ? 2 : $MyApp_contFlowTmp$ === $r3$.ɵɵpipeBind1(5, 6, 1) ? 6 : 7);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = $r3$.ɵɵpipeBind1(3, 2, ctx.value())) === $r3$.ɵɵpipeBind1(4, 4, 0) ? 2 : $MyApp_contFlowTmp$ === $r3$.ɵɵpipeBind1(5, 6, 1) ? 6 : 7);
}
}
2 changes: 1 addition & 1 deletion ...st/compliance/test_cases/r3_view_compiler_control_flow/switch_without_default_template.js
View file
Expand Up @@ -28,6 +28,6 @@ function MyApp_Template(rf, ctx) {
$r3$.ɵɵadvance();
$r3$.ɵɵtextInterpolate1(" ", ctx.message, " ");
$r3$.ɵɵadvance();
$r3$.ɵɵconditional(2, ($MyApp_contFlowTmp$ = ctx.value()) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : $MyApp_contFlowTmp$ === 2 ? 4 : -1);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = ctx.value()) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : $MyApp_contFlowTmp$ === 2 ? 4 : -1);
}
}
4 changes: 2 additions & 2 deletions ...piler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/conditional_template.js
View file
Expand Up @@ -88,9 +88,9 @@ MyApp.ɵcmp = /*@__PURE__*/ $r3$.ɵɵdefineComponent({
}
if (rf & 2) {
$r3$.ɵɵadvance(2);
$r3$.ɵɵconditional(2, ctx.count === 0 ? 2 : ctx.count === 1 ? 3 : 4);
$r3$.ɵɵconditional(ctx.count === 0 ? 2 : ctx.count === 1 ? 3 : 4);
$r3$.ɵɵadvance(3);
$r3$.ɵɵconditional(5, ctx.count === 7 ? 5 : -1);
$r3$.ɵɵconditional(ctx.count === 7 ? 5 : -1);
}
},
…
Expand Down
2 changes: 1 addition & 1 deletion ...s/compiler-cli/test/compliance/test_cases/r3_view_compiler_i18n/blocks/switch_template.js
View file
Expand Up @@ -77,7 +77,7 @@ MyApp.ɵcmp = /*@__PURE__*/ $r3$.ɵɵdefineComponent({
if (rf & 2) {
let $MyApp_contFlowTmp$;
$r3$.ɵɵadvance(2);
$r3$.ɵɵconditional(2, ($MyApp_contFlowTmp$ = ctx.count) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : 4);
$r3$.ɵɵconditional(($MyApp_contFlowTmp$ = ctx.count) === 0 ? 2 : $MyApp_contFlowTmp$ === 1 ? 3 : 4);
}
},
…
Expand Down
10 changes: 2 additions & 8 deletions packages/compiler/src/template/pipeline/ir/src/ops/update.ts
View file
Expand Up @@ -598,11 +598,6 @@ export interface ConditionalOp extends Op<ConditionalOp>, DependsOnSlotContextOp
*/
target: XrefId;

/**
* The slot of the target, to be populated during slot allocation.
*/
targetSlot: SlotHandle;

/**
* The main test expression (for a switch), or `null` (for an if, which has no test
* expression).
Expand Down Expand Up @@ -634,12 +629,11 @@ export interface ConditionalOp extends Op<ConditionalOp>, DependsOnSlotContextOp
* Create a conditional op, which will display an embedded view according to a condtion.
*/
export function createConditionalOp(
target: XrefId, targetSlot: SlotHandle, test: o.Expression|null,
conditions: Array<ConditionalCaseExpr>, sourceSpan: ParseSourceSpan): ConditionalOp {
target: XrefId, test: o.Expression|null, conditions: Array<ConditionalCaseExpr>,
sourceSpan: ParseSourceSpan): ConditionalOp {
return {
kind: OpKind.Conditional,
target,
targetSlot,
test,
conditions,
processed: null,
Expand Down
15 changes: 4 additions & 11 deletions packages/compiler/src/template/pipeline/src/ingest.ts
View file
Expand Up @@ -355,7 +355,6 @@ function ingestBoundText(
*/
function ingestIfBlock(unit: ViewCompilationUnit, ifBlock: t.IfBlock): void {
let firstXref: ir.XrefId|null = null;
let firstSlotHandle: ir.SlotHandle|null = null;
let conditions: Array<ir.ConditionalCaseExpr> = [];
for (let i = 0; i < ifBlock.branches.length; i++) {
const ifCase = ifBlock.branches[i];
Expand All @@ -381,7 +380,6 @@ function ingestIfBlock(unit: ViewCompilationUnit, ifBlock: t.IfBlock): void {

if (firstXref === null) {
firstXref = cView.xref;
firstSlotHandle = templateOp.handle;
}

const caseExpr = ifCase.expression ? convertAst(ifCase.expression, unit.job, null) : null;
Expand All @@ -390,9 +388,7 @@ function ingestIfBlock(unit: ViewCompilationUnit, ifBlock: t.IfBlock): void {
conditions.push(conditionalCaseExpr);
ingestNodes(cView, ifCase.children);
}
const conditional =
ir.createConditionalOp(firstXref!, firstSlotHandle!, null, conditions, ifBlock.sourceSpan);
unit.update.push(conditional);
unit.update.push(ir.createConditionalOp(firstXref!, null, conditions, ifBlock.sourceSpan));
}

/**
Expand All @@ -405,7 +401,6 @@ function ingestSwitchBlock(unit: ViewCompilationUnit, switchBlock: t.SwitchBlock
}

let firstXref: ir.XrefId|null = null;
let firstSlotHandle: ir.SlotHandle|null = null;
let conditions: Array<ir.ConditionalCaseExpr> = [];
for (const switchCase of switchBlock.cases) {
const cView = unit.job.allocateView(unit.xref);
Expand All @@ -424,7 +419,6 @@ function ingestSwitchBlock(unit: ViewCompilationUnit, switchBlock: t.SwitchBlock
unit.create.push(templateOp);
if (firstXref === null) {
firstXref = cView.xref;
firstSlotHandle = templateOp.handle;
}
const caseExpr = switchCase.expression ?
convertAst(switchCase.expression, unit.job, switchBlock.startSourceSpan) :
Expand All @@ -434,10 +428,9 @@ function ingestSwitchBlock(unit: ViewCompilationUnit, switchBlock: t.SwitchBlock
conditions.push(conditionalCaseExpr);
ingestNodes(cView, switchCase.children);
}
const conditional = ir.createConditionalOp(
firstXref!, firstSlotHandle!, convertAst(switchBlock.expression, unit.job, null), conditions,
switchBlock.sourceSpan);
unit.update.push(conditional);
unit.update.push(ir.createConditionalOp(
firstXref!, convertAst(switchBlock.expression, unit.job, null), conditions,
switchBlock.sourceSpan));
}

function ingestDeferView(
Expand Down
4 changes: 2 additions & 2 deletions packages/compiler/src/template/pipeline/src/instruction.ts
View file
Expand Up @@ -560,9 +560,9 @@ function call<OpT extends ir.CreateOp|ir.UpdateOp>(
}

export function conditional(
slot: number, condition: o.Expression, contextValue: o.Expression|null,
condition: o.Expression, contextValue: o.Expression|null,
sourceSpan: ParseSourceSpan|null): ir.UpdateOp {
const args = [o.literal(slot), condition];
const args = [condition];
if (contextValue !== null) {
args.push(contextValue);
}
Expand Down
6 changes: 1 addition & 5 deletions packages/compiler/src/template/pipeline/src/phases/reify.ts
View file
Expand Up @@ -416,11 +416,7 @@ function reifyUpdateOperations(_unit: CompilationUnit, ops: ir.OpList<ir.UpdateO
if (op.processed === null) {
throw new Error(`Conditional test was not set.`);
}
if (op.targetSlot.slot === null) {
throw new Error(`Conditional slot was not set.`);
}
ir.OpList.replace(
op, ng.conditional(op.targetSlot.slot, op.processed, op.contextValue, op.sourceSpan));
ir.OpList.replace(op, ng.conditional(op.processed, op.contextValue, op.sourceSpan));
break;
case ir.OpKind.Repeater:
ir.OpList.replace(op, ng.repeater(op.collection, op.sourceSpan));
Expand Down
10 changes: 5 additions & 5 deletions packages/core/src/render3/instructions/control_flow.ts
View file
Expand Up @@ -33,12 +33,12 @@ import {declareTemplate} from './template';
* built-in "if" and "switch". On the high level this instruction is responsible for adding and
* removing views selected by a conditional expression.
*
* @param matchingTemplateIndex index of a template TNode representing a conditional view to be
* @param matchingTemplateIndex Index of a template TNode representing a conditional view to be
* inserted; -1 represents a special case when there is no view to insert.
* @param contextValue Value that should be exposed as the context of the conditional.
* @codeGenApi
*/
export function ɵɵconditional<T>(containerIndex: number, matchingTemplateIndex: number, value?: T) {
// TODO: we could remove the containerIndex argument to this instruction now (!)
export function ɵɵconditional<T>(matchingTemplateIndex: number, contextValue?: T) {
performanceMarkFeature('NgControlFlow');

const hostLView = getLView();
Expand Down Expand Up @@ -69,7 +69,7 @@ export function ɵɵconditional<T>(containerIndex: number, matchingTemplateIndex
const dehydratedView =
findMatchingDehydratedView(nextContainer, templateTNode.tView!.ssrId);
const embeddedLView =
createAndRenderEmbeddedLView(hostLView, templateTNode, value, {dehydratedView});
createAndRenderEmbeddedLView(hostLView, templateTNode, contextValue, {dehydratedView});

addLViewToLContainer(
nextContainer, embeddedLView, viewInContainerIdx,
Expand All @@ -83,7 +83,7 @@ export function ɵɵconditional<T>(containerIndex: number, matchingTemplateIndex
// changed - re-bind in context.
const lView = getLViewFromLContainer<T|undefined>(prevContainer, viewInContainerIdx);
if (lView !== undefined) {
lView[CONTEXT] = value;
lView[CONTEXT] = contextValue;
}
}
}
Expand Down

0 comments on commit 5fc8aa2

Please sign in to comment.