Skip to content

Commit 32012a1

Browse files
tboschIgorMinar
tbosch
authored and
IgorMinar
committed
refactor(core): view engine - move handleEvent function from view to element
Some versions of TypeScript are super slow to compile functions that contain a lot of `if` conditions in them. Splitting the handle event expressions per element is similar to what we did in the old codegen.
1 parent 58ba4f0 commit 32012a1

File tree

16 files changed

+187
-183
lines changed

16 files changed

+187
-183
lines changed

‎modules/@angular/compiler/src/view_compiler_next/view_compiler.ts‎

Lines changed: 87 additions & 90 deletions
Large diffs are not rendered by default.

‎modules/@angular/core/src/view/element.ts‎

Lines changed: 14 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,12 +9,18 @@
99
import {isDevMode} from '../application_ref';
1010
import {SecurityContext} from '../security';
1111

12-
import {BindingDef, BindingType, DebugContext, DisposableFn, ElementData, ElementOutputDef, NodeData, NodeDef, NodeFlags, NodeType, QueryValueType, Services, ViewData, ViewDefinition, ViewDefinitionFactory, ViewFlags, asElementData} from './types';
12+
import {BindingDef, BindingType, DebugContext, DisposableFn, ElementData, ElementHandleEventFn, ElementOutputDef, NodeData, NodeDef, NodeFlags, NodeType, QueryValueType, Services, ViewData, ViewDefinition, ViewDefinitionFactory, ViewFlags, asElementData} from './types';
1313
import {checkAndUpdateBinding, dispatchEvent, elementEventFullName, filterQueryId, getParentRenderElement, resolveViewDefinition, sliceErrorStack, splitMatchedQueriesDsl, splitNamespace} from './util';
1414

15+
const NOOP: any = () => {};
16+
1517
export function anchorDef(
1618
flags: NodeFlags, matchedQueriesDsl: [string | number, QueryValueType][],
17-
ngContentIndex: number, childCount: number, templateFactory?: ViewDefinitionFactory): NodeDef {
19+
ngContentIndex: number, childCount: number, handleEvent?: ElementHandleEventFn,
20+
templateFactory?: ViewDefinitionFactory): NodeDef {
21+
if (!handleEvent) {
22+
handleEvent = NOOP;
23+
}
1824
const {matchedQueries, references, matchedQueryIds} = splitMatchedQueriesDsl(matchedQueriesDsl);
1925
// skip the call to sliceErrorStack itself + the call to this function.
2026
const source = isDevMode() ? sliceErrorStack(2, 3) : '';
@@ -43,7 +49,7 @@ export function anchorDef(
4349
// will bet set by the view definition
4450
component: undefined,
4551
publicProviders: undefined,
46-
allProviders: undefined,
52+
allProviders: undefined, handleEvent
4753
},
4854
provider: undefined,
4955
text: undefined,
@@ -60,7 +66,10 @@ export function elementDef(
6066
bindings?:
6167
([BindingType.ElementClass, string] | [BindingType.ElementStyle, string, string] |
6268
[BindingType.ElementAttribute | BindingType.ElementProperty, string, SecurityContext])[],
63-
outputs?: (string | [string, string])[]): NodeDef {
69+
outputs?: (string | [string, string])[], handleEvent?: ElementHandleEventFn): NodeDef {
70+
if (!handleEvent) {
71+
handleEvent = NOOP;
72+
}
6473
// skip the call to sliceErrorStack itself + the call to this function.
6574
const source = isDevMode() ? sliceErrorStack(2, 3) : '';
6675
const {matchedQueries, references, matchedQueryIds} = splitMatchedQueriesDsl(matchedQueriesDsl);
@@ -131,7 +140,7 @@ export function elementDef(
131140
// will bet set by the view definition
132141
component: undefined,
133142
publicProviders: undefined,
134-
allProviders: undefined,
143+
allProviders: undefined, handleEvent,
135144
},
136145
provider: undefined,
137146
text: undefined,

‎modules/@angular/core/src/view/types.ts‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -205,13 +205,16 @@ export interface ElementDef {
205205
*/
206206
allProviders: {[tokenKey: string]: NodeDef};
207207
source: string;
208+
handleEvent: ElementHandleEventFn;
208209
}
209210

210211
export interface ElementOutputDef {
211212
target: string;
212213
eventName: string;
213214
}
214215

216+
export type ElementHandleEventFn = (view: ViewData, eventName: string, event: any) => boolean;
217+
215218
export interface ProviderDef {
216219
type: ProviderType;
217220
token: any;

‎modules/@angular/core/src/view/view.ts‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ const NOOP = (): any => undefined;
2323

2424
export function viewDef(
2525
flags: ViewFlags, nodes: NodeDef[], updateDirectives?: ViewUpdateFn,
26-
updateRenderer?: ViewUpdateFn, handleEvent?: ViewHandleEventFn): ViewDefinition {
26+
updateRenderer?: ViewUpdateFn): ViewDefinition {
2727
// clone nodes and set auto calculated values
2828
if (nodes.length === 0) {
2929
throw new Error(`Illegal State: Views without nodes are not allowed!`);
@@ -131,6 +131,8 @@ export function viewDef(
131131
}
132132
currentParent = newParent;
133133
}
134+
const handleEvent: ViewHandleEventFn = (view, nodeIndex, eventName, event) =>
135+
nodes[nodeIndex].element.handleEvent(view, eventName, event);
134136
return {
135137
nodeFlags: viewNodeFlags,
136138
nodeMatchedQueries: viewMatchedQueries, flags,
@@ -143,6 +145,8 @@ export function viewDef(
143145
};
144146
}
145147

148+
149+
146150
function calculateReverseChildIndex(
147151
currentParent: NodeDef, i: number, childCount: number, nodeCount: number) {
148152
// Notes about reverse child order:

‎modules/@angular/core/test/view/anchor_spec.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,9 +15,9 @@ import {createRootView, isBrowser} from './helper';
1515
export function main() {
1616
describe(`View Anchor`, () => {
1717
function compViewDef(
18-
nodes: NodeDef[], updateDirectives?: ViewUpdateFn, updateRenderer?: ViewUpdateFn,
19-
handleEvent?: ViewHandleEventFn): ViewDefinition {
20-
return viewDef(ViewFlags.None, nodes, updateDirectives, updateRenderer, handleEvent);
18+
nodes: NodeDef[], updateDirectives?: ViewUpdateFn,
19+
updateRenderer?: ViewUpdateFn): ViewDefinition {
20+
return viewDef(ViewFlags.None, nodes, updateDirectives, updateRenderer);
2121
}
2222

2323
function createAndGetRootNodes(

‎modules/@angular/core/test/view/component_view_spec.ts‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ export function main() {
1616
describe(`Component Views`, () => {
1717
function compViewDef(
1818
nodes: NodeDef[], updateDirectives?: ViewUpdateFn, updateRenderer?: ViewUpdateFn,
19-
handleEvent?: ViewHandleEventFn, viewFlags: ViewFlags = ViewFlags.None): ViewDefinition {
20-
return viewDef(viewFlags, nodes, updateDirectives, updateRenderer, handleEvent);
19+
viewFlags: ViewFlags = ViewFlags.None): ViewDefinition {
20+
return viewDef(viewFlags, nodes, updateDirectives, updateRenderer);
2121
}
2222

2323
function createAndGetRootNodes(viewDef: ViewDefinition): {rootNodes: any[], view: ViewData} {
@@ -191,7 +191,7 @@ export function main() {
191191
[
192192
elementDef(NodeFlags.None, null, null, 0, 'span', null, null, ['click']),
193193
],
194-
update, null, null, ViewFlags.OnPush)),
194+
update, null, ViewFlags.OnPush)),
195195
],
196196
(check, view) => { check(view, 1, ArgumentType.Inline, compInputValue); }));
197197

‎modules/@angular/core/test/view/element_spec.ts‎

Lines changed: 23 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ export function main() {
1717
describe(`View Elements`, () => {
1818
function compViewDef(
1919
nodes: NodeDef[], updateDirectives?: ViewUpdateFn, updateRenderer?: ViewUpdateFn,
20-
handleEvent?: ViewHandleEventFn, viewFlags: ViewFlags = ViewFlags.None): ViewDefinition {
21-
return viewDef(viewFlags, nodes, updateDirectives, updateRenderer, handleEvent);
20+
viewFlags: ViewFlags = ViewFlags.None): ViewDefinition {
21+
return viewDef(viewFlags, nodes, updateDirectives, updateRenderer);
2222
}
2323

2424
function createAndGetRootNodes(
@@ -189,18 +189,16 @@ export function main() {
189189
const handleEventSpy = jasmine.createSpy('handleEvent');
190190
const removeListenerSpy =
191191
spyOn(HTMLElement.prototype, 'removeEventListener').and.callThrough();
192-
const {view, rootNodes} = createAndAttachAndGetRootNodes(compViewDef(
193-
[elementDef(NodeFlags.None, null, null, 0, 'button', null, null, ['click'])], null,
194-
null, handleEventSpy));
192+
const {view, rootNodes} = createAndAttachAndGetRootNodes(compViewDef([elementDef(
193+
NodeFlags.None, null, null, 0, 'button', null, null, ['click'], handleEventSpy)]));
195194

196195
rootNodes[0].click();
197196

198197
expect(handleEventSpy).toHaveBeenCalled();
199198
let handleEventArgs = handleEventSpy.calls.mostRecent().args;
200199
expect(handleEventArgs[0]).toBe(view);
201-
expect(handleEventArgs[1]).toBe(0);
202-
expect(handleEventArgs[2]).toBe('click');
203-
expect(handleEventArgs[3]).toBeTruthy();
200+
expect(handleEventArgs[1]).toBe('click');
201+
expect(handleEventArgs[2]).toBeTruthy();
204202

205203
Services.destroyView(view);
206204

@@ -211,11 +209,9 @@ export function main() {
211209
const handleEventSpy = jasmine.createSpy('handleEvent');
212210
const addListenerSpy = spyOn(window, 'addEventListener');
213211
const removeListenerSpy = spyOn(window, 'removeEventListener');
214-
const {view, rootNodes} = createAndAttachAndGetRootNodes(compViewDef(
215-
[elementDef(
216-
NodeFlags.None, null, null, 0, 'button', null, null,
217-
[['window', 'windowClick']])],
218-
null, null, handleEventSpy));
212+
const {view, rootNodes} = createAndAttachAndGetRootNodes(compViewDef([elementDef(
213+
NodeFlags.None, null, null, 0, 'button', null, null, [['window', 'windowClick']],
214+
handleEventSpy)]));
219215

220216
expect(addListenerSpy).toHaveBeenCalled();
221217
expect(addListenerSpy.calls.mostRecent().args[0]).toBe('windowClick');
@@ -224,9 +220,8 @@ export function main() {
224220
expect(handleEventSpy).toHaveBeenCalled();
225221
const handleEventArgs = handleEventSpy.calls.mostRecent().args;
226222
expect(handleEventArgs[0]).toBe(view);
227-
expect(handleEventArgs[1]).toBe(0);
228-
expect(handleEventArgs[2]).toBe('window:windowClick');
229-
expect(handleEventArgs[3]).toBeTruthy();
223+
expect(handleEventArgs[1]).toBe('window:windowClick');
224+
expect(handleEventArgs[2]).toBeTruthy();
230225

231226
Services.destroyView(view);
232227

@@ -237,11 +232,9 @@ export function main() {
237232
const handleEventSpy = jasmine.createSpy('handleEvent');
238233
const addListenerSpy = spyOn(document, 'addEventListener');
239234
const removeListenerSpy = spyOn(document, 'removeEventListener');
240-
const {view, rootNodes} = createAndAttachAndGetRootNodes(compViewDef(
241-
[elementDef(
242-
NodeFlags.None, null, null, 0, 'button', null, null,
243-
[['document', 'documentClick']])],
244-
null, null, handleEventSpy));
235+
const {view, rootNodes} = createAndAttachAndGetRootNodes(compViewDef([elementDef(
236+
NodeFlags.None, null, null, 0, 'button', null, null, [['document', 'documentClick']],
237+
handleEventSpy)]));
245238

246239
expect(addListenerSpy).toHaveBeenCalled();
247240
expect(addListenerSpy.calls.mostRecent().args[0]).toBe('documentClick');
@@ -250,9 +243,8 @@ export function main() {
250243
expect(handleEventSpy).toHaveBeenCalled();
251244
const handleEventArgs = handleEventSpy.calls.mostRecent().args;
252245
expect(handleEventArgs[0]).toBe(view);
253-
expect(handleEventArgs[1]).toBe(0);
254-
expect(handleEventArgs[2]).toBe('document:documentClick');
255-
expect(handleEventArgs[3]).toBeTruthy();
246+
expect(handleEventArgs[1]).toBe('document:documentClick');
247+
expect(handleEventArgs[2]).toBeTruthy();
256248

257249
Services.destroyView(view);
258250

@@ -263,12 +255,12 @@ export function main() {
263255
let eventHandlerResult: any;
264256
let preventDefaultSpy: jasmine.Spy;
265257

266-
const {view, rootNodes} = createAndAttachAndGetRootNodes(compViewDef(
267-
[elementDef(NodeFlags.None, null, null, 0, 'button', null, null, ['click'])], null,
268-
null, (view, index, eventName, event) => {
258+
const {view, rootNodes} = createAndAttachAndGetRootNodes(compViewDef([elementDef(
259+
NodeFlags.None, null, null, 0, 'button', null, null, ['click'],
260+
(view, eventName, event) => {
269261
preventDefaultSpy = spyOn(event, 'preventDefault').and.callThrough();
270262
return eventHandlerResult;
271-
}));
263+
})]));
272264

273265
eventHandlerResult = undefined;
274266
rootNodes[0].click();
@@ -290,8 +282,9 @@ export function main() {
290282
it('should report debug info on event errors', () => {
291283
const addListenerSpy = spyOn(HTMLElement.prototype, 'addEventListener').and.callThrough();
292284
const {view, rootNodes} = createAndAttachAndGetRootNodes(compViewDef(
293-
[elementDef(NodeFlags.None, null, null, 0, 'button', null, null, ['click'])], null,
294-
null, () => { throw new Error('Test'); }));
285+
[elementDef(NodeFlags.None, null, null, 0, 'button', null, null, ['click'], () => {
286+
throw new Error('Test');
287+
})]));
295288

296289
let err: any;
297290
try {

‎modules/@angular/core/test/view/embedded_view_spec.ts‎

Lines changed: 10 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -17,8 +17,8 @@ export function main() {
1717
describe(`Embedded Views`, () => {
1818
function compViewDef(
1919
nodes: NodeDef[], updateDirectives?: ViewUpdateFn, updateRenderer?: ViewUpdateFn,
20-
handleEvent?: ViewHandleEventFn, viewFlags: ViewFlags = ViewFlags.None): ViewDefinition {
21-
return viewDef(viewFlags, nodes, updateDirectives, updateRenderer, handleEvent);
20+
viewFlags: ViewFlags = ViewFlags.None): ViewDefinition {
21+
return viewDef(viewFlags, nodes, updateDirectives, updateRenderer);
2222
}
2323

2424
function embeddedViewDef(nodes: NodeDef[], update?: ViewUpdateFn): ViewDefinitionFactory {
@@ -40,7 +40,7 @@ export function main() {
4040
compViewDef([
4141
elementDef(NodeFlags.None, null, null, 1, 'div'),
4242
anchorDef(
43-
NodeFlags.HasEmbeddedViews, null, null, 0,
43+
NodeFlags.HasEmbeddedViews, null, null, 0, null,
4444
embeddedViewDef([elementDef(NodeFlags.None, null, null, 0, 'span')])),
4545
]),
4646
parentContext);
@@ -54,10 +54,10 @@ export function main() {
5454
it('should attach and detach embedded views', () => {
5555
const {view: parentView, rootNodes} = createAndGetRootNodes(compViewDef([
5656
elementDef(NodeFlags.None, null, null, 2, 'div'),
57-
anchorDef(NodeFlags.HasEmbeddedViews, null, null, 0, embeddedViewDef([
57+
anchorDef(NodeFlags.HasEmbeddedViews, null, null, 0, null, embeddedViewDef([
5858
elementDef(NodeFlags.None, null, null, 0, 'span', [['name', 'child0']])
5959
])),
60-
anchorDef(NodeFlags.None, null, null, 0, embeddedViewDef([
60+
anchorDef(NodeFlags.None, null, null, 0, null, embeddedViewDef([
6161
elementDef(NodeFlags.None, null, null, 0, 'span', [['name', 'child1']])
6262
]))
6363
]));
@@ -84,10 +84,10 @@ export function main() {
8484
it('should move embedded views', () => {
8585
const {view: parentView, rootNodes} = createAndGetRootNodes(compViewDef([
8686
elementDef(NodeFlags.None, null, null, 2, 'div'),
87-
anchorDef(NodeFlags.HasEmbeddedViews, null, null, 0, embeddedViewDef([
87+
anchorDef(NodeFlags.HasEmbeddedViews, null, null, 0, null, embeddedViewDef([
8888
elementDef(NodeFlags.None, null, null, 0, 'span', [['name', 'child0']])
8989
])),
90-
anchorDef(NodeFlags.None, null, null, 0, embeddedViewDef([
90+
anchorDef(NodeFlags.None, null, null, 0, null, embeddedViewDef([
9191
elementDef(NodeFlags.None, null, null, 0, 'span', [['name', 'child1']])
9292
]))
9393
]));
@@ -111,7 +111,7 @@ export function main() {
111111

112112
it('should include embedded views in root nodes', () => {
113113
const {view: parentView} = createAndGetRootNodes(compViewDef([
114-
anchorDef(NodeFlags.HasEmbeddedViews, null, null, 0, embeddedViewDef([
114+
anchorDef(NodeFlags.HasEmbeddedViews, null, null, 0, null, embeddedViewDef([
115115
elementDef(NodeFlags.None, null, null, 0, 'span', [['name', 'child0']])
116116
])),
117117
elementDef(NodeFlags.None, null, null, 0, 'span', [['name', 'after']])
@@ -136,7 +136,7 @@ export function main() {
136136
const {view: parentView, rootNodes} = createAndGetRootNodes(compViewDef([
137137
elementDef(NodeFlags.None, null, null, 1, 'div'),
138138
anchorDef(
139-
NodeFlags.HasEmbeddedViews, null, null, 0,
139+
NodeFlags.HasEmbeddedViews, null, null, 0, null,
140140
embeddedViewDef(
141141
[elementDef(
142142
NodeFlags.None, null, null, 0, 'span', null,
@@ -172,7 +172,7 @@ export function main() {
172172

173173
const {view: parentView} = createAndGetRootNodes(compViewDef([
174174
elementDef(NodeFlags.None, null, null, 1, 'div'),
175-
anchorDef(NodeFlags.HasEmbeddedViews, null, null, 0, embeddedViewDef([
175+
anchorDef(NodeFlags.HasEmbeddedViews, null, null, 0, null, embeddedViewDef([
176176
elementDef(NodeFlags.None, null, null, 1, 'span'),
177177
directiveDef(NodeFlags.OnDestroy, null, 0, ChildProvider, [])
178178
]))

‎modules/@angular/core/test/view/ng_content_spec.ts‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,8 +16,8 @@ export function main() {
1616
describe(`View NgContent`, () => {
1717
function compViewDef(
1818
nodes: NodeDef[], updateDirectives?: ViewUpdateFn, updateRenderer?: ViewUpdateFn,
19-
handleEvent?: ViewHandleEventFn, viewFlags: ViewFlags = ViewFlags.None): ViewDefinition {
20-
return viewDef(viewFlags, nodes, updateDirectives, updateRenderer, handleEvent);
19+
viewFlags: ViewFlags = ViewFlags.None): ViewDefinition {
20+
return viewDef(viewFlags, nodes, updateDirectives, updateRenderer);
2121
}
2222

2323
function embeddedViewDef(nodes: NodeDef[], update?: ViewUpdateFn): ViewDefinitionFactory {
@@ -85,8 +85,8 @@ export function main() {
8585

8686
const {view, rootNodes} = createAndGetRootNodes(compViewDef(hostElDef(
8787
[
88-
anchorDef(
89-
NodeFlags.HasEmbeddedViews, null, 0, 1, embeddedViewDef([textDef(null, ['a'])])),
88+
anchorDef(NodeFlags.HasEmbeddedViews, null, 0, 1, null, embeddedViewDef([textDef(
89+
null, ['a'])])),
9090
directiveDef(
9191
NodeFlags.None, null, 0, CreateViewService, [TemplateRef, ViewContainerRef])
9292
],
@@ -103,7 +103,7 @@ export function main() {
103103
it('should include projected nodes when attaching / detaching embedded views', () => {
104104
const {view, rootNodes} = createAndGetRootNodes(compViewDef(hostElDef([textDef(0, ['a'])], [
105105
elementDef(NodeFlags.None, null, null, 1, 'div'),
106-
anchorDef(NodeFlags.HasEmbeddedViews, null, 0, 0, embeddedViewDef([
106+
anchorDef(NodeFlags.HasEmbeddedViews, null, 0, 0, null, embeddedViewDef([
107107
ngContentDef(null, 0),
108108
// The anchor would be added by the compiler after the ngContent
109109
anchorDef(NodeFlags.None, null, null, 0),

0 commit comments

Comments
 (0)