feat(core): add initial view engine #14014
Conversation
tbosch
force-pushed
the
view_manager
branch
3 times, most recently
from
bded18a
to
0e4cee4
Compare
| @@ -8,7 +8,7 @@ | |||
| "moduleResolution": "node", | |||
| "outDir": "../dist/all/", | |||
| "noImplicitAny": true, | |||
| "noFallthroughCasesInSwitch": true, | |||
| "noFallthroughCasesInSwitch": false, | |||
There was a problem hiding this comment.
Is this allowed in G3?
There was a problem hiding this comment.
Yes, for third party code.
| providerDef(NodeFlags.None, TreeComponent, [], null, () => TreeComponent_0), | ||
| ], | ||
| null, | ||
| new RenderComponentType('TreeComponent_Host', 'someUrl', 0, ViewEncapsulation.None, [], {}), ); |
There was a problem hiding this comment.
As we use directDom we don't need this here.
| viewFlags, | ||
| [ | ||
| elementDef(NodeFlags.None, 1, 'tree'), | ||
| providerDef(NodeFlags.None, TreeComponent, [], {'data': 0}, () => TreeComponent_0), |
There was a problem hiding this comment.
should not be quoted
There was a problem hiding this comment.
Check the spec what happens with JavaScript keywords:
- required quoting for an object literal vs allow to use via
.x?
There was a problem hiding this comment.
Checked in IE11 and Firefox, both allow var x = {function: 1} as well as x.function
| NodeFlags.None, 1, 'span', null, [[BindingType.ElementStyle, 'backgroundColor', null]]), | ||
| textDef([' ', ' ']), | ||
| anchorDef(NodeFlags.HasEmbeddedViews, 1, TreeComponent_1), | ||
| providerDef(NodeFlags.None, NgIf, [ViewContainerRef, TemplateRef], {'ngIf': 0}), |
There was a problem hiding this comment.
don't quote the properties
| updater.checkInline(view, 3, cmp.data.left != null); | ||
| updater.checkInline(view, 5, cmp.data.right != null); | ||
| }, | ||
| new RenderComponentType('TreeComponent_0', 'someUrl', 0, ViewEncapsulation.None, [], {}), ); |
There was a problem hiding this comment.
remove RendercomponentType in this example
|
|
||
| export interface NodeUpdater { | ||
| checkInline( | ||
| view: ViewData, index: number, v0?: any, v1?: any, v2?: any, v3?: any, v4?: any, v5?: any, |
|
|
||
| export interface TextDef { prefix: string; } | ||
|
|
||
| export interface NodeDef { |
There was a problem hiding this comment.
Move this up, think about order
| export type ViewUpdateFn = (updater: NodeUpdater, view: ViewData, component: any, context: any) => | ||
| void; | ||
|
|
||
| export interface ViewDefinition { |
| void; | ||
|
|
||
| export interface ViewDefinition { | ||
| nodeFlags: NodeFlags; |
There was a problem hiding this comment.
Add comment: aggregation of flags of all nodes
| nodeFlags: NodeFlags; | ||
| flags: ViewFlags; | ||
| nodes: NodeDef[]; | ||
| reverseChildNodes: NodeDef[]; |
There was a problem hiding this comment.
Comment: providers are always after elements / anchors.
| export interface ViewDefinition { | ||
| nodeFlags: NodeFlags; | ||
| flags: ViewFlags; | ||
| nodes: NodeDef[]; |
There was a problem hiding this comment.
Comment: providers are always before elements / anchors.
| bindingIndex: number; | ||
| bindings: BindingDef[]; | ||
| element: ElementDef; | ||
| providerIndices: {[tokenKey: string]: number}; |
There was a problem hiding this comment.
move into ProviderDef?
| embeddedViews: ViewData[]; | ||
| } | ||
|
|
||
| export interface ViewData { |
There was a problem hiding this comment.
Add comment: performance sensitive (numbe rof properties)
| // Data | ||
| // ------------------------------------- | ||
|
|
||
| export interface NodeData { |
There was a problem hiding this comment.
Add comment: performance sensitive (numbe rof properties)
| setElementAttribute(view, binding, renderNode, name, value); | ||
| break; | ||
| case BindingType.ElementClass: | ||
| if (view.renderer) { |
There was a problem hiding this comment.
move into helpe ras well
| if (view.renderer) { | ||
| view.renderer.setElementAttribute(renderNode, name, renderValue); | ||
| } else { | ||
| renderNode.setAttribute(name, renderValue); |
There was a problem hiding this comment.
Add code for atribute removal (see DomRenderer)
| if (view.renderer) { | ||
| view.renderer.setElementStyle(renderNode, name, renderValue); | ||
| } else { | ||
| renderNode.style[name] = renderValue; |
There was a problem hiding this comment.
check style removal?
| import {checkAndUpdateBinding, checkAndUpdateBindingWithChange, setBindingDebugInfo} from './util'; | ||
|
|
||
| let _tokenKeyCache = new Map<any, string>(); | ||
| let _nextTokenKeyIndex = 0; |
There was a problem hiding this comment.
remove, use the size of the map
| if (Array.isArray(value)) { | ||
| [flags, token] = value; | ||
| } else { | ||
| token = value; |
| return key; | ||
| } | ||
|
|
||
| export function resolveDep( |
There was a problem hiding this comment.
move to createIOnstance
| view: ViewData, elIndex: number, depDef: DepDef, | ||
| notFoundValue: any = Injector.THROW_IF_NOT_FOUND): any { | ||
| const tokenKey = depDef.tokenKey; | ||
| let elDef = view.def.nodes[elIndex]; |
| } | ||
|
|
||
| while (view) { | ||
| elDef = view.def.nodes[elIndex]; |
| view: ViewData, def: NodeDef, v0: any, v1: any, v2: any, v3: any, v4: any, v5: any, v6: any, | ||
| v7: any, v8: any, v9: any) { | ||
| const provider = view.nodes[def.index].provider; | ||
| let changes: {[prop: string]: SimpleChange}; |
There was a problem hiding this comment.
Use SimpleChanges type
| if (changes) { | ||
| provider.ngOnChanges(changes); | ||
| } | ||
| if ((def.flags & NodeFlags.OnInit) && view.firstChange) { |
There was a problem hiding this comment.
move view.firstChange first
| if (changes) { | ||
| provider.ngOnChanges(changes); | ||
| } | ||
| if ((def.flags & NodeFlags.OnInit) && view.firstChange) { |
There was a problem hiding this comment.
move view.firstChagne first
|
|
||
| function checkAndUpdateProp( | ||
| view: ViewData, provider: any, def: NodeDef, bindingIdx: number, value: any, | ||
| changes: {[propName: string]: SimpleChange}): {[propName: string]: SimpleChange} { |
| i + nodesWithoutIndices[i].childCount - (currentParent.index + 1); | ||
| const parentEndIndexInReverseChildOrder = | ||
| currentParent.reverseChildIndex + currentParent.childCount; | ||
| reverseChildIndex = |
| let componentView: ViewData; | ||
| if (nodeDef.component) { | ||
| const compViewDef = nodeDef.component(); | ||
| const services = view.services; |
| } | ||
|
|
||
| const CheckAndUpdate: NodeUpdater = { | ||
| checkInline( |
There was a problem hiding this comment.
change to checkInline: (...)?!
| }; | ||
|
|
||
| export function destroyView(view: ViewData) { | ||
| execComponentViewsAction(view, ViewAction.Destroy); |
There was a problem hiding this comment.
Add comment: Bottom up
| export function destroyView(view: ViewData) { | ||
| execComponentViewsAction(view, ViewAction.Destroy); | ||
| execEmbeddedViewsAction(view, ViewAction.Destroy); | ||
| callLifecycleHooksChildrenFirst(view, NodeFlags.OnDestroy); |
There was a problem hiding this comment.
call destroy before the views are destroyed
| const nextSibling = prevRenderNode.nextSibling; | ||
| if (parentNode) { | ||
| directDomAttachDetachSiblingRenderNodes( | ||
| view, 0, nextSibling ? DirectDomAction.InsertBefore : DirectDomAction.AppendChild, |
There was a problem hiding this comment.
move action into const before
| } | ||
| } | ||
| } | ||
| i += nodeDef.childCount; |
There was a problem hiding this comment.
add comment: This jumps to the next sibling
| } | ||
| } | ||
| } | ||
| i += nodeDef.childCount; |
There was a problem hiding this comment.
add comment: This jumps to the next sibling
| @@ -0,0 +1,212 @@ | |||
| /** | |||
| [BindingType.ElementAttribute, 'a2', SecurityContext.NONE] | ||
| ]), | ||
| ], | ||
| (updater, view) => updater.checkDynamic(view, 0, ['v1', 'v2']))); |
There was a problem hiding this comment.
Refactor these tests into 1 test or a generic test that just takes the updater as argument
| expect(instance.b).toBe('v2'); | ||
| }); | ||
|
|
||
| it('should update via array', () => { |
| }); | ||
|
|
||
| describe('change text', () => { | ||
| it('should update inline', () => { |
|
|
||
| export function main() { | ||
| describe('viewDef', () => { | ||
| describe('reverseChild oreder', () => { |
| it('should reverse child order for mixed levels', () => { | ||
| const vd = viewDef(ViewFlags.None, [ | ||
| textDef(['a']), // level 0 | ||
| elementDef(NodeFlags.None, 5, 'span'), // level 1 |
There was a problem hiding this comment.
level 0, adjust others
There was a problem hiding this comment.
add indices to the comment
tbosch
force-pushed
the
view_manager
branch
2 times, most recently
from
3861e93
to
4653203
Compare
The new view engine allows our codegen to produce less code, as it can interpret view definitions during runtime. The view engine is not feature complete yet, but already allows to implement a tree benchmark based on it. Part of angular#14013
The new view engine allows our codegen to produce less code, as it can interpret view definitions during runtime. The view engine is not feature complete yet, but already allows to implement a tree benchmark based on it. Part of angular#14013
The new view engine allows our codegen to produce less code, as it can interpret view definitions during runtime. The view engine is not feature complete yet, but already allows to implement a tree benchmark based on it. Part of angular#14013
|
This issue has been automatically locked due to inactivity. Read more about our automatic conversation locking policy. This action has been performed automatically by a bot. |
The new view engine allows our codegen to produce less code,
as it can interpret view definitions during runtime.
The view engine is not feature complete yet, but already
allows to implement a tree benchmark based on it.
Part of #14013