Skip to content

Commit

Permalink
fix(ivy): fix styling context resolution for host bindings on contain…
Browse files Browse the repository at this point in the history
…ers (angular#28221)

Previous to this change, the isStylingContext() function was improperly
returning true for LContainers because it used the presence of an array
at index 2 to determine whether it was a styling context. Unfortunately,
LContainers also contain arrays at index 2, so this would return a false
positive. This led to other errors down the line because we would treat
nodes with containers as if they already had styling contexts (even if
they did not), so the proper initialization logic for styling contexts
was not run.

This commit fixes the isStylingContext() function to use LCONTAINER_LENGTH
as a marker rather than the presence of an array, which in turn fixes
host bindings to styles on nodes with containers.

PR Close angular#28221
  • Loading branch information
kara authored and alxhub committed Jan 22, 2019
1 parent 7980f1d commit 058aafc
Show file tree
Hide file tree
Showing 2 changed files with 55 additions and 3 deletions.
4 changes: 2 additions & 2 deletions packages/core/src/render3/styling/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import '../../util/ng_dev_mode';

import {StyleSanitizeFn} from '../../sanitization/style_sanitizer';
import {getLContext} from '../context_discovery';
import {LContainer} from '../interfaces/container';
import {LCONTAINER_LENGTH, LContainer} from '../interfaces/container';
import {LContext} from '../interfaces/context';
import {AttributeMarker, TAttributes, TNode, TNodeFlags} from '../interfaces/node';
import {PlayState, Player, PlayerContext, PlayerIndex} from '../interfaces/player';
Expand Down Expand Up @@ -96,7 +96,7 @@ export function getStylingContext(index: number, viewData: LView): StylingContex
export function isStylingContext(value: any): value is StylingContext {
// Not an LView or an LContainer
return Array.isArray(value) && typeof value[StylingIndex.MasterFlagPosition] === 'number' &&
Array.isArray(value[StylingIndex.InitialStyleValuesPosition]);
value.length !== LCONTAINER_LENGTH;
}

export function isAnimationProp(name: string): boolean {
Expand Down
54 changes: 53 additions & 1 deletion packages/core/test/render3/host_binding_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
* found in the LICENSE file at https://angular.io/license
*/

import {ElementRef, QueryList} from '@angular/core';
import {ElementRef, QueryList, ViewContainerRef} from '@angular/core';

import {AttributeMarker, defineComponent, template, defineDirective, InheritDefinitionFeature, ProvidersFeature} from '../../src/render3/index';
import {allocHostVars, bind, directiveInject, element, elementAttribute, elementEnd, elementProperty, elementStyleProp, elementStyling, elementStylingApply, elementStart, listener, load, text, textBinding, loadQueryList, registerContentQuery, elementHostAttrs} from '../../src/render3/instructions';
Expand Down Expand Up @@ -1131,6 +1131,58 @@ describe('host bindings', () => {
expect(hostBindingEl.style.width).toEqual('5px');
});

it('should bind to host styles on containers', () => {
let hostBindingDir !: HostBindingToStyles;
/**
* host: {
* '[style.width.px]': 'width'
* }
*/
class HostBindingToStyles {
width = 2;

static ngDirectiveDef = defineDirective({
type: HostBindingToStyles,
selectors: [['', 'hostStyles', '']],
factory: () => hostBindingDir = new HostBindingToStyles(),
hostBindings: (rf: RenderFlags, ctx: HostBindingToStyles, elIndex: number) => {
if (rf & RenderFlags.Create) {
elementStyling(null, ['width'], null, ctx);
}
if (rf & RenderFlags.Update) {
elementStyleProp(0, 0, ctx.width, 'px', ctx);
elementStylingApply(0, ctx);
}
}
});
}

class ContainerDir {
constructor(public vcr: ViewContainerRef) {}

static ngDirectiveDef = defineDirective({
type: ContainerDir,
selectors: [['', 'containerDir', '']],
factory: () => new ContainerDir(directiveInject(ViewContainerRef as any)),
});
}

/** <div hostStyles containerDir></div> */
const App = createComponent('app', (rf: RenderFlags, ctx: any) => {
if (rf & RenderFlags.Create) {
element(0, 'div', ['containerDir', '', 'hostStyles', '']);
}
}, 1, 0, [ContainerDir, HostBindingToStyles]);

const fixture = new ComponentFixture(App);
const hostBindingEl = fixture.hostElement.querySelector('div') as HTMLElement;
expect(hostBindingEl.style.width).toEqual('2px');

hostBindingDir.width = 5;
fixture.update();
expect(hostBindingEl.style.width).toEqual('5px');
});

it('should apply static host classes', () => {
/**
* host: {
Expand Down

0 comments on commit 058aafc

Please sign in to comment.