8000 fix(compiler): optimize away unnecessary restore/reset view calls · angular/angular@ce80136 · GitHub
[go: up one dir, main page]
Skip to content

Commit ce80136

Browse files
crisbetoamishne
crisbeto
authored and
amishne
committed
fix(compiler): optimize away unnecessary restore/reset view calls
When producing a listener, the template pipeline does the following in separate phases: 1. Generates all the variables available within its scope. 2. Adds `restoreView` and `resetView` calls if there are any referenced to local variables (e.g. `@let` or local refs). 3. Optimizes away the variables that aren't used. This means that we can end up in a situation where the references to the variables in the scope no longer exist, but we still enter and leave the view as if they're there which is unnecessary. These changes add a simple optimization pass that looks specifically for the pattern of a `restoreView` followed by a `return resetView(expr)`. Furthermore, by changing the order of some optimizations, we're able to drop the `getCurrentView` variable as well. Fixes #66286.
1 parent 1cf1370 commit ce80136

File tree

6 files changed

+144
-82
lines changed

6 files changed

+144
-82
lines changed

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_arrow_functions/arrow_function_defined_let.js‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,4 @@
1-
const arrowFn0 = (ctx, view) => (a, b) => {
2-
// NOTE: the restoreView and resetView calls here are the result of variable optimization not picking up some cases. We can remove them once #66286 is resolved.
3-
$r3$.ɵɵrestoreView(view);
4-
return $r3$.ɵɵresetView(ctx.componentValue + a + b);
5-
};
1+
const arrowFn0 = (ctx, view) => (a, b) => ctx.componentValue + a + b;
62
…
73
function TestComp_Conditional_2_Template(rf, ctx) {
84
if (rf & 1) {

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/GOLDEN_PARTIAL.js‎

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1042,3 +1042,38 @@ export declare class TestCmp {
10421042
static ɵcmp: i0.ɵɵComponentDeclaration<TestCmp, "test-cmp", never, {}, {}, never, never, true, never>;
10431043
}
10441044

1045+
/****************************************************************************************************
1046+
* PARTIAL FILE: listener_unused_let.js
1047+
****************************************************************************************************/
1048+
import { Component } from '@angular/core';
1049+
import * as i0 from "@angular/core";
1050+
export class TestCmp {
1051+
noop() { }
1052+
static ɵfac = i0.ɵɵngDeclareFactory({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestCmp, deps: [], target: i0.ɵɵFactoryTarget.Component });
1053+
static ɵcmp = i0.ɵɵngDeclareComponent({ minVersion: "14.0.0", version: "0.0.0-PLACEHOLDER", type: TestCmp, isStandalone: true, selector: "ng-component", ngImport: i0, template: `
1054+
@let foo = 123;
1055+
<button (click)="noop()"></button>
1056+
{{foo}}
1057+
`, isInline: true });
1058+
}
1059+
i0.ɵɵngDeclareClassMetadata({ minVersion: "12.0.0", version: "0.0.0-PLACEHOLDER", ngImport: i0, type: TestCmp, decorators: [{
1060+
type: Component,
1061+
args: [{
1062+
template: `
1063+
@let foo = 123;
1064+
<button (click)="noop()"></button>
1065+
{{foo}}
1066+
`,
1067+
}]
1068+
}] });
1069+
1070+
/****************************************************************************************************
1071+
* PARTIAL FILE: listener_unused_let.d.ts
1072+
****************************************************************************************************/
1073+
import * as i0 from "@angular/core";
1074+
export declare class TestCmp {
1075+
noop(): void;
1076+
static ɵfac: i0.ɵɵFactoryDeclaration<TestCmp, never>;
1077+
static ɵcmp: i0.ɵɵComponentDeclaration<TestCmp, "ng-component", never, {}, {}, never, never, true, never>;
1078+
}
1079+

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/TEST_CASES.json‎

Lines changed: 38 additions & 69 deletions
Original file line numberDiff line numberDiff line change
@@ -3,9 +3,7 @@
33
"cases": [
44
{
55
"description": "should create listener instruction on element",
6-
"inputFiles": [
7-
"element_listener.ts"
8-
],
6+
"inputFiles": ["element_listener.ts"],
97
"expectations": [
108
{
119
"files": [
@@ -20,9 +18,7 @@
2018
},
2119
{
2220
"description": "should create listener instruction on other components",
23-
"inputFiles": [
24-
"component_listener.ts"
25-
],
21+
"inputFiles": ["component_listener.ts"],
2622
"expectations": [
2723
{
2824
"files": [
@@ -37,9 +33,7 @@
3733
},
3834
{
3935
"description": "should create multiple listener instructions that share a view snapshot",
40< 4D1C code>-
"inputFiles": [
41-
"shared_snapshot_listeners.ts"
42-
],
36+
"inputFiles": ["shared_snapshot_listeners.ts"],
4337
"expectations": [
4438
{
4539
"files": [
@@ -54,9 +48,7 @@
5448
},
5549
{
5650
"description": "local refs in listeners defined before the local refs",
57-
"inputFiles": [
58-
"local_ref_before_listener.ts"
59-
],
51+
"inputFiles": ["local_ref_before_listener.ts"],
6052
"expectations": [
6153
{
6254
"files": [
@@ -80,9 +72,7 @@
8072
},
8173
{
8274
"description": "should chain multiple listeners on the same element",
83-
"inputFiles": [
84-
"same_element_chained_listeners.ts"
85-
],
75+
"inputFiles": ["same_element_chained_listeners.ts"],
8676
"expectations": [
8777
{
8878
"files": [
@@ -97,9 +87,7 @@
9787
},
9888
{
9989
"description": "should chain multiple listeners across elements",
100-
"inputFiles": [
101-
"cross_element_chained_listeners.ts"
102-
],
90+
"inputFiles": ["cross_element_chained_listeners.ts"],
10391
"expectations": [
10492
{
10593
"files": [
@@ -114,9 +102,7 @@
114102
},
115103
{
116104
"description": "should chain multiple listeners on the same template",
117-
"inputFiles": [
118-
"template_chained_listeners.ts"
119-
],
105+
"inputFiles": ["template_chained_listeners.ts"],
120106
"expectations": [
121107
{
122108
"files": [
@@ -131,9 +117,7 @@
131117
},
132118
{
133119
"description": "should not generate the $event argument if it is not being used in a template",
134-
"inputFiles": [
135-
"no_event_arg_listener.ts"
136-
],
120+
"inputFiles": ["no_event_arg_listener.ts"],
137121
"expectations": [
138122
{
139123
"files": [
@@ -148,9 +132,7 @@
148132
},
149133
{
150134
"description": "should not generate the $event argument if it is not being used in a host listener",
151-
"inputFiles": [
152-
"no_event_arg_host_listener.ts"
153-
],
135+
"inputFiles": ["no_event_arg_host_listener.ts"],
154136
"expectations": [
155137
{
156138
"files": [
@@ -165,9 +147,7 @@
165147
},
166148
{
167149
"description": "should generate the $event argument if it is being used in a host listener",
168-
"inputFiles": [
169-
"has_event_arg_host_listener.ts"
170-
],
150+
"inputFiles": ["has_event_arg_host_listener.ts"],
171151
"expectations": [
172152
{
173153
"files": [
@@ -182,9 +162,7 @@
182162
},
183163
{
184164
"description": "should assume $event is referring to the event variable in a listener by default",
185-
"inputFiles": [
186-
"event_arg_listener_implicit_meaning.ts"
187-
],
165+
"inputFiles": ["event_arg_listener_implicit_meaning.ts"],
188166
"expectations": [
189167
{
190168
"files": [
@@ -199,9 +177,7 @@
199177
},
200178
{
201179
4D1C "description": "should preserve accesses to $event if it is done through `this` in a listener",
202-
"inputFiles": [
203-
"event_explicit_access.ts"
204-
],
180+
"inputFiles": ["event_explicit_access.ts"],
205181
"expectations": [
206182
{
207183
"files": [
@@ -215,9 +191,7 @@
215191
},
216192
{
217193
"description": "should not assume that $event is referring to an event object inside a property",
218-
"inputFiles": [
219-
"event_in_property_binding.ts"
220-
],
194+
"inputFiles": ["event_in_property_binding.ts"],
221195
"expectations": [
222196
{
223197
"files": [
@@ -232,9 +206,7 @@
232206
},
233207
{
234208
"description": "should assume $event is referring to the event variable in a listener by default inside a host binding",
235-
"inputFiles": [
236-
"event_arg_host_listener_implicit_meaning.ts"
237-
],
209+
"inputFiles": ["event_arg_host_listener_implicit_meaning.ts"],
238210
"expectations": [
239211
{
240212
"files": [
@@ -249,9 +221,7 @@
249221
},
250222
{
251223
"description": "should preserve accesses to $event if it is done through `this` in a listener inside a host binding",
252-
"inputFiles": [
253-
"event_host_explicit_access.ts"
254-
],
224+
"inputFiles": ["event_host_explicit_access.ts"],
255225
"expectations": [
256226
{
257227
"files": [
@@ -266,9 +236,7 @@
266236
},
267237
{
268238
"description": "should generate the view restoration statements if a keyed write is used in an event listener from within an ng-template",
269-
"inputFiles": [
270-
"implicit_receiver_keyed_write_inside_template.ts"
271-
],
239+
"inputFiles": ["implicit_receiver_keyed_write_inside_template.ts"],
272240
"expectations": [
273241
{
274242
"files": [
@@ -283,9 +251,7 @@
283251
},
284252
{
285253
"description": "should reference correct context in listener inside embedded view",
286-
"inputFiles": [
287-
"embedded_view_listener_context.ts"
288-
],
254+
"inputFiles": ["embedded_view_listener_context.ts"],
289255
"expectations": [
290256
{
291257
"files": [
@@ -300,9 +266,7 @@
300266
},
301267
{
302268
"description": "should generate a simple two-way binding",
303-
"inputFiles": [
304-
"simple_two_way.ts"
305-
],
269+
"inputFiles": ["simple_two_way.ts"],
306270
"expectations": [
307271
{
308272
"files": [
@@ -317,9 +281,7 @@
317281
},
318282
{
319283
"description": "should generate a nested two-way binding",
320-
"inputFiles": [
321-
"nested_two_way.ts"
322-
],
284+
"inputFiles": ["nested_two_way.ts"],
323285
"expectations": [
324286
{
325287
"files": [
@@ -334,9 +296,7 @@
334296
},
335297
{
336298
"description": "should generate listener with multiple statements",
337-
"inputFiles": [
338-
"multiple_statements.ts"
339-
],
299+
"inputFiles": ["multiple_statements.ts"],
340300
"expectations": [
341301
{
342302
"failureMessage": "Incorrect template"
@@ -345,9 +305,7 @@
345305
},
346306
{
347307
"description": "should maintain the binding order between plain listeners and listeners part of a two-way binding",
348-
"inputFiles": [
349-
"mixed_one_way_two_way_listener_order.ts"
350-
],
308+
"inputFiles": ["mixed_one_way_two_way_listener_order.ts"],
351309
"expectations": [
352310
{
353311
"failureMessage": "Incorrect template"
@@ -356,9 +314,7 @@
356314
},
357315
{
358316
"description": "should generate a two-way binding to a @for loop variable that is a signal",
359-
"inputFiles": [
360-
"two_way_binding_to_signal_loop_variable.ts"
361-
],
317+
"inputFiles": ["two_way_binding_to_signal_loop_variable.ts"],
362318
"expectations": [
363319
{
364320
"files": [
@@ -373,9 +329,7 @@
373329
},
374330
{
375331
"description": "should generate a two-way binding to a $any expression",
376-
"inputFiles": [
377-
"two_way_to_any.ts"
378-
],
332+
"inputFiles": ["two_way_to_any.ts"],
379333
"expectations": [
380334
{
381335
"files": [
@@ -387,6 +341,21 @@
387341
"failureMessage": "Incorrect template"
388342
}
389343
]
344+
},
345+
{
346+
"description": "should not generate restore/reset view when listener does not use @let in the same scope",
347+
"inputFiles": ["listener_unused_let.ts"],
348+
"expectations": [
349+
{
350+
"files": [
351+
{
352+
"expected": "listener_unused_let_template.js",
353+
"generated": "listener_unused_let.js"
354+
}
355+
],
356+
"failureMessage": "Incorrect template"
357+
}
358+
]
390359
}
391360
]
392361
}

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/listener_unused_let.ts‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,12 @@
1+
import {Component} from '@angular/core';
2+
3+
@Component({
4+
template: `
5+
@let foo = 123;
6+
<button (click)="noop()"></button>
7+
{{foo}}
8+
`,
9+
})
10+
export class TestCmp {
11+
noop() {}
12+
}

‎packages/compiler-cli/test/compliance/test_cases/r3_view_compiler_listener/listener_unused_let_template.js‎

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,20 @@
1+
$r3$.ɵɵdefineComponent({
2+
…
3+
decls: 2,
4+
vars: 1,
5+
consts: [[3, "click"]],
6+
template: function TestCmp_Template(rf, ctx) {
7+
if (rf & 1) {
8+
$r3$.ɵɵdomElementStart(0, "button", 0);
9+
$r3$.ɵɵdomListener("click", function TestCmp_Template_button_click_0_listener() { return ctx.noop(); });
10+
$r3$.ɵɵdomElementEnd();
11+
$r3$.ɵɵtext(1);
12+
}
13+
if (rf & 2) {
14+
const $foo_r1$ = 123;
15+
$r3$.ɵɵadvance();
16+
$r3$.ɵɵtextInterpolate1(" ", $foo_r1$, " ");
17+
}
18+
},
19+
…
20+
});

0 commit comments

Comments
 (0)
0