Skip to content

Commit 04baa46

Browse files
vsavkinvsavkin
committed
fix(di): removed default visibility
BREAKING CHANGE: Directives will use the Unbounded visibility by default, whereas before the change they used Self
1 parent 4bdc918 commit 04baa46

10 files changed

Lines changed: 26 additions & 69 deletions

File tree

‎modules/angular2/di.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -14,8 +14,7 @@ export {
1414
AncestorMetadata,
1515
UnboundedMetadata,
1616
DependencyMetadata,
17-
self,
18-
unbounded
17+
DEFAULT_VISIBILITY
1918
} from './src/di/metadata';
2019

2120
// we have to reexport * because Dart and TS export two different sets of types

‎modules/angular2/src/core/annotations_impl/annotations.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
import {CONST, CONST_EXPR} from 'angular2/src/facade/lang';
22
import {List} from 'angular2/src/facade/collection';
3-
import {InjectableMetadata, self} from 'angular2/src/di/metadata';
3+
import {InjectableMetadata} from 'angular2/src/di/metadata';
44
import {DEFAULT} from 'angular2/change_detection';
55

66
/**
@@ -792,7 +792,7 @@ export class Directive extends InjectableMetadata {
792792
exportAs?: string,
793793
compileChildren?: boolean,
794794
} = {}) {
795-
super(self);
795+
super();
796796
this.selector = selector;
797797
this.properties = properties;
798798
this.events = events;

‎modules/angular2/src/core/compiler/element_injector.ts‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -26,8 +26,7 @@ import {
2626
CyclicDependencyError,
2727
resolveForwardRef,
2828
VisibilityMetadata,
29-
DependencyProvider,
30-
self
29+
DependencyProvider
3130
} from 'angular2/di';
3231
import {
3332
InjectorInlineStrategy,

‎modules/angular2/src/di/binding.ts‎

Lines changed: 4 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,7 @@ import {
1616
InjectableMetadata,
1717
VisibilityMetadata,
1818
OptionalMetadata,
19-
unbounded,
19+
DEFAULT_VISIBILITY,
2020
DependencyMetadata
2121
} from './metadata';
2222
import {NoAnnotationError} from './exceptions';
@@ -30,7 +30,7 @@ export class Dependency {
3030
public properties: List<any>) {}
3131

3232
static fromKey(key: Key): Dependency {
33-
return new Dependency(key, false, _defaulVisiblity(key.token), []);
33+
return new Dependency(key, false, DEFAULT_VISIBILITY, []);
3434
}
3535
}
3636

@@ -397,18 +397,16 @@ function _extractToken(typeOrFunc, annotations /*List<any> | any*/, params: List
397397
var optional = false;
398398

399399
if (!isArray(annotations)) {
400-
return _createDependency(annotations, optional, _defaulVisiblity(annotations), depProps);
400+
return _createDependency(annotations, optional, DEFAULT_VISIBILITY, depProps);
401401
}
402402

403-
var visibility = null;
404-
var defaultVisibility = unbounded;
403+
var visibility = DEFAULT_VISIBILITY;
405404

406405
for (var i = 0; i < annotations.length; ++i) {
407406
var paramAnnotation = annotations[i];
408407

409408
if (paramAnnotation instanceof Type) {
410409
token = paramAnnotation;
411-
defaultVisibility = _defaulVisiblity(token);
412410

413411
} else if (paramAnnotation instanceof InjectMetadata) {
414412
token = paramAnnotation.token;
@@ -427,10 +425,6 @@ function _extractToken(typeOrFunc, annotations /*List<any> | any*/, params: List
427425
}
428426
}
429427

430-
if (isBlank(visibility)) {
431-
visibility = defaultVisibility;
432-
}
433-
434428
token = resolveForwardRef(token);
435429

436430
if (isPresent(token)) {
@@ -439,16 +433,6 @@ function _extractToken(typeOrFunc, annotations /*List<any> | any*/, params: List
439433
throw new NoAnnotationError(typeOrFunc, params);
440434
}
441435
}
442-
function _defaulVisiblity(typeOrFunc) {
443-
if (!(typeOrFunc instanceof Type)) return unbounded;
444-
445-
// TODO: vsavkin revisit this after clarifying lookup rules
446-
if (!reflector.isReflectionEnabled()) return unbounded;
447-
448-
var f =
449-
ListWrapper.filter(reflector.annotations(typeOrFunc), s => s instanceof InjectableMetadata);
450-
return f.length === 0 ? unbounded : f[0].visibility;
451-
}
452436

453437
function _createDependency(token, optional, visibility, depProps): Dependency {
454438
return new Dependency(Key.get(token), optional, visibility, depProps);

‎modules/angular2/src/di/decorators.dart‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ class Optional extends OptionalMetadata {
2121
* {@link InjectableMetadata}.
2222
*/
2323
class Injectable extends InjectableMetadata {
24-
const Injectable([VisibilityMetadata visibility = unbounded]): super(visibility);
24+
const Injectable(): super();
2525
}
2626

2727
/**

‎modules/angular2/src/di/decorators.ts‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -30,8 +30,8 @@ export interface OptionalFactory {
3030
* Factory for creating {@link InjectableMetadata}.
3131
*/
3232
export interface InjectableFactory {
33-
(visibility?: VisibilityMetadata): any;
34-
new (visibility?: VisibilityMetadata): InjectableMetadata;
33+
(): any;
34+
new (): InjectableMetadata;
3535
}
3636

3737
/**

‎modules/angular2/src/di/injector.ts‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import {
1414
import {FunctionWrapper, Type, isPresent, isBlank, CONST_EXPR} from 'angular2/src/facade/lang';
1515
import {Key} from './key';
1616
import {resolveForwardRef} from './forward_ref';
17-
import {VisibilityMetadata, unbounded} from './metadata';
17+
import {VisibilityMetadata, DEFAULT_VISIBILITY} from './metadata';
1818

1919
const _constructing = CONST_EXPR(new Object());
2020
const _notFound = CONST_EXPR(new Object());
@@ -521,7 +521,9 @@ export class Injector {
521521
*binding).
522522
* @returns an instance represented by the token. Throws if not found.
523523
*/
524-
get(token): any { return this._getByKey(Key.get(token), unbounded, false, PUBLIC_AND_PRIVATE); }
524+
get(token): any {
525+
return this._getByKey(Key.get(token), DEFAULT_VISIBILITY, false, PUBLIC_AND_PRIVATE);
526+
}
525527

526528
/**
527529
* Retrieves an instance from the injector.
@@ -530,7 +532,7 @@ export class Injector {
530532
* @returns an instance represented by the token. Returns `null` if not found.
531533
*/
532534
getOptional(token): any {
533-
return this._getByKey(Key.get(token), unbounded, true, PUBLIC_AND_PRIVATE);
535+
return this._getByKey(Key.get(token), DEFAULT_VISIBILITY, true, PUBLIC_AND_PRIVATE);
534536
}
535537

536538
/**

‎modules/angular2/src/di/metadata.ts‎

Lines changed: 2 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,7 @@ export class DependencyMetadata {
7979
*/
8080
@CONST()
8181
export class InjectableMetadata {
82-
constructor(public visibility: VisibilityMetadata = unbounded) {}
82+
constructor() {}
8383
}
8484

8585
/**
@@ -123,8 +123,6 @@ export class SelfMetadata extends VisibilityMetadata {
123123
toString(): string { return `@Self()`; }
124124
}
125125

126-
export const self = CONST_EXPR(new SelfMetadata());
127-
128126
/**
129127
* Specifies that an injector should retrieve a dependency from the direct parent.
130128
*
@@ -235,4 +233,4 @@ export class UnboundedMetadata extends VisibilityMetadata {
235233
toString(): string { return `@Unbounded(self: ${this.includeSelf}})`; }
236234
}
237235

238-
export const unbounded = CONST_EXPR(new UnboundedMetadata({self: true}));
236+
export const DEFAULT_VISIBILITY = CONST_EXPR(new UnboundedMetadata({self: true}));

‎modules/angular2/test/core/compiler/element_injector_spec.ts‎

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,7 +40,7 @@ import {
4040
Directive,
4141
onDestroy
4242
} from 'angular2/annotations';
43-
import {bind, Injector, Binding, Optional, Inject, Injectable, Self, Parent, Ancestor, Unbounded, self, InjectMetadata, ParentMetadata} from 'angular2/di';
43+
import {bind, Injector, Binding, Optional, Inject, Injectable, Self, Parent, Ancestor, Unbounded, InjectMetadata, ParentMetadata} from 'angular2/di';
4444
import {AppProtoView, AppView} from 'angular2/src/core/compiler/view';
4545
import {ViewContainerRef} from 'angular2/src/core/compiler/view_container_ref';
4646
import {ProtoViewRef} from 'angular2/src/core/compiler/view_ref';
@@ -73,16 +73,16 @@ class DummyElementRef extends SpyObject {
7373
noSuchMethod(m) { return super.noSuchMethod(m); }
7474
}
7575

76-
@Injectable(self)
76+
@Injectable()
7777
class SimpleDirective {}
7878

7979
class SimpleService {}
8080

81-
@Injectable(self)
81+
@Injectable()
8282
class SomeOtherDirective {}
8383

8484
var _constructionCount = 0;
85-
@Injectable(self)
85+
@Injectable()
8686
class CountingDirective {
8787
count;
8888
constructor() {
@@ -91,21 +91,21 @@ class CountingDirective {
9191
}
9292
}
9393

94-
@Injectable(self)
94+
@Injectable()
9595
class FancyCountingDirective extends CountingDirective {
9696
constructor() { super(); }
9797
}
9898

9999
@Injectable()
100100
class NeedsDirective {
101101
dependency: SimpleDirective;
102-
constructor(dependency: SimpleDirective) { this.dependency = dependency; }
102+
constructor(@Self() dependency: SimpleDirective) { this.dependency = dependency; }
103103
}
104104

105105
@Injectable()
106106
class OptionallyNeedsDirective {
107107
dependency: SimpleDirective;
108-
constructor(@Optional() dependency: SimpleDirective) { this.dependency = dependency; }
108+
constructor(@Self() @Optional() dependency: SimpleDirective) { this.dependency = dependency; }
109109
}
110110

111111
@Injectable()

‎modules/angular2/test/di/injector_spec.ts‎

Lines changed: 1 addition & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@ import {
1616
forwardRef,
1717
DependencyMetadata,
1818
Injectable,
19-
InjectMetadata,
20-
self,
21-
unbounded
19+
InjectMetadata
2220
} from 'angular2/di';
2321

2422
import {InjectorInlineStrategy, InjectorDynamicStrategy} from 'angular2/src/di/injector';
@@ -29,15 +27,6 @@ class CustomDependencyMetadata extends DependencyMetadata {}
2927

3028
class Engine {}
3129

32-
@Injectable(self)
33-
class EngineWithSetVisibility {
34-
}
35-
36-
@Injectable()
37-
class CarNeedsEngineWithSetVisibility {
38-
constructor(engine: EngineWithSetVisibility) {}
39-
}
40-
4130
class BrokenEngine {
4231
constructor() { throw new BaseException("Broken Engine"); }
4332
}
@@ -417,19 +406,5 @@ export function main() {
417406
expect(binding.dependencies[0].properties).toEqual([new CustomDependencyMetadata()]);
418407
});
419408
});
420-
421-
describe("default visibility", () => {
422-
it("should use the provided visibility", () => {
423-
var bindings = Injector.resolve([CarNeedsEngineWithSetVisibility, EngineWithSetVisibility]);
424-
var carBinding = bindings[0];
425-
expect(carBinding.dependencies[0].visibility).toEqual(self);
426-
});
427-
428-
it("should set the default visibility to unbounded", () => {
429-
var bindings = Injector.resolve([Car, Engine]);
430-
var carBinding = bindings[0];
431-
expect(carBinding.dependencies[0].visibility).toEqual(unbounded);
432-
});
433-
});
434409
});
435410
}

0 commit comments

Comments
 (0)