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

cleanup(DI): clean up visibility decorators #3372

Closed
wants to merge 1 commit 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 modules/angular2/annotations.ts
Expand Up @@ -5,7 +5,7 @@
* Annotations provide the additional information that Angular requires in order to run your
* application. This module
* contains {@link Component}, {@link Directive}, and {@link View} annotations, as well as
* the {@link Ancestor} annotation that is used by Angular to resolve dependencies.
* the {@link Host} annotation that is used by Angular to resolve dependencies.
*
*/

Expand Down
8 changes: 3 additions & 5 deletions modules/angular2/di.ts
Expand Up @@ -8,12 +8,10 @@ export {
InjectMetadata,
OptionalMetadata,
InjectableMetadata,
VisibilityMetadata,
SelfMetadata,
AncestorMetadata,
UnboundedMetadata,
DependencyMetadata,
DEFAULT_VISIBILITY
HostMetadata,
SkipSelfMetadata,
DependencyMetadata
} from './src/di/metadata';

// we have to reexport * because Dart and TS export two different sets of types
Expand Down
10 changes: 5 additions & 5 deletions modules/angular2/docs/core/02_directives.md
Expand Up @@ -261,7 +261,7 @@ Injecting other directives into directives follows a similar mechanism as inject
There are five kinds of visibilities:

* (no annotation): Inject dependent directives only if they are on the current element.
* `@ancestor`: Inject a directive if it is at any element above the current element.
* `@SkipSelf()`: Inject a directive if it is at any element above the current element.
* `@child`: Inject a list of direct children which match a given type. (Used with `Query`)
* `@descendant`: Inject a list of any children which match a given type. (Used with `Query`)

Expand Down Expand Up @@ -299,8 +299,8 @@ class FieldSet { |
@Directive({ selector: 'field' }) |
class Field { |
constructor( |
@ancestor field:Form, |
@ancestor field:FieldSet, |
@SkipSelf() field:Form, |
Copy link
Contributor

Choose a reason for hiding this comment

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

indentation

@SkipSelf() field:FieldSet, |
) { ... } |
} |
|
Expand Down Expand Up @@ -336,7 +336,7 @@ Shadow DOM provides an encapsulation for components, so as a general rule it doe
})
class Kid {
constructor(
@Ancestor() dad:Dad,
@SkipSelf() dad:Dad,
@Optional() grandpa:Grandpa
) {
this.name = 'Billy';
Expand All @@ -353,7 +353,7 @@ class Kid {
directives: [Kid]
})
class Dad {
constructor(@Ancestor() dad:Grandpa) {
constructor(@SkipSelf() dad:Grandpa) {
this.name = 'Joe Jr';
this.dad = dad.name;
}
Expand Down
4 changes: 2 additions & 2 deletions modules/angular2/src/change_detection/pipes/pipes.ts
@@ -1,7 +1,7 @@
import {ListWrapper, isListLikeIterable, StringMapWrapper} from 'angular2/src/facade/collection';
import {isBlank, isPresent, BaseException, CONST} from 'angular2/src/facade/lang';
import {Pipe, PipeFactory} from './pipe';
import {Injectable, UnboundedMetadata, OptionalMetadata} from 'angular2/di';
import {Injectable, OptionalMetadata, SkipSelfMetadata} from 'angular2/di';
import {ChangeDetectorRef} from '../change_detector_ref';
import {Binding} from 'angular2/di';

Expand Down Expand Up @@ -80,7 +80,7 @@ export class Pipes {
return Pipes.create(config, pipes);
},
// Dependency technically isn't optional, but we can provide a better error message this way.
deps: [[Pipes, new UnboundedMetadata(), new OptionalMetadata()]]
deps: [[Pipes, new SkipSelfMetadata(), new OptionalMetadata()]]
});
}

Expand Down
18 changes: 7 additions & 11 deletions modules/angular2/src/core/annotations_impl/annotations.ts
Expand Up @@ -53,11 +53,9 @@ import {DEFAULT} from 'angular2/change_detection';
*
* To inject other directives, declare the constructor parameter as:
* - `directive:DirectiveType`: a directive on the current element only
* - `@Ancestor() directive:DirectiveType`: any directive that matches the type between the current
* - `@Host() directive:DirectiveType`: any directive that matches the type between the current
* element and the
* Shadow DOM root. Current element is not included in the resolution, therefore even if it could
* resolve it, it will
* be ignored.
* Shadow DOM root.
* - `@Query(DirectiveType) query:QueryList<DirectiveType>`: A live collection of direct child
* directives.
* - `@QueryDescendants(DirectiveType) query:QueryList<DirectiveType>`: A live collection of any
Expand Down Expand Up @@ -164,21 +162,19 @@ import {DEFAULT} from 'angular2/change_detection';
* ### Injecting a directive from any ancestor elements
*
* Directives can inject other directives declared on any ancestor element (in the current Shadow
* DOM), i.e. on the
* parent element and its parents. By definition, a directive with an `@Ancestor` annotation does
* not attempt to
* resolve dependencies for the current element, even if this would satisfy the dependency.
*
* DOM), i.e. on the current element, the
* parent element, or its parents.
* ```
* @Directive({ selector: '[my-directive]' })
* class MyDirective {
* constructor(@Ancestor() dependency: Dependency) {
* constructor(@Host() dependency: Dependency) {
* expect(dependency.id).toEqual(2);
* }
* }
* ```
*
* `@Ancestor` checks the parent, as well as its parents recursively. If `dependency="2"` didn't
* `@Host` checks the current element, the parent, as well as its parents recursively. If
* `dependency="2"` didn't
* exist on the direct parent, this injection would
* have returned
* `dependency="1"`.
Expand Down
14 changes: 7 additions & 7 deletions modules/angular2/src/core/compiler/element_injector.ts
Expand Up @@ -25,7 +25,6 @@ import {
AbstractBindingError,
CyclicDependencyError,
resolveForwardRef,
VisibilityMetadata,
DependencyProvider
} from 'angular2/di';
import {
Expand Down Expand Up @@ -167,9 +166,10 @@ export class TreeNode<T extends TreeNode<any>> {
}

export class DirectiveDependency extends Dependency {
constructor(key: Key, optional: boolean, visibility: any, properties: List<any>,
public attributeName: string, public queryDecorator: Query) {
super(key, optional, visibility, properties);
constructor(key: Key, optional: boolean, lowerBoundVisibility: Object,
upperBoundVisibility: Object, properties: List<any>, public attributeName: string,
public queryDecorator: Query) {
super(key, optional, lowerBoundVisibility, upperBoundVisibility, properties);
this._verify();
}

Expand All @@ -183,9 +183,9 @@ export class DirectiveDependency extends Dependency {
}

static createFrom(d: Dependency): Dependency {
return new DirectiveDependency(d.key, d.optional, d.visibility, d.properties,
DirectiveDependency._attributeName(d.properties),
DirectiveDependency._query(d.properties));
return new DirectiveDependency(
d.key, d.optional, d.lowerBoundVisibility, d.upperBoundVisibility, d.properties,
DirectiveDependency._attributeName(d.properties), DirectiveDependency._query(d.properties));
}

static _attributeName(properties): string {
Expand Down
64 changes: 37 additions & 27 deletions modules/angular2/src/di/binding.ts
Expand Up @@ -14,9 +14,10 @@ import {Key} from './key';
import {
InjectMetadata,
InjectableMetadata,
VisibilityMetadata,
OptionalMetadata,
DEFAULT_VISIBILITY,
SelfMetadata,
HostMetadata,
SkipSelfMetadata,
DependencyMetadata
} from './metadata';
import {NoAnnotationError} from './exceptions';
Expand All @@ -26,12 +27,10 @@ import {resolveForwardRef} from './forward_ref';
* @private
*/
export class Dependency {
constructor(public key: Key, public optional: boolean, public visibility: VisibilityMetadata,
public properties: List<any>) {}
constructor(public key: Key, public optional: boolean, public lowerBoundVisibility: any,
public upperBoundVisibility: any, public properties: List<any>) {}

static fromKey(key: Key): Dependency {
return new Dependency(key, false, DEFAULT_VISIBILITY, []);
}
static fromKey(key: Key): Dependency { return new Dependency(key, false, null, null, []); }
}

const _EMPTY_LIST = CONST_EXPR([]);
Expand Down Expand Up @@ -390,50 +389,61 @@ function _dependenciesFor(typeOrFunc): List<Dependency> {
return ListWrapper.map(params, (p: List<any>) => _extractToken(typeOrFunc, p, params));
}

function _extractToken(typeOrFunc, annotations /*List<any> | any*/, params: List<List<any>>):
function _extractToken(typeOrFunc, metadata /*List<any> | any*/, params: List<List<any>>):
Dependency {
var depProps = [];
var token = null;
var optional = false;

if (!isArray(annotations)) {
return _createDependency(annotations, optional, DEFAULT_VISIBILITY, depProps);
if (!isArray(metadata)) {
return _createDependency(metadata, optional, null, null, depProps);
}

var visibility = DEFAULT_VISIBILITY;
var lowerBoundVisibility = null;
;
var upperBoundVisibility = null;
;

for (var i = 0; i < annotations.length; ++i) {
var paramAnnotation = annotations[i];
for (var i = 0; i < metadata.length; ++i) {
var paramMetadata = metadata[i];

if (paramAnnotation instanceof Type) {
token = paramAnnotation;
if (paramMetadata instanceof Type) {
token = paramMetadata;

} else if (paramAnnotation instanceof InjectMetadata) {
token = paramAnnotation.token;
} else if (paramMetadata instanceof InjectMetadata) {
token = paramMetadata.token;

} else if (paramAnnotation instanceof OptionalMetadata) {
} else if (paramMetadata instanceof OptionalMetadata) {
optional = true;

} else if (paramAnnotation instanceof VisibilityMetadata) {
visibility = paramAnnotation;
} else if (paramMetadata instanceof SelfMetadata) {
upperBoundVisibility = paramMetadata;

} else if (paramMetadata instanceof HostMetadata) {
upperBoundVisibility = paramMetadata;

} else if (paramAnnotation instanceof DependencyMetadata) {
if (isPresent(paramAnnotation.token)) {
token = paramAnnotation.token;
} else if (paramMetadata instanceof SkipSelfMetadata) {
lowerBoundVisibility = paramMetadata;

} else if (paramMetadata instanceof DependencyMetadata) {
if (isPresent(paramMetadata.token)) {
token = paramMetadata.token;
}
depProps.push(paramAnnotation);
depProps.push(paramMetadata);
}
}

token = resolveForwardRef(token);

if (isPresent(token)) {
return _createDependency(token, optional, visibility, depProps);
return _createDependency(token, optional, lowerBoundVisibility, upperBoundVisibility, depProps);
} else {
throw new NoAnnotationError(typeOrFunc, params);
}
}

function _createDependency(token, optional, visibility, depProps): Dependency {
return new Dependency(Key.get(token), optional, visibility, depProps);
function _createDependency(token, optional, lowerBoundVisibility, upperBoundVisibility, depProps):
Dependency {
return new Dependency(Key.get(token), optional, lowerBoundVisibility, upperBoundVisibility,
depProps);
}
12 changes: 6 additions & 6 deletions modules/angular2/src/di/decorators.dart
Expand Up @@ -32,15 +32,15 @@ class Self extends SelfMetadata {
}

/**
* {@link AncestorMetadata}.
* {@link HostMetadata}.
*/
class Ancestor extends AncestorMetadata {
const Ancestor({bool self}) : super(self: self);
class Host extends HostMetadata {
const Host() : super();
}

/**
* {@link UnboundedMetadata}.
* {@link SkipSelfMetadata}.
*/
class Unbounded extends UnboundedMetadata {
const Unbounded({bool self}) : super(self: self);
class SkipSelf extends SkipSelfMetadata {
const SkipSelf() : super();
}
29 changes: 14 additions & 15 deletions modules/angular2/src/di/decorators.ts
Expand Up @@ -3,9 +3,8 @@ import {
OptionalMetadata,
InjectableMetadata,
SelfMetadata,
VisibilityMetadata,
AncestorMetadata,
UnboundedMetadata
HostMetadata,
SkipSelfMetadata
} from './metadata';
import {makeDecorator, makeParamDecorator, TypeDecorator} from '../util/decorators';

Expand Down Expand Up @@ -42,19 +41,19 @@ export interface SelfFactory {
}

/**
* Factory for creating {@link AncestorMetadata}.
* Factory for creating {@link HostMetadata}.
*/
export interface AncestorFactory {
(visibility?: {self: boolean}): any;
new (visibility?: {self: boolean}): AncestorMetadata;
export interface HostFactory {
(): any;
new (): HostMetadata;
}

/**
* Factory for creating {@link UnboundedMetadata}.
* Factory for creating {@link SkipSelfMetadata}.
*/
export interface UnboundedFactory {
(visibility?: {self: boolean}): any;
new (visibility?: {self: boolean}): UnboundedMetadata;
export interface SkipSelfFactory {
(): any;
new (): SkipSelfMetadata;
}

/**
Expand All @@ -78,11 +77,11 @@ export var Injectable: InjectableFactory = <InjectableFactory>makeDecorator(Inje
export var Self: SelfFactory = makeParamDecorator(SelfMetadata);

/**
* Factory for creating {@link AncestorMetadata}.
* Factory for creating {@link HostMetadata}.
*/
export var Ancestor: AncestorFactory = makeParamDecorator(AncestorMetadata);
export var Host: HostFactory = makeParamDecorator(HostMetadata);

/**
* Factory for creating {@link UnboundedMetadata}.
* Factory for creating {@link SkipSelfMetadata}.
*/
export var Unbounded: UnboundedFactory = makeParamDecorator(UnboundedMetadata);
export var SkipSelf: SkipSelfFactory = makeParamDecorator(SkipSelfMetadata);