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

test(ivy): add missing host binding and query tests #22213

Closed
wants to merge 6 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
1 change: 1 addition & 0 deletions packages/core/src/core_render3_private_export.ts
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ export {
e as ɵe,
p as ɵp,
pD as ɵpD,
a as ɵa,
s as ɵs,
t as ɵt,
v as ɵv,
Expand Down
3 changes: 2 additions & 1 deletion packages/core/src/render3/definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ export function defineComponent<T>(componentDefinition: ComponentDefArgs<T>): Co
tag: (componentDefinition as ComponentDefArgs<T>).tag || null !,
template: (componentDefinition as ComponentDefArgs<T>).template || null !,
h: componentDefinition.hostBindings || noop,
attributes: componentDefinition.attributes || null,
inputs: invertObject(componentDefinition.inputs),
outputs: invertObject(componentDefinition.outputs),
methods: invertObject(componentDefinition.methods),
Expand Down Expand Up @@ -177,4 +178,4 @@ export function definePipe<T>(
{type, factory, pure}: {type: Type<T>, factory: () => PipeTransform, pure?: boolean}):
PipeDef<T> {
throw new Error('TODO: implement!');
}
}
6 changes: 6 additions & 0 deletions packages/core/src/render3/instructions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -902,6 +902,12 @@ export function directiveCreate<T>(
diPublic(directiveDef !);
}

if (directiveDef !.attributes != null &&
(previousOrParentNode.flags & LNodeFlags.TYPE_MASK) == LNodeFlags.Element) {
setUpAttributes(
(previousOrParentNode as LElementNode).native, directiveDef !.attributes as string[]);
}

const tNode: TNode|null = previousOrParentNode.tNode !;
if (tNode && tNode.attrs) {
setInputsFromAttrs<T>(instance, directiveDef !.inputs, tNode);
Expand Down
9 changes: 9 additions & 0 deletions packages/core/src/render3/interfaces/definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,14 @@ export interface DirectiveDef<T> {
*/
h(directiveIndex: number, elementIndex: number): void;

/**
* Static attributes to set on host element.
*
* Even indices: attribute name
* Odd indices: attribute value
*/
attributes: string[]|null;

/* The following are lifecycle hooks for this component */
onInit: (() => void)|null;
doCheck: (() => void)|null;
Expand Down Expand Up @@ -140,6 +148,7 @@ export interface PipeDef<T> {
export interface DirectiveDefArgs<T> {
type: Type<T>;
factory: () => T | [T];
attributes?: string[];
Copy link
Contributor

Choose a reason for hiding this comment

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

This is public API and string[] is internal perf implementation. We should take input from user in form of {[key:string]:string} and convert it into [string, string,...] (just as we do with inputs and outputs)

inputs?: {[P in keyof T]?: string};
outputs?: {[P in keyof T]?: string};
methods?: {[P in keyof T]?: string};
Expand Down
222 changes: 210 additions & 12 deletions packages/core/test/render3/compiler_canonical_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 {Component, ContentChild, Directive, Injectable, Input, NgModule, OnDestroy, Optional, Pipe, PipeTransform, QueryList, SimpleChanges, TemplateRef, ViewChild, ViewContainerRef} from '../../src/core';
import {Component, ContentChild, ContentChildren, Directive, HostBinding, HostListener, Injectable, Input, NgModule, OnDestroy, Optional, Pipe, PipeTransform, QueryList, SimpleChanges, TemplateRef, ViewChild, ViewChildren, ViewContainerRef} from '../../src/core';
import * as $r3$ from '../../src/core_render3_private_export';

import {renderComponent, toHtml} from './render_util';
Expand Down Expand Up @@ -129,6 +129,192 @@ describe('compiler specification', () => {
expect(log).toEqual(['ChildComponent', 'SomeDirective']);
});

it('should support host bindings', () => {
type $MyApp$ = MyApp;
Copy link
Contributor

Choose a reason for hiding this comment

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

This demonstrates host bindings but not host attributes or host listeners.

For host attributes we should consider adding a create mode file the as this only has to be performed once.

Copy link
Contributor Author

@kara kara Feb 16, 2018

Choose a reason for hiding this comment

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

Okay, happy to add cases of both. By host attributes, I'm assuming you mean something like:

@HostBinding('attr.aria-label') label: string;

The runtime has been implemented for the above, but the bindings can change after the fact.

Unless you are talking about @Attibute() in the constructor? That won't change, but it hasn't been implemented yet in ivy, as far as I know, so probably outside the scope of this test PR.

Copy link
Contributor

Choose a reason for hiding this comment

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

Neither actually. They are odd as they can only appear in the host configuration field of a directive or component:

@Directive({
  ...
  host: {
    'foo': 'bar'
  }
})
...

This means to unconditionally always set the foo attribute of the host element to bar. This means that it only really needs to be done in create mode. This would imply a host binding similar to:

   hostBinding: (dirIndex, elIndex, cm) => {
     if (cm) {
       $r3$.ɵp(elIndex, 'foo', 'bar');
     }
   }

I currently generate these as:

   hostBinding: (dirIndex, elIndex, cm) => {
     $r3$.ɵp(elIndex, 'foo', 'bar');
   }

which constantly and unnecessarily sets these every refresh cycle. I could generate this as,

   hostBinding: (dirIndex, elIndex, cm) => {
     $r3$.ɵp(elIndex, 'foo', $r3$.ɵb('bar'));
   }

which saves the unnecessary DOM write at the cost of a slot. The cm parameter saves both.

Copy link
Contributor

Choose a reason for hiding this comment

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

One example use I found was material uses it to set the role attribute: https://github.com/angular/material2/blob/master/src/lib/select/select.ts#L194

Copy link
Contributor Author

@kara kara Feb 16, 2018

Choose a reason for hiding this comment

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

Ohhhh, I forgot about those! Thanks for clarifying. Seems like I could add a test for the HostBinding attr above and also for host attributes, so we have full coverage.

In the case above, it seems like we should probably use the a instruction (p calls setElementProperty), but in the style of creation-time bindings as you suggest. I'll play around with it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Actually, thinking about it more, I think we should be able to put the instruction in factory. That way, the attribute is set with other attributes on the element, the instruction does not need to re-run every CD, and we can avoid the code size hit of passing in cm for every comp.


@Directive({selector: '[hostBindingDir]'})
class HostBindingDir {
@HostBinding('id') dirId = 'some id';

// NORMATIVE
static ngDirectiveDef = $r3$.ɵdefineDirective({
type: HostBindingDir,
factory: function HostBindingDir_Factory() { return new HostBindingDir(); },
hostBindings: function HostBindingDir_HostBindings(
dirIndex: $number$, elIndex: $number$) {
$r3$.ɵp(elIndex, 'id', $r3$.ɵb($r3$.ɵm<HostBindingDir>(dirIndex).dirId));
}
});
// /NORMATIVE
}

const $e0_attrs$ = ['hostBindingDir', ''];
const $e0_dirs$ = [HostBindingDir];

@Component({
selector: 'my-app',
template: `
<div hostBindingDir></div>
`
})
class MyApp {
static ngComponentDef = $r3$.ɵdefineComponent({
type: MyApp,
tag: 'my-app',
factory: function MyApp_Factory() { return new MyApp(); },
template: function MyApp_Template(ctx: $MyApp$, cm: $boolean$) {
if (cm) {
$r3$.ɵE(0, 'div', $e0_attrs$, $e0_dirs$);
$r3$.ɵe();
}
HostBindingDir.ngDirectiveDef.h(1, 0);
$r3$.ɵr(1, 0);
}
});
}

expect(renderComp(MyApp)).toEqual(`<div hostbindingdir="" id="some id"></div>`);
});

it('should support host listeners', () => {
type $MyApp$ = MyApp;

@Directive({selector: '[hostlistenerDir]'})
class HostListenerDir {
@HostListener('click')
onClick() {}

// NORMATIVE
static ngDirectiveDef = $r3$.ɵdefineDirective({
type: HostListenerDir,
factory: function HostListenerDir_Factory() {
const $dir$ = new HostListenerDir();
$r3$.ɵL('click', function HostListenerDir_click_Handler(event) { $dir$.onClick(); });
return $dir$;
},
});
// /NORMATIVE
}

const $e0_attrs$ = ['hostListenerDir', ''];
const $e0_dirs$ = [HostListenerDir];

@Component({
selector: 'my-app',
template: `
<button hostListenerDir>Click</button>
`
})
class MyApp {
static ngComponentDef = $r3$.ɵdefineComponent({
type: MyApp,
tag: 'my-app',
factory: function MyApp_Factory() { return new MyApp(); },
template: function MyApp_Template(ctx: $MyApp$, cm: $boolean$) {
if (cm) {
$r3$.ɵE(0, 'button', $e0_attrs$, $e0_dirs$);
$r3$.ɵT(2, 'Click');
$r3$.ɵe();
}
HostListenerDir.ngDirectiveDef.h(1, 0);
$r3$.ɵr(1, 0);
}
});
}

expect(renderComp(MyApp)).toEqual(`<button hostlistenerdir="">Click</button>`);
});


it('should support setting of host attributes', () => {
type $MyApp$ = MyApp;

@Directive({selector: '[hostAttributeDir]', host: {'role': 'listbox'}})
class HostAttributeDir {
// NORMATIVE
static ngDirectiveDef = $r3$.ɵdefineDirective({
type: HostAttributeDir,
factory: function HostAttributeDir_Factory() { return new HostAttributeDir(); },
attributes: ['role', 'listbox']
});
// /NORMATIVE
}

const $e0_attrs$ = ['hostAttributeDir', ''];
const $e0_dirs$ = [HostAttributeDir];

@Component({
selector: 'my-app',
template: `
<div hostAttributeDir></div>
`
})
class MyApp {
static ngComponentDef = $r3$.ɵdefineComponent({
type: MyApp,
tag: 'my-app',
factory: function MyApp_Factory() { return new MyApp(); },
template: function MyApp_Template(ctx: $MyApp$, cm: $boolean$) {
if (cm) {
$r3$.ɵE(0, 'div', $e0_attrs$, $e0_dirs$);
$r3$.ɵe();
}
HostAttributeDir.ngDirectiveDef.h(1, 0);
$r3$.ɵr(1, 0);
}
});
}

expect(renderComp(MyApp)).toEqual(`<div hostattributedir="" role="listbox"></div>`);
});

it('should support bindings of host attributes', () => {
type $MyApp$ = MyApp;

@Directive({selector: '[hostBindingDir]'})
class HostBindingDir {
@HostBinding('attr.aria-label') label = 'some label';

// NORMATIVE
static ngDirectiveDef = $r3$.ɵdefineDirective({
type: HostBindingDir,
factory: function HostBindingDir_Factory() { return new HostBindingDir(); },
hostBindings: function HostBindingDir_HostBindings(
dirIndex: $number$, elIndex: $number$) {
$r3$.ɵa(elIndex, 'aria-label', $r3$.ɵb($r3$.ɵm<HostBindingDir>(dirIndex).label));
}
});
// /NORMATIVE
}

const $e0_attrs$ = ['hostBindingDir', ''];
const $e0_dirs$ = [HostBindingDir];

@Component({
selector: 'my-app',
template: `
<div hostBindingDir></div>
`
})
class MyApp {
static ngComponentDef = $r3$.ɵdefineComponent({
type: MyApp,
tag: 'my-app',
factory: function MyApp_Factory() { return new MyApp(); },
template: function MyApp_Template(ctx: $MyApp$, cm: $boolean$) {
if (cm) {
$r3$.ɵE(0, 'div', $e0_attrs$, $e0_dirs$);
$r3$.ɵe();
}
HostBindingDir.ngDirectiveDef.h(1, 0);
$r3$.ɵr(1, 0);
}
});
}

expect(renderComp(MyApp)).toEqual(`<div aria-label="some label" hostbindingdir=""></div>`);
});

xit('should support structural directives', () => {
type $MyComponent$ = MyComponent;

Expand Down Expand Up @@ -622,7 +808,7 @@ describe('compiler specification', () => {
})
class ViewQueryComponent {
@ViewChild(SomeDirective) someDir: SomeDirective;

@ViewChildren(SomeDirective) someDirList: QueryList<SomeDirective>;

// NORMATIVE
static ngComponentDef = $r3$.ɵdefineComponent({
Expand All @@ -634,21 +820,27 @@ describe('compiler specification', () => {
let $tmp$: any;
if (cm) {
$r3$.ɵQ(0, SomeDirective, false);
$r3$.ɵE(1, 'div', $e1_attrs$, $e1_dirs$);
$r3$.ɵQ(1, SomeDirective, false);
$r3$.ɵE(2, 'div', $e1_attrs$, $e1_dirs$);
$r3$.ɵe();
}
$r3$.ɵqR($tmp$ = $r3$.ɵm<QueryList<any>>(0)) &&
(ctx.someDir = $tmp$ as QueryList<any>);
SomeDirective.ngDirectiveDef.h(2, 1);
$r3$.ɵr(2, 1);

$r3$.ɵqR($tmp$ = $r3$.ɵm<QueryList<any>>(0)) && (ctx.someDir = $tmp$.first);
$r3$.ɵqR($tmp$ = $r3$.ɵm<QueryList<any>>(1)) &&
(ctx.someDirList = $tmp$ as QueryList<any>);
SomeDirective.ngDirectiveDef.h(3, 2);
$r3$.ɵr(3, 2);
}
});
// /NORMATIVE
}


const viewQueryComp = renderComponent(ViewQueryComponent);
expect((viewQueryComp.someDir as QueryList<SomeDirective>).toArray()).toEqual([someDir !]);
expect(viewQueryComp.someDir).toEqual(someDir);
expect((viewQueryComp.someDirList as QueryList<SomeDirective>).toArray()).toEqual([
someDir !
]);
});

it('should support content queries', () => {
Expand All @@ -665,19 +857,24 @@ describe('compiler specification', () => {
})
class ContentQueryComponent {
@ContentChild(SomeDirective) someDir: SomeDirective;
@ContentChildren(SomeDirective) someDirList: QueryList<SomeDirective>;

// NORMATIVE
static ngComponentDef = $r3$.ɵdefineComponent({
type: ContentQueryComponent,
tag: 'content-query-component',
factory: function ContentQueryComponent_Factory() {
return [new ContentQueryComponent(), $r3$.ɵQ(null, SomeDirective, false)];
return [
new ContentQueryComponent(), $r3$.ɵQ(null, SomeDirective, false),
$r3$.ɵQ(null, SomeDirective, false)
];
},
hostBindings: function ContentQueryComponent_HostBindings(
dirIndex: $number$, elIndex: $number$) {
let $tmp$: any;
$r3$.ɵqR($tmp$ = $r3$.ɵm<any[]>(dirIndex)[1]) &&
($r3$.ɵm<any[]>(dirIndex)[0].someDir = $tmp$);
const $instance$ = $r3$.ɵm<any[]>(dirIndex)[0];
$r3$.ɵqR($tmp$ = $r3$.ɵm<any[]>(dirIndex)[1]) && ($instance$.someDir = $tmp$.first);
$r3$.ɵqR($tmp$ = $r3$.ɵm<any[]>(dirIndex)[2]) && ($instance$.someDirList = $tmp$);
},
template: function ContentQueryComponent_Template(
ctx: $ContentQueryComponent$, cm: $boolean$) {
Expand Down Expand Up @@ -730,7 +927,8 @@ describe('compiler specification', () => {
expect(renderComp(MyApp))
.toEqual(
`<content-query-component><div><div somedir=""></div></div></content-query-component>`);
expect((contentQueryComp !.someDir as QueryList<SomeDirective>).toArray()).toEqual([
expect(contentQueryComp !.someDir).toEqual(someDir !);
expect((contentQueryComp !.someDirList as QueryList<SomeDirective>).toArray()).toEqual([
someDir !
]);
});
Expand Down
37 changes: 36 additions & 1 deletion packages/core/test/render3/integration_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 {defineComponent} from '../../src/render3/index';
import {defineComponent, defineDirective} from '../../src/render3/index';
import {NO_CHANGE, bind, componentRefresh, container, containerRefreshEnd, containerRefreshStart, elementAttribute, elementClass, elementEnd, elementProperty, elementStart, elementStyle, embeddedViewEnd, embeddedViewStart, interpolation1, interpolation2, interpolation3, interpolation4, interpolation5, interpolation6, interpolation7, interpolation8, interpolationV, memory, projection, projectionDef, text, textBinding} from '../../src/render3/instructions';

import {containerEl, renderToHtml} from './render_util';
Expand Down Expand Up @@ -664,6 +664,41 @@ describe('render3 integration test', () => {
expect(renderToHtml(Template, ctx))
.toEqual('<span title="Hello"><b title="Goodbye"></b></span>');
});

it('should support host attribute bindings', () => {
let hostBindingDir: HostBindingDir;

class HostBindingDir {
/* @HostBinding('attr.aria-label') */
label = 'some label';

static ngDirectiveDef = defineDirective({
type: HostBindingDir,
factory: function HostBindingDir_Factory() {
return hostBindingDir = new HostBindingDir();
},
hostBindings: function HostBindingDir_HostBindings(dirIndex: number, elIndex: number) {
elementAttribute(elIndex, 'aria-label', bind(memory<HostBindingDir>(dirIndex).label));
}
});
}

function Template(ctx: any, cm: boolean) {
if (cm) {
elementStart(0, 'div', ['hostBindingDir', ''], [HostBindingDir]);
elementEnd();
}
HostBindingDir.ngDirectiveDef.h(1, 0);
componentRefresh(1, 0);
}

expect(renderToHtml(Template, {}))
.toEqual(`<div aria-label="some label" hostbindingdir=""></div>`);

hostBindingDir !.label = 'other label';
expect(renderToHtml(Template, {}))
.toEqual(`<div aria-label="other label" hostbindingdir=""></div>`);
});
});

describe('elementStyle', () => {
Expand Down