From d82d58621ecc3ec4dfc40bccdb9daa7e646788e2 Mon Sep 17 00:00:00 2001 From: Dylan Hunn Date: Mon, 23 Oct 2023 18:55:11 -0700 Subject: [PATCH] refactor(compiler): Don't double-create pipes in switch cases (#52289) Previously, we would emit *two* pipe creation instructions for each pipe in a switch case. This is because we were visiting both the transformed and raw versions of the pipe bindings. Now, we clear the raw case expressions array after generating the transformed test expression. Also, we introduce some new goldens, because our pipe creation order is harmlessly different. PR Close #52289 --- .../TEST_CASES.json | 12 +++---- .../if_with_pipe_template.pipeline.js | 36 +++++++++++++++++++ .../switch_with_pipe_template.pipeline.js | 20 +++++++++++ .../pipeline/src/phases/conditionals.ts | 4 +++ 4 files changed, 66 insertions(+), 6 deletions(-) create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/if_with_pipe_template.pipeline.js create mode 100644 packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_with_pipe_template.pipeline.js diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json index 2b96eddb5cb..408a5e3e11d 100644 --- a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/TEST_CASES.json @@ -62,13 +62,13 @@ "files": [ { "expected": "switch_with_pipe_template.js", - "generated": "switch_with_pipe.js" + "generated": "switch_with_pipe.js", + "templatePipelineExpected": "switch_with_pipe_template.pipeline.js" } ], "failureMessage": "Incorrect template" } - ], - "skipForTemplatePipeline": true + ] }, { "description": "should generate a basic if block", @@ -148,13 +148,13 @@ "files": [ { "expected": "if_with_pipe_template.js", - "generated": "if_with_pipe.js" + "generated": "if_with_pipe.js", + "templatePipelineExpected": "if_with_pipe_template.pipeline.js" } ], "failureMessage": "Incorrect template" } - ], - "skipForTemplatePipeline": true + ] }, { "description": "should generate an if block with an aliased expression", diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/if_with_pipe_template.pipeline.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/if_with_pipe_template.pipeline.js new file mode 100644 index 00000000000..b14c39b00f7 --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/if_with_pipe_template.pipeline.js @@ -0,0 +1,36 @@ +function $MyApp_Conditional_3_Template$(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵtext(0, " one "); + } + } + + function $MyApp_Conditional_5_Template$(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵtext(0, " two "); + } + } + + function $MyApp_Conditional_6_Template$(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵtext(0, " three "); + } + } + … + function MyApp_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵelementStart(0, "div"); + $r3$.ɵɵtext(1); + $r3$.ɵɵtemplate(2, MyApp_Conditional_2_Template, 1, 0); + $r3$.ɵɵpipe(3, "test"); + $r3$.ɵɵpipe(4, "test"); + $r3$.ɵɵtemplate(5, MyApp_Conditional_5_Template, 1, 0)(6, MyApp_Conditional_6_Template, 1, 0); + $r3$.ɵɵelementEnd(); + } + if (rf & 2) { + $r3$.ɵɵadvance(1); + $r3$.ɵɵtextInterpolate1(" ", ctx.message, " "); + $r3$.ɵɵadvance(1); + $r3$.ɵɵconditional(2, $r3$.ɵɵpipeBind1(3, 2, ctx.val) === 1 ? 2 : $r3$.ɵɵpipeBind1(4, 4, ctx.val) === 2 ? 5 : 6); + } + } + \ No newline at end of file diff --git a/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_with_pipe_template.pipeline.js b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_with_pipe_template.pipeline.js new file mode 100644 index 00000000000..38d4b608c2f --- /dev/null +++ b/packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_control_flow/switch_with_pipe_template.pipeline.js @@ -0,0 +1,20 @@ +function MyApp_Template(rf, ctx) { + if (rf & 1) { + $r3$.ɵɵelementStart(0, "div"); + $r3$.ɵɵtext(1); + $r3$.ɵɵtemplate(2, MyApp_Case_2_Template, 1, 0); + $r3$.ɵɵpipe(3, "test"); + $r3$.ɵɵpipe(4, "test"); + $r3$.ɵɵpipe(5, "test"); + $r3$.ɵɵtemplate(6, MyApp_Case_6_Template, 1, 0)(7, MyApp_Case_7_Template, 1, 0); + $r3$.ɵɵelementEnd(); + } + if (rf & 2) { + let $MyApp_contFlowTmp$; + $r3$.ɵɵadvance(1); + $r3$.ɵɵtextInterpolate1(" ", ctx.message, " "); + $r3$.ɵɵadvance(1); + $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); + } + } + \ No newline at end of file diff --git a/packages/compiler/src/template/pipeline/src/phases/conditionals.ts b/packages/compiler/src/template/pipeline/src/phases/conditionals.ts index 20e705dbae7..cde87a99503 100644 --- a/packages/compiler/src/template/pipeline/src/phases/conditionals.ts +++ b/packages/compiler/src/template/pipeline/src/phases/conditionals.ts @@ -58,6 +58,10 @@ export function phaseConditionals(job: ComponentCompilationJob): void { // Save the resulting aggregate Joost-expression. op.processed = test; + + // Clear the original conditions array, since we no longer need it, and don't want it to + // affect subsequent phases (e.g. pipe creation). + op.conditions = []; } } }