Skip to content

Commit 79fc1e3

Browse files
tboschIgorMinar
authored andcommitted
fix(compiler): apply element bindings before host bindings (angular#14823)
1 parent 9402df9 commit 79fc1e3

2 files changed

Lines changed: 84 additions & 55 deletions

File tree

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

Lines changed: 49 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -33,7 +33,7 @@ export class ViewCompileResult {
3333
@CompilerInjectable()
3434
export class ViewCompiler {
3535
constructor(
36-
private _genConfigNext: CompilerConfig, private _schemaRegistryNext: ElementSchemaRegistry) {}
36+
private _genConfigNext: CompilerConfig, private _schemaRegistry: ElementSchemaRegistry) {}
3737

3838
compileComponent(
3939
component: CompileDirectiveMetadata, template: TemplateAst[], styles: o.Expression,
@@ -288,24 +288,23 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver, BuiltinConverter
288288
elName = null;
289289
}
290290

291-
const {flags, usedEvents, queryMatchesExpr, hostBindings, hostEvents} =
291+
const {flags, usedEvents, queryMatchesExpr, hostBindings: dirHostBindings, hostEvents} =
292292
this._visitElementOrTemplate(nodeIndex, ast);
293293

294294
let inputDefs: o.Expression[] = [];
295295
let outputDefs: o.Expression[] = [];
296296
if (elName) {
297-
ast.inputs.forEach(
298-
(inputAst) => hostBindings.push({context: COMP_VAR, value: inputAst.value}));
297+
const hostBindings = ast.inputs
298+
.map((inputAst) => ({
299+
context: COMP_VAR as o.Expression,
300+
value: inputAst.value,
301+
bindingDef: elementBindingDef(inputAst, null),
302+
}))
303+
.concat(dirHostBindings);
299304
if (hostBindings.length) {
300305
this._addUpdateExpressions(nodeIndex, hostBindings, this.updateRendererExpressions);
306+
inputDefs = hostBindings.map(entry => entry.bindingDef);
301307
}
302-
// Note: inputDefs have to be in the same order as hostBindings:
303-
// - first the entries from the directives, then the ones from the element.
304-
ast.directives.forEach(
305-
(dirAst, dirIndex) =>
306-
inputDefs.push(...elementBindingDefs(dirAst.hostProperties, dirAst)));
307-
inputDefs.push(...elementBindingDefs(ast.inputs, null));
308-
309308
outputDefs = usedEvents.map(
310309
([target, eventName]) => o.literalArr([o.literal(target), o.literal(eventName)]));
311310
}
@@ -355,7 +354,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver, BuiltinConverter
355354
flags: NodeFlags,
356355
usedEvents: [string, string][],
357356
queryMatchesExpr: o.Expression,
358-
hostBindings: {value: AST, context: o.Expression}[],
357+
hostBindings: {value: AST, context: o.Expression, bindingDef: o.Expression}[],
359358
hostEvents: {context: o.Expression, eventAst: BoundEventAst, dirAst: DirectiveAst}[],
360359
} {
361360
let flags = NodeFlags.None;
@@ -373,7 +372,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver, BuiltinConverter
373372
usedEvents.set(elementEventFullName(target, name), [target, name]);
374373
});
375374
});
376-
const hostBindings: {value: AST, context: o.Expression}[] = [];
375+
const hostBindings: {value: AST, context: o.Expression, bindingDef: o.Expression}[] = [];
377376
const hostEvents: {context: o.Expression, eventAst: BoundEventAst, dirAst: DirectiveAst}[] = [];
378377
const componentFactoryResolverProvider = createComponentFactoryResolver(ast.directives);
379378
if (componentFactoryResolverProvider) {
@@ -443,7 +442,7 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver, BuiltinConverter
443442
providerAst: ProviderAst, dirAst: DirectiveAst, directiveIndex: number,
444443
elementNodeIndex: number, refs: ReferenceAst[], queryMatches: QueryMatch[],
445444
usedEvents: Map<string, any>, queryIds: StaticAndDynamicQueryIds): {
446-
hostBindings: {value: AST, context: o.Expression}[],
445+
hostBindings: {value: AST, context: o.Expression, bindingDef: o.Expression}[],
447446
hostEvents: {context: o.Expression, eventAst: BoundEventAst, dirAst: DirectiveAst}[]
448447
} {
449448
const nodeIndex = this.nodeDefs.length;
@@ -513,14 +512,16 @@ class ViewBuilder implements TemplateAstVisitor, LocalResolver, BuiltinConverter
513512
const dirContextExpr = o.importExpr(createIdentifier(Identifiers.nodeValue)).callFn([
514513
VIEW_VAR, o.literal(nodeIndex)
515514
]);
516-
const hostBindings = dirAst.hostProperties.map((hostBindingAst) => {
517-
return {
518-
value: (<ASTWithSource>hostBindingAst.value).ast,
519-
context: dirContextExpr,
520-
};
521-
});
522-
const hostEvents = dirAst.hostEvents.map(
523-
(hostEventAst) => { return {context: dirContextExpr, eventAst: hostEventAst, dirAst}; });
515+
const hostBindings =
516+
dirAst.hostProperties.map((hostBindingAst) => ({
517+
value: (<ASTWithSource>hostBindingAst.value).ast,
518+
context: dirContextExpr,
519+
bindingDef: elementBindingDef(hostBindingAst, dirAst),
520+
}));
521+
const hostEvents = dirAst.hostEvents.map((hostEventAst) => ({
522+
context: dirContextExpr,
523+
eventAst: hostEventAst, dirAst,
524+
}));
524525

525526

526527
// directiveDef(
@@ -906,36 +907,32 @@ function lifecycleHookToNodeFlag(lifecycleHook: LifecycleHooks): NodeFlags {
906907
return nodeFlag;
907908
}
908909

909-
function elementBindingDefs(
910-
inputAsts: BoundElementPropertyAst[], dirAst: DirectiveAst): o.Expression[] {
911-
return inputAsts.map((inputAst) => {
912-
switch (inputAst.type) {
913-
case PropertyBindingType.Attribute:
914-
return o.literalArr([
915-
o.literal(BindingType.ElementAttribute), o.literal(inputAst.name),
916-
o.literal(inputAst.securityContext)
917-
]);
918-
case PropertyBindingType.Property:
919-
return o.literalArr([
920-
o.literal(BindingType.ElementProperty), o.literal(inputAst.name),
921-
o.literal(inputAst.securityContext)
922-
]);
923-
case PropertyBindingType.Animation:
924-
const bindingType = dirAst && dirAst.directive.isComponent ?
925-
BindingType.ComponentHostProperty :
926-
BindingType.ElementProperty;
927-
return o.literalArr([
928-
o.literal(bindingType), o.literal('@' + inputAst.name),
929-
o.literal(inputAst.securityContext)
930-
]);
931-
case PropertyBindingType.Class:
932-
return o.literalArr([o.literal(BindingType.ElementClass), o.literal(inputAst.name)]);
933-
case PropertyBindingType.Style:
934-
return o.literalArr([
935-
o.literal(BindingType.ElementStyle), o.literal(inputAst.name), o.literal(inputAst.unit)
936-
]);
937-
}
938-
});
910+
function elementBindingDef(inputAst: BoundElementPropertyAst, dirAst: DirectiveAst): o.Expression {
911+
switch (inputAst.type) {
912+
case PropertyBindingType.Attribute:
913+
return o.literalArr([
914+
o.literal(BindingType.ElementAttribute), o.literal(inputAst.name),
915+
o.literal(inputAst.securityContext)
916+
]);
917+
case PropertyBindingType.Property:
918+
return o.literalArr([
919+
o.literal(BindingType.ElementProperty), o.literal(inputAst.name),
920+
o.literal(inputAst.securityContext)
921+
]);
922+
case PropertyBindingType.Animation:
923+
const bindingType = dirAst && dirAst.directive.isComponent ?
924+
BindingType.ComponentHostProperty :
925+
BindingType.ElementProperty;
926+
return o.literalArr([
927+
o.literal(bindingType), o.literal('@' + inputAst.name), o.literal(inputAst.securityContext)
928+
]);
929+
case PropertyBindingType.Class:
930+
return o.literalArr([o.literal(BindingType.ElementClass), o.literal(inputAst.name)]);
931+
case PropertyBindingType.Style:
932+
return o.literalArr([
933+
o.literal(BindingType.ElementStyle), o.literal(inputAst.name), o.literal(inputAst.unit)
934+
]);
935+
}
939936
}
940937

941938

modules/@angular/core/test/linker/change_detection_integration_spec.ts

Lines changed: 35 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,11 +8,13 @@
88

99
import {ElementSchemaRegistry} from '@angular/compiler/src/schema/element_schema_registry';
1010
import {TEST_COMPILER_PROVIDERS} from '@angular/compiler/testing/test_bindings';
11-
import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, ChangeDetectionStrategy, ChangeDetectorRef, Component, DebugElement, Directive, DoCheck, Inject, Injectable, Input, OnChanges, OnDestroy, OnInit, Output, Pipe, PipeTransform, RenderComponentType, Renderer, RendererFactoryV2, RootRenderer, SimpleChange, SimpleChanges, TemplateRef, Type, ViewChild, ViewContainerRef, WrappedValue} from '@angular/core';
11+
import {AfterContentChecked, AfterContentInit, AfterViewChecked, AfterViewInit, ChangeDetectionStrategy, ChangeDetectorRef, Component, DebugElement, Directive, DoCheck, HostBinding, Inject, Injectable, Input, OnChanges, OnDestroy, OnInit, Output, Pipe, PipeTransform, RenderComponentType, Renderer, RendererFactoryV2, RootRenderer, SimpleChange, SimpleChanges, TemplateRef, Type, ViewChild, ViewContainerRef, WrappedValue} from '@angular/core';
1212
import {ComponentFixture, TestBed, fakeAsync} from '@angular/core/testing';
1313
import {By} from '@angular/platform-browser/src/dom/debug/by';
1414
import {getDOM} from '@angular/platform-browser/src/dom/dom_adapter';
15+
import {expect} from '@angular/platform-browser/testing/matchers';
1516

17+
import {DomElementSchemaRegistry} from '../../../compiler/index';
1618
import {MockSchemaRegistry} from '../../../compiler/testing/index';
1719
import {EventEmitter} from '../../src/facade/async';
1820

@@ -1241,6 +1243,36 @@ export function main() {
12411243
expect(renderLog.loggedValues).toEqual(['Tom']);
12421244
});
12431245
});
1246+
1247+
describe('class binding', () => {
1248+
it('should coordinate class attribute and class host binding', () => {
1249+
@Component({template: `<div class="{{initClasses}}" someDir></div>`})
1250+
class Comp {
1251+
initClasses = 'init';
1252+
}
1253+
1254+
@Directive({selector: '[someDir]'})
1255+
class SomeDir {
1256+
@HostBinding('class.foo')
1257+
fooClass = true;
1258+
}
1259+
1260+
const ctx =
1261+
TestBed
1262+
.configureCompiler({
1263+
providers:
1264+
[{provide: ElementSchemaRegistry, useExisting: DomElementSchemaRegistry}]
1265+
})
1266+
.configureTestingModule({declarations: [Comp, SomeDir]})
1267+
.createComponent(Comp);
1268+
1269+
ctx.detectChanges();
1270+
1271+
const divEl = ctx.debugElement.children[0];
1272+
expect(divEl.nativeElement).toHaveCssClass('init');
1273+
expect(divEl.nativeElement).toHaveCssClass('foo');
1274+
});
1275+
});
12441276
});
12451277
}
12461278

@@ -1285,13 +1317,13 @@ function patchLoggingRendererV2(rendererFactory: RendererFactoryV2, log: RenderL
12851317
const origSetValue = renderer.setValue;
12861318
renderer.setProperty = function(el: any, name: string, value: any): void {
12871319
log.setElementProperty(el, name, value);
1288-
origSetProperty.call(this, el, name, value);
1320+
origSetProperty.call(renderer, el, name, value);
12891321
};
12901322
renderer.setValue = function(node: any, value: string): void {
12911323
if (getDOM().isTextNode(node)) {
12921324
log.setText(node, value);
12931325
}
1294-
origSetValue.call(this, node, value);
1326+
origSetValue.call(renderer, node, value);
12951327
};
12961328
return renderer;
12971329
};

0 commit comments

Comments
 (0)