Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(ivy): ensure host bindings and host styling works on a root component #28664

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
2 changes: 1 addition & 1 deletion integration/_payload-limits.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
"master": {
"uncompressed": {
"runtime": 1440,
"main": 12708,
"main": 12885,
"polyfills": 38390
}
}
Expand Down
11 changes: 6 additions & 5 deletions packages/core/src/render3/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,14 +18,14 @@ import {getComponentDef} from './definition';
import {diPublicInInjector, getOrCreateNodeInjectorForNode} from './di';
import {publishDefaultGlobalUtils} from './global_utils';
import {registerPostOrderHooks, registerPreOrderHooks} from './hooks';
import {addToViewTree, CLEAN_PROMISE, createLView, createNodeAtIndex, createTNode, createTView, getOrCreateTView, initNodeFlags, instantiateRootComponent, locateHostElement, queueComponentIndexForCheck, refreshDescendantViews,} from './instructions';
import {CLEAN_PROMISE, addToViewTree, createLView, createNodeAtIndex, createTNode, createTView, getOrCreateTView, initNodeFlags, instantiateRootComponent, invokeHostBindingsInCreationMode, locateHostElement, queueComponentIndexForCheck, refreshDescendantViews} from './instructions';
import {ComponentDef, ComponentType, RenderFlags} from './interfaces/definition';
import {TElementNode, TNode, TNodeFlags, TNodeType} from './interfaces/node';
import {PlayerHandler} from './interfaces/player';
import {RElement, Renderer3, RendererFactory3, domRendererFactory3} from './interfaces/renderer';
import {CONTEXT, FLAGS, HEADER_OFFSET, HOST, LView, LViewFlags, RootContext, RootContextFlags, TVIEW, T_HOST} from './interfaces/view';
import {enterView, getPreviousOrParentTNode, leaveView, resetComponentState, setCurrentDirectiveDef} from './state';
import {defaultScheduler, getRootView, readPatchedLView, renderStringify} from './util';
import {applyOnCreateInstructions, defaultScheduler, getRootView, readPatchedLView, renderStringify} from './util';



Expand Down Expand Up @@ -200,9 +200,10 @@ export function createRootComponent<T>(

if (tView.firstTemplatePass && componentDef.hostBindings) {
const rootTNode = getPreviousOrParentTNode();
setCurrentDirectiveDef(componentDef);
componentDef.hostBindings(RenderFlags.Create, component, rootTNode.index - HEADER_OFFSET);
setCurrentDirectiveDef(null);
const expando = tView.expandoInstructions !;
invokeHostBindingsInCreationMode(
componentDef, expando, component, rootTNode, tView.firstTemplatePass);
rootTNode.onElementCreationFns && applyOnCreateInstructions(rootTNode);
}

return component;
Expand Down
47 changes: 22 additions & 25 deletions packages/core/src/render3/instructions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import {CssSelectorList, NG_PROJECT_AS_ATTR_NAME} from './interfaces/projection'
import {LQueries} from './interfaces/query';
import {GlobalTargetResolver, ProceduralRenderer3, RComment, RElement, RText, Renderer3, RendererFactory3, isProceduralRenderer} from './interfaces/renderer';
import {SanitizerFn} from './interfaces/sanitization';
import {BINDING_INDEX, CLEANUP, CONTAINER_INDEX, CONTEXT, DECLARATION_VIEW, FLAGS, HEADER_OFFSET, HOST, INJECTOR, InitPhaseState, LView, LViewFlags, NEXT, OpaqueViewState, PARENT, QUERIES, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, TAIL, TData, TVIEW, TView, T_HOST} from './interfaces/view';
import {BINDING_INDEX, CLEANUP, CONTAINER_INDEX, CONTEXT, DECLARATION_VIEW, ExpandoInstructions, FLAGS, HEADER_OFFSET, HOST, INJECTOR, InitPhaseState, LView, LViewFlags, NEXT, OpaqueViewState, PARENT, QUERIES, RENDERER, RENDERER_FACTORY, RootContext, RootContextFlags, SANITIZER, TAIL, TData, TVIEW, TView, T_HOST} from './interfaces/view';
import {assertNodeOfPossibleTypes, assertNodeType} from './node_assert';
import {appendChild, appendProjectedNode, createTextNode, getLViewChild, insertView, removeView} from './node_manipulation';
import {isNodeMatchingSelectorList, matchingSelectorIndex} from './node_selector_matcher';
Expand All @@ -41,7 +41,7 @@ import {getInitialClassNameValue, getInitialStyleStringValue, initializeStaticCo
import {BoundPlayerFactory} from './styling/player_factory';
import {ANIMATION_PROP_PREFIX, allocateDirectiveIntoContext, createEmptyStylingContext, forceClassesAsString, forceStylesAsString, getStylingContext, hasClassInput, hasStyleInput, hasStyling, isAnimationProp} from './styling/util';
import {NO_CHANGE} from './tokens';
import {INTERPOLATION_DELIMITER, findComponentView, getComponentViewByIndex, getNativeByIndex, getNativeByTNode, getRootContext, getRootView, getTNode, isComponent, isComponentDef, isContentQueryHost, isRootView, loadInternal, readElementValue, readPatchedLView, renderStringify} from './util';
import {INTERPOLATION_DELIMITER, applyOnCreateInstructions, findComponentView, getComponentViewByIndex, getNativeByIndex, getNativeByTNode, getRootContext, getRootView, getTNode, isComponent, isComponentDef, isContentQueryHost, isRootView, loadInternal, readElementValue, readPatchedLView, renderStringify} from './util';



Expand Down Expand Up @@ -1079,18 +1079,9 @@ export function elementEnd(): void {
setPreviousOrParentTNode(previousOrParentTNode);
}

// there may be some instructions that need to run in a specific
// order because the CREATE block in a directive runs before the
// CREATE block in a template. To work around this instructions
// can get access to the function array below and defer any code
// to run after the element is created.
let fns: Function[]|null;
if (fns = previousOrParentTNode.onElementCreationFns) {
for (let i = 0; i < fns.length; i++) {
fns[i]();
}
previousOrParentTNode.onElementCreationFns = null;
}
// this is required for all host-level styling-related instructions to run
// in the correct order
previousOrParentTNode.onElementCreationFns && applyOnCreateInstructions(previousOrParentTNode);

ngDevMode && assertNodeType(previousOrParentTNode, TNodeType.Element);
const lView = getLView();
Expand Down Expand Up @@ -1815,23 +1806,29 @@ function invokeDirectivesHostBindings(tView: TView, viewData: LView, tNode: TNod
const def = tView.data[i] as DirectiveDef<any>;
const directive = viewData[i];
if (def.hostBindings) {
const previousExpandoLength = expando.length;
setCurrentDirectiveDef(def);
def.hostBindings !(RenderFlags.Create, directive, tNode.index - HEADER_OFFSET);
setCurrentDirectiveDef(null);
// `hostBindings` function may or may not contain `allocHostVars` call
// (e.g. it may not if it only contains host listeners), so we need to check whether
// `expandoInstructions` has changed and if not - we still push `hostBindings` to
// expando block, to make sure we execute it for DI cycle
if (previousExpandoLength === expando.length && firstTemplatePass) {
expando.push(def.hostBindings);
}
invokeHostBindingsInCreationMode(def, expando, directive, tNode, firstTemplatePass);
} else if (firstTemplatePass) {
expando.push(null);
}
}
}

export function invokeHostBindingsInCreationMode(
def: DirectiveDef<any>, expando: ExpandoInstructions, directive: any, tNode: TNode,
firstTemplatePass: boolean) {
const previousExpandoLength = expando.length;
setCurrentDirectiveDef(def);
def.hostBindings !(RenderFlags.Create, directive, tNode.index - HEADER_OFFSET);
setCurrentDirectiveDef(null);
// `hostBindings` function may or may not contain `allocHostVars` call
// (e.g. it may not if it only contains host listeners), so we need to check whether
// `expandoInstructions` has changed and if not - we still push `hostBindings` to
// expando block, to make sure we execute it for DI cycle
if (previousExpandoLength === expando.length && firstTemplatePass) {
expando.push(def.hostBindings);
}
}

/**
* Generates a new block in TView.expandoInstructions for this node.
*
Expand Down
9 changes: 8 additions & 1 deletion packages/core/src/render3/interfaces/view.ts
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,13 @@ export const enum InitPhaseState {
InitPhaseCompleted = 0b11,
}

/**
* Set of instructions used to process host bindings efficiently.
*
* See VIEW_DATA.md for more information.
*/
export interface ExpandoInstructions extends Array<number|HostBindingsFunction<any>|null> {}

/**
* The static data for an LView (shared between all templates of a
* given type).
Expand Down Expand Up @@ -401,7 +408,7 @@ export interface TView {
*
* See VIEW_DATA.md for more information.
*/
expandoInstructions: (number|HostBindingsFunction<any>|null)[]|null;
expandoInstructions: ExpandoInstructions|null;

/**
* Full registry of directives and components that may be found in this view.
Expand Down
12 changes: 8 additions & 4 deletions packages/core/src/render3/styling/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ import {LContext} from '../interfaces/context';
import {AttributeMarker, TAttributes, TNode, TNodeFlags} from '../interfaces/node';
import {PlayState, Player, PlayerContext, PlayerIndex} from '../interfaces/player';
import {RElement} from '../interfaces/renderer';
import {InitialStylingValues, StylingContext, StylingFlags, StylingIndex} from '../interfaces/styling';
import {InitialStylingValues, InitialStylingValuesIndex, StylingContext, StylingFlags, StylingIndex} from '../interfaces/styling';
import {HEADER_OFFSET, HOST, LView, RootContext} from '../interfaces/view';
import {getTNode} from '../util';

Expand Down Expand Up @@ -100,10 +100,14 @@ export function getStylingContext(index: number, viewData: LView): StylingContex
}
}

export function isStylingContext(value: any): value is StylingContext {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why change the return type?

export function isStylingContext(value: any): boolean {
// Not an LView or an LContainer
return Array.isArray(value) && typeof value[StylingIndex.MasterFlagPosition] === 'number' &&
value.length !== LCONTAINER_LENGTH;
if (Array.isArray(value) && value.length >= StylingIndex.SingleStylesStartPosition) {
return typeof value[StylingIndex.MasterFlagPosition] === 'number' &&
value[StylingIndex.InitialClassValuesPosition]
[InitialStylingValuesIndex.DefaultNullValuePosition] === null;
}
return false;
}

export function isAnimationProp(name: string): boolean {
Expand Down
15 changes: 15 additions & 0 deletions packages/core/src/render3/util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -317,3 +317,18 @@ export const INTERPOLATION_DELIMITER = `�`;
export function isPropMetadataString(str: string): boolean {
return str.indexOf(INTERPOLATION_DELIMITER) >= 0;
}

export function applyOnCreateInstructions(tNode: TNode) {
// there may be some instructions that need to run in a specific
// order because the CREATE block in a directive runs before the
// CREATE block in a template. To work around this instructions
// can get access to the function array below and defer any code
// to run after the element is created.
let fns: Function[]|null;
if (fns = tNode.onElementCreationFns) {
for (let i = 0; i < fns.length; i++) {
fns[i]();
}
tNode.onElementCreationFns = null;
}
}
43 changes: 43 additions & 0 deletions packages/core/test/acceptance/integration_spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
/**
* @license
* Copyright Google Inc. All Rights Reserved.
*
* Use of this source code is governed by an MIT-style license that can be
* found in the LICENSE file at https://angular.io/license
*/
import {Component, HostBinding} from '@angular/core';
import {TestBed} from '@angular/core/testing';
import {expect} from '@angular/platform-browser/testing/src/matchers';
import {onlyInIvy} from '@angular/private/testing';

describe('acceptance integration tests', () => {
onlyInIvy('[style] and [class] bindings are a new feature')
.it('should render host bindings on the root component', () => {
@Component({template: '...'})
class MyApp {
@HostBinding('style') public myStylesExp = {};
@HostBinding('class') public myClassesExp = {};
}

TestBed.configureTestingModule({declarations: [MyApp]});
const fixture = TestBed.createComponent(MyApp);
const element = fixture.nativeElement;
fixture.detectChanges();

const component = fixture.componentInstance;
component.myStylesExp = {width: '100px'};
component.myClassesExp = 'foo';
fixture.detectChanges();

expect(element.style['width']).toEqual('100px');
expect(element.classList.contains('foo')).toBeTruthy();

component.myStylesExp = {width: '200px'};
component.myClassesExp = 'bar';
fixture.detectChanges();

expect(element.style['width']).toEqual('200px');
expect(element.classList.contains('foo')).toBeFalsy();
expect(element.classList.contains('bar')).toBeTruthy();
});
});
9 changes: 7 additions & 2 deletions packages/core/test/bundling/animation_world/base.css
Original file line number Diff line number Diff line change
Expand Up @@ -137,5 +137,10 @@ hr {

.learn-bar > .learn {
left: 8px;
}
}
}
}

.border {
outline:2px solid maroon;
display:block;
}
4 changes: 3 additions & 1 deletion packages/core/test/bundling/animation_world/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,11 +91,13 @@ class BoxWithOverriddenStylesComponent {

<box-with-overridden-styles
style="display:block"
[style]="{'border-radius':'50px', 'border': '50px solid teal'}" [ngStyle]="{transform:'rotate(50deg)'}">
[style]="{'border-radius':'50px', 'border': '50px solid teal'}">
</box-with-overridden-styles>
`,
})
class AnimationWorldComponent {
@HostBinding('class') classVal = 'border';

items: any[] = [
{value: 1, active: false}, {value: 2, active: false}, {value: 3, active: false},
{value: 4, active: false}, {value: 5, active: false}, {value: 6, active: false},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,6 @@
{
"name": "INJECTOR_BLOOM_PARENT_SIZE"
},
{
"name": "LCONTAINER_LENGTH"
},
{
"name": "MONKEY_PATCH_KEY_NAME"
},
Expand Down Expand Up @@ -170,6 +167,9 @@
{
"name": "appendChild"
},
{
"name": "applyOnCreateInstructions"
},
{
"name": "attachPatchData"
},
Expand Down Expand Up @@ -449,6 +449,9 @@
{
"name": "invokeDirectivesHostBindings"
},
{
"name": "invokeHostBindingsInCreationMode"
},
{
"name": "isAnimationProp"
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -131,6 +131,9 @@
{
"name": "appendChild"
},
{
"name": "applyOnCreateInstructions"
},
{
"name": "attachPatchData"
},
Expand Down Expand Up @@ -320,6 +323,9 @@
{
"name": "invertObject"
},
{
"name": "invokeHostBindingsInCreationMode"
},
{
"name": "isComponentDef"
},
Expand Down
6 changes: 6 additions & 0 deletions packages/core/test/bundling/todo/bundle.golden_symbols.json
Original file line number Diff line number Diff line change
Expand Up @@ -386,6 +386,9 @@
{
"name": "appendChild"
},
{
"name": "applyOnCreateInstructions"
},
{
"name": "assertTemplate"
},
Expand Down Expand Up @@ -911,6 +914,9 @@
{
"name": "invokeDirectivesHostBindings"
},
{
"name": "invokeHostBindingsInCreationMode"
},
{
"name": "isAnimationProp"
},
Expand Down
6 changes: 3 additions & 3 deletions packages/core/test/render3/render_util.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ import {extractDirectiveDef, extractPipeDef} from '../../src/render3/definition'
import {NG_ELEMENT_ID} from '../../src/render3/fields';
import {ComponentTemplate, ComponentType, DirectiveDef, DirectiveType, ProvidersFeature, RenderFlags, defineComponent, defineDirective, renderComponent as _renderComponent, tick} from '../../src/render3/index';
import {renderTemplate} from '../../src/render3/instructions';
import {DirectiveDefList, DirectiveTypesOrFactory, PipeDef, PipeDefList, PipeTypesOrFactory} from '../../src/render3/interfaces/definition';
import {DirectiveDefList, DirectiveTypesOrFactory, HostBindingsFunction, PipeDef, PipeDefList, PipeTypesOrFactory} from '../../src/render3/interfaces/definition';
import {PlayerHandler} from '../../src/render3/interfaces/player';
import {ProceduralRenderer3, RComment, RElement, RNode, RText, Renderer3, RendererFactory3, RendererStyleFlags3, domRendererFactory3} from '../../src/render3/interfaces/renderer';
import {HEADER_OFFSET, LView} from '../../src/render3/interfaces/view';
Expand Down Expand Up @@ -314,7 +314,7 @@ export function createComponent(
name: string, template: ComponentTemplate<any>, consts: number = 0, vars: number = 0,
directives: DirectiveTypesOrFactory = [], pipes: PipeTypesOrFactory = [],
viewQuery: ComponentTemplate<any>| null = null, providers: Provider[] = [],
viewProviders: Provider[] = []): ComponentType<any> {
viewProviders: Provider[] = [], hostBindings?: HostBindingsFunction<any>): ComponentType<any> {
return class Component {
value: any;
static ngComponentDef = defineComponent({
Expand All @@ -325,7 +325,7 @@ export function createComponent(
factory: () => new Component,
template: template,
viewQuery: viewQuery,
directives: directives,
directives: directives, hostBindings,
pipes: pipes,
features: (providers.length > 0 || viewProviders.length > 0)?
[ProvidersFeature(providers || [], viewProviders || [])]: []
Expand Down