Skip to content

Commit

Permalink
refactor(ivy): remove type from DirectiveDef (angular#21374)
Browse files Browse the repository at this point in the history
This change makes the code cleaner for the user. It does mean
a little bit more work for us since we have to patch the `type` back
into the `DirectiveDef`. However since the patching happens only once
on startup it should not be significant.

PR Close angular#21374
  • Loading branch information
mhevery authored and alexeagle committed Jan 11, 2018
1 parent 16232f0 commit 5eaaac3
Show file tree
Hide file tree
Showing 23 changed files with 108 additions and 212 deletions.
5 changes: 1 addition & 4 deletions modules/benchmarks/src/largetable/render3/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,16 +16,13 @@ export class LargeTableComponent {

/** @nocollapse */
static ngComponentDef: ComponentDef<LargeTableComponent> = defineComponent({
type: LargeTableComponent,
tag: 'largetable',
template: function(ctx: LargeTableComponent, cm: boolean) {
if (cm) {
E(0, 'table');
{
E(1, 'tbody');
{
C(2);
}
{ C(2); }
e();
}
e();
Expand Down
2 changes: 0 additions & 2 deletions modules/benchmarks/src/tree/render3/tree.ts
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ export class TreeComponent {

/** @nocollapse */
static ngComponentDef: ComponentDef<TreeComponent> = defineComponent({
type: TreeComponent,
tag: 'tree',
template: function(ctx: TreeComponent, cm: boolean) {
if (cm) {
Expand Down Expand Up @@ -93,7 +92,6 @@ export class TreeFunction extends TreeComponent {

/** @nocollapse */
static ngComponentDef: ComponentDef<TreeFunction> = defineComponent({
type: TreeFunction,
tag: 'tree',
template: function(ctx: TreeFunction, cm: boolean) {
// bit of a hack
Expand Down
10 changes: 6 additions & 4 deletions packages/core/src/render3/component.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,13 +13,14 @@ import {ComponentRef as viewEngine_ComponentRef} from '../linker/component_facto
import {EmbeddedViewRef as viewEngine_EmbeddedViewRef} from '../linker/view_ref';

import {assertNotNull} from './assert';
import {NG_HOST_SYMBOL, createError, createLView, directive, enterView, hostElement, leaveView, locateHostElement, renderComponentOrTemplate, directiveCreate} from './instructions';
import {ComponentDef, ComponentType} from './interfaces/definition';
import {NG_HOST_SYMBOL, createError, createLView, directive, directiveCreate, enterView, hostElement, leaveView, locateHostElement, renderComponentOrTemplate} from './instructions';
import {ComponentDef, ComponentType, TypedComponentDef} from './interfaces/definition';
import {LElementNode} from './interfaces/node';
import {RElement, RendererFactory3, domRendererFactory3} from './interfaces/renderer';
import {RElement, Renderer3, RendererFactory3, domRendererFactory3} from './interfaces/renderer';
import {notImplemented, stringify} from './util';



/** Options that control how the component should be bootstrapped. */
export interface CreateComponentOptions {
/** Which renderer factory to use. */
Expand Down Expand Up @@ -165,7 +166,8 @@ export const NULL_INJECTOR: Injector = {
export function renderComponent<T>(
componentType: ComponentType<T>, opts: CreateComponentOptions = {}): T {
const rendererFactory = opts.rendererFactory || domRendererFactory3;
const componentDef = componentType.ngComponentDef;
const componentDef = componentType.ngComponentDef as TypedComponentDef<T>;
if (componentDef.type != componentType) componentDef.type = componentType;
let component: T;
const hostNode = locateHostElement(rendererFactory, opts.host || componentDef.tag);
const oldView = enterView(
Expand Down
1 change: 0 additions & 1 deletion packages/core/src/render3/definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ import {ComponentDef, ComponentDefArgs, DirectiveDef, DirectiveDefArgs} from './
*/
export function defineComponent<T>(componentDefinition: ComponentDefArgs<T>): ComponentDef<T> {
const def = <ComponentDef<any>>{
type: componentDefinition.type,
diPublic: null,
n: componentDefinition.factory,
tag: (componentDefinition as ComponentDefArgs<T>).tag || null !,
Expand Down
6 changes: 3 additions & 3 deletions packages/core/src/render3/di.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ import {ViewContainerRef as viewEngine_ViewContainerRef} from '../linker/view_co
import {EmbeddedViewRef as viewEngine_EmbeddedViewRef, ViewRef as viewEngine_ViewRef} from '../linker/view_ref';
import {Type} from '../type';

import {ComponentTemplate, DirectiveDef} from './interfaces/definition';
import {ComponentTemplate, DirectiveDef, TypedDirectiveDef} from './interfaces/definition';
import {LInjector} from './interfaces/injector';
import {LContainerNode, LElementNode, LNodeFlags} from './interfaces/node';
import {assertNodeType} from './node_assert';
Expand Down Expand Up @@ -147,7 +147,7 @@ function createInjectionError(text: string, token: any) {
* @param di The node injector in which a directive will be added
* @param def The definition of the directive to be made public
*/
export function diPublicInInjector(di: LInjector, def: DirectiveDef<any>): void {
export function diPublicInInjector(di: LInjector, def: TypedDirectiveDef<any>): void {
bloomAdd(di, def.type);
}

Expand Down Expand Up @@ -211,7 +211,7 @@ export function getOrCreateInjectable<T>(di: LInjector, token: Type<T>, flags?:
for (let i = start, ii = start + size; i < ii; i++) {
// Get the definition for the directive at this index and, if it is injectable (diPublic),
// and matches the given token, return the directive instance.
const directiveDef = ngStaticData[i] as DirectiveDef<any>;
const directiveDef = ngStaticData[i] as TypedDirectiveDef<any>;
if (directiveDef.diPublic && directiveDef.type == token) {
return node.view.data[i];
}
Expand Down
13 changes: 8 additions & 5 deletions packages/core/src/render3/instructions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ import {LContainerNode, LElementNode, LNode, LNodeFlags, LProjectionNode, LTextN
import {assertNodeType} from './node_assert';
import {appendChild, insertChild, insertView, processProjectedNode, removeView} from './node_manipulation';
import {isNodeMatchingSelector} from './node_selector_matcher';
import {ComponentDef, ComponentTemplate, ComponentType, DirectiveDef, DirectiveType} from './interfaces/definition';
import {ComponentDef, ComponentTemplate, ComponentType, DirectiveDef, DirectiveType, TypedDirectiveDef, TypedComponentDef} from './interfaces/definition';
import {InjectFlags, diPublicInInjector, getOrCreateNodeInjectorForNode, getOrCreateElementRef, getOrCreateTemplateRef, getOrCreateContainerRef, getOrCreateInjectable} from './di';
import {QueryList, LQuery_} from './query';
import {RComment, RElement, RText, Renderer3, RendererFactory3, ProceduralRenderer3, ObjectOrientedRenderer3, RendererStyleFlags3} from './interfaces/renderer';
Expand Down Expand Up @@ -343,7 +343,7 @@ export function getOrCreateNodeInjector(): LInjector {
*
* @param def The definition of the directive to be made public
*/
export function diPublic(def: DirectiveDef<any>): void {
export function diPublic(def: TypedDirectiveDef<any>): void {
diPublicInInjector(getOrCreateNodeInjector(), def);
}

Expand Down Expand Up @@ -475,6 +475,8 @@ export function elementStart(
if (hostComponentDef) {
// TODO(mhevery): This assumes that the directives come in correct order, which
// is not guaranteed. Must be refactored to take it into account.
(hostComponentDef as TypedComponentDef<any>).type =
nameOrComponentType as ComponentType<any>;
directiveCreate(++index, hostComponentDef.n(), hostComponentDef, queryName);
}
hack_declareDirectives(index, directiveTypes, localRefs);
Expand All @@ -500,7 +502,9 @@ function hack_declareDirectives(
// template
// code for slight startup(first run) performance. (No impact on subsequent runs)
// TODO(misko): refactor this to store the `DirectiveDef` in `TView.data`.
const directiveDef = directiveTypes[i].ngDirectiveDef;
const directiveType = directiveTypes[i];
const directiveDef = directiveType.ngDirectiveDef;
(directiveDef as TypedDirectiveDef<any>).type = directiveType;
directiveCreate(
++index, directiveDef.n(), directiveDef, hack_findQueryName(directiveDef, localRefs));
}
Expand Down Expand Up @@ -945,8 +949,7 @@ export function directiveCreate<T>(
if (index >= ngStaticData.length) {
ngStaticData[index] = directiveDef !;
if (queryName) {
ngDevMode &&
assertNotNull(previousOrParentNode.tNode, 'previousOrParentNode.staticData');
ngDevMode && assertNotNull(previousOrParentNode.tNode, 'previousOrParentNode.staticData');
const nodeStaticData = previousOrParentNode !.tNode !;
(nodeStaticData.localNames || (nodeStaticData.localNames = [])).push(queryName, index);
}
Expand Down
30 changes: 17 additions & 13 deletions packages/core/src/render3/interfaces/definition.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,11 +28,6 @@ export const enum DirectiveDefFlags {ContentQuery = 0b10}
* `DirectiveDef` is a compiled version of the Directive used by the renderer instructions.
*/
export interface DirectiveDef<T> {
/**
* Token representing the directive. Used by DI.
*/
type: Type<T>;

/** Function that makes a directive public to the DI system. */
diPublic: ((def: DirectiveDef<any>) => void)|null;

Expand All @@ -41,26 +36,26 @@ export interface DirectiveDef<T> {
*
* The key is minified property name whereas the value is the original unminified name.
*/
inputs: {[P in keyof T]: P};
readonly inputs: {[P in keyof T]: P};

/**
* List of outputs which are part of the components public API.
*
* The key is minified property name whereas the value is the original unminified name.=
*/
outputs: {[P in keyof T]: P};
readonly outputs: {[P in keyof T]: P};

/**
* List of methods which are part of the components public API.
*
* The key is minified property name whereas the value is the original unminified name.
*/
methods: {[P in keyof T]: P};
readonly methods: {[P in keyof T]: P};

/**
* Name under which the directive is exported (for use with local references in template)
*/
exportAs: string|null;
readonly exportAs: string|null;

/**
* factory function used to create a new directive instance.
Expand Down Expand Up @@ -111,25 +106,34 @@ export interface ComponentDef<T> extends DirectiveDef<T> {
*
* NOTE: only used with component directives.
*/
tag: string;
readonly tag: string;

/**
* The View template of the component.
*
* NOTE: only used with component directives.
*/
template: ComponentTemplate<T>;
readonly template: ComponentTemplate<T>;

/**
* Renderer type data of the component.
*
* NOTE: only used with component directives.
*/
rendererType: RendererType2|null;
readonly rendererType: RendererType2|null;
}

/**
* Private: do not export
*/
export interface TypedDirectiveDef<T> extends DirectiveDef<T> { type: DirectiveType<T>; }

/**
* Private: do not export
*/
export interface TypedComponentDef<T> extends ComponentDef<T> { type: ComponentType<T>; }

export interface DirectiveDefArgs<T> {
type: Type<T>;
factory: () => T;
refresh?: (directiveIndex: number, elementIndex: number) => void;
inputs?: {[P in keyof T]?: string};
Expand Down
5 changes: 3 additions & 2 deletions packages/core/src/render3/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,14 @@ import {Type} from '../type';

import {assertNotNull} from './assert';
import {getOrCreateContainerRef, getOrCreateElementRef, getOrCreateNodeInjectorForNode, getOrCreateTemplateRef} from './di';
import {DirectiveDef} from './interfaces/definition';
import {DirectiveDef, TypedDirectiveDef} from './interfaces/definition';
import {LInjector} from './interfaces/injector';
import {LContainerNode, LElementNode, LNode, LNodeFlags, LViewNode, TNode} from './interfaces/node';
import {LQuery, QueryReadType} from './interfaces/query';
import {assertNodeOfPossibleTypes} from './node_assert';



/**
* A predicate which determines if a given element/directive should be included in the query
*/
Expand Down Expand Up @@ -142,7 +143,7 @@ function geIdxOfMatchingDirective(node: LNode, type: Type<any>): number|null {
for (let i = flags >> LNodeFlags.INDX_SHIFT,
ii = i + ((flags & LNodeFlags.SIZE_MASK) >> LNodeFlags.SIZE_SHIFT);
i < ii; i++) {
const def = ngStaticData[i] as DirectiveDef<any>;
const def = ngStaticData[i] as TypedDirectiveDef<any>;
if (def.diPublic && def.type === type) {
return i;
}
Expand Down
1 change: 0 additions & 1 deletion packages/core/test/render3/basic_perf.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,6 @@ describe('iv perf test', () => {
it(`${iteration}. create ${count} divs in Render3`, () => {
class Component {
static ngComponentDef = defineComponent({
type: Component,
tag: 'div',
template: function Template(ctx: any, cm: any) {
if (cm) {
Expand Down
17 changes: 7 additions & 10 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, Directive, Type, NgModule, Injectable, Optional, TemplateRef} from '../../src/core';
import {Component, Directive, Injectable, NgModule, Optional, TemplateRef, Type} from '../../src/core';
import * as r3 from '../../src/render3/index';

import {containerEl, renderComponent, requestAnimationFrame, toHtml} from './render_util';
Expand All @@ -27,7 +27,6 @@ describe('compiler specification', () => {
class MyComponent {
// NORMATIVE
static ngComponentDef = r3.defineComponent({
type: MyComponent,
tag: 'my-component',
factory: () => new MyComponent(),
template: function(ctx: MyComponent, cm: boolean) {
Expand Down Expand Up @@ -60,7 +59,6 @@ describe('compiler specification', () => {
constructor() { log.push('ChildComponent'); }
// NORMATIVE
static ngComponentDef = r3.defineComponent({
type: ChildComponent,
tag: `child`,
factory: () => new ChildComponent(),
template: function(ctx: ChildComponent, cm: boolean) {
Expand All @@ -79,7 +77,6 @@ describe('compiler specification', () => {
constructor() { log.push('SomeDirective'); }
// NORMATIVE
static ngDirectiveDef = r3.defineDirective({
type: ChildComponent,
factory: () => new SomeDirective(),
});
// /NORMATIVE
Expand Down Expand Up @@ -121,13 +118,13 @@ describe('compiler specification', () => {
constructor(template: TemplateRef<any>) { log.push('ifDirective'); }
// NORMATIVE
static ngDirectiveDef = r3.defineDirective({
type: IfDirective,
factory: () => new IfDirective(r3.injectTemplateRef()),
});
// /NORMATIVE
}

@Component({selector: 'my-component', template: `<ul #foo><li *if>{{salutation}} {{foo}}</li></ul>`})
@Component(
{selector: 'my-component', template: `<ul #foo><li *if>{{salutation}} {{foo}}</li></ul>`})
class MyComponent {
salutation = 'Hello';
// NORMATIVE
Expand Down Expand Up @@ -198,7 +195,7 @@ describe('compiler specification', () => {

xdescribe('NgModule', () => {
interface Injectable {
scope?: /*InjectorDefType<any>*/any;
scope?: /*InjectorDefType<any>*/ any;
factory: Function;
}

Expand Down Expand Up @@ -237,8 +234,8 @@ xdescribe('NgModule', () => {
static ngInjectorDef = defineInjector({
factory: () => new MyModule(inject(Toast)),
provider: [
{provide: Toast, deps: [String]}, // If Toast has matadata generate this line
Toast, // If toast has not metadata generate this line.
{provide: Toast, deps: [String]}, // If Toast has matadata generate this line
Toast, // If toast has not metadata generate this line.
{provide: String, useValue: 'Hello'}
],
imports: [CommonModule]
Expand All @@ -247,7 +244,7 @@ xdescribe('NgModule', () => {
}

@Injectable(/*{MyModule}*/)
class BurntToast{
class BurntToast {
constructor(@Optional() toast: Toast|null, name: String) {}
// NORMATIVE
static ngInjectableDef = defineInjectable({
Expand Down
6 changes: 0 additions & 6 deletions packages/core/test/render3/component_spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ describe('component', () => {
increment() { this.count++; }

static ngComponentDef = defineComponent({
type: CounterComponent,
tag: 'counter',
template: function(ctx: CounterComponent, cm: boolean) {
if (cm) {
Expand Down Expand Up @@ -64,7 +63,6 @@ describe('component', () => {
describe('encapsulation', () => {
class WrapperComponent {
static ngComponentDef = defineComponent({
type: WrapperComponent,
tag: 'wrapper',
template: function(ctx: WrapperComponent, cm: boolean) {
if (cm) {
Expand All @@ -80,7 +78,6 @@ describe('encapsulation', () => {

class EncapsulatedComponent {
static ngComponentDef = defineComponent({
type: EncapsulatedComponent,
tag: 'encapsulated',
template: function(ctx: EncapsulatedComponent, cm: boolean) {
if (cm) {
Expand All @@ -99,7 +96,6 @@ describe('encapsulation', () => {

class LeafComponent {
static ngComponentDef = defineComponent({
type: LeafComponent,
tag: 'leaf',
template: function(ctx: LeafComponent, cm: boolean) {
if (cm) {
Expand Down Expand Up @@ -129,7 +125,6 @@ describe('encapsulation', () => {
it('should encapsulate host and children with different attributes', () => {
class WrapperComponentWith {
static ngComponentDef = defineComponent({
type: WrapperComponent,
tag: 'wrapper',
template: function(ctx: WrapperComponentWith, cm: boolean) {
if (cm) {
Expand All @@ -147,7 +142,6 @@ describe('encapsulation', () => {

class LeafComponentwith {
static ngComponentDef = defineComponent({
type: LeafComponentwith,
tag: 'leaf',
template: function(ctx: LeafComponentwith, cm: boolean) {
if (cm) {
Expand Down
Loading

0 comments on commit 5eaaac3

Please sign in to comment.