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

Merge injectables when creating ElementInjector #2288

Closed
wants to merge 2 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
6 changes: 6 additions & 0 deletions modules/angular2/src/core/compiler/element_injector.ts
Expand Up @@ -460,8 +460,14 @@ export class ProtoElementInjector {

private static _createHostInjectorBindingData(bindings: List<ResolvedBinding>,
bd: List<BindingData>) {
var visitedIds: Map<number, boolean> = MapWrapper.create();
ListWrapper.forEach(bindings, b => {
ListWrapper.forEach(b.resolvedHostInjectables, b => {
if (MapWrapper.contains(visitedIds, b.key.id)) {
throw new BaseException(
`Multiple directives defined the same host injectable: "${stringify(b.key.token)}"`);
}
MapWrapper.set(visitedIds, b.key.id, true);
ListWrapper.push(bd, new BindingData(ProtoElementInjector._createBinding(b), LIGHT_DOM));
});
});
Expand Down
38 changes: 26 additions & 12 deletions modules/angular2/test/core/compiler/element_injector_spec.ts
Expand Up @@ -106,7 +106,7 @@ class NeedsDirectiveFromParent {
@Injectable()
class NeedsDirectiveFromParentOrSelf {
dependency: SimpleDirective;
constructor(@Parent({self:true}) dependency: SimpleDirective) { this.dependency = dependency; }
constructor(@Parent({self: true}) dependency: SimpleDirective) { this.dependency = dependency; }
}

@Injectable()
Expand Down Expand Up @@ -467,8 +467,21 @@ export function main() {
expect(pei.getBindingAtIndex(i).key.token).toBe(i);
}
});
});

it('should throw whenever multiple directives declare the same host injectable', () => {
expect(() => {
createPei(null, 0, [
DirectiveBinding.createFromType(SimpleDirective, new dirAnn.Component({
hostInjector: [bind('injectable1').toValue('injectable1')]
})),
DirectiveBinding.createFromType(SomeOtherDirective, new dirAnn.Component({
hostInjector: [bind('injectable1').toValue('injectable2')]
}))
]);
}).toThrowError('Multiple directives defined the same host injectable: "injectable1"');
});

});
});

describe("ElementInjector", () => {
Expand Down Expand Up @@ -575,15 +588,6 @@ export function main() {
expect(childInj.get('injectable2')).toEqual('injectable1-injectable2');
});

it("should instantiate components that depends on viewInjector dependencies", function() {
var inj = injector(
[DirectiveBinding.createFromType(
NeedsService,
new dirAnn.Component({viewInjector: [bind('service').toValue('service')]}))],
null, true);
expect(inj.get(NeedsService).service).toEqual('service');
});

it("should instantiate hostInjector injectables that have dependencies", () => {
var inj = injector(ListWrapper.concat(
[DirectiveBinding.createFromType(SimpleDirective, new dirAnn.Directive({
Expand All @@ -600,7 +604,7 @@ export function main() {
expect(inj.get('injectable2')).toEqual('injectable1-injectable2');
});

it("should instantiate components that depends on viewInjector dependencies", function() {
it("should instantiate components that depends on viewInjector dependencies", () => {
var inj = injector(
ListWrapper.concat([DirectiveBinding.createFromType(NeedsService, new dirAnn.Component({
viewInjector: [bind('service').toValue('service')]
Expand All @@ -610,6 +614,16 @@ export function main() {
expect(inj.get(NeedsService).service).toEqual('service');
});

it("should prioritize hostInjector over viewInjector for the same binding", () => {
var inj = injector(
ListWrapper.concat([DirectiveBinding.createFromType(NeedsService, new dirAnn.Component({
hostInjector: [bind('service').toValue('hostService')],
viewInjector: [bind('service').toValue('viewService')]})
)], extraBindings), null, true);
expect(inj.get(NeedsService).service).toEqual('hostService');
});


it("should instantiate directives that depend on app services", () => {
var appInjector = Injector.resolveAndCreate(
ListWrapper.concat([bind("service").toValue("service")], extraBindings));
Expand Down
35 changes: 34 additions & 1 deletion modules/angular2/test/core/compiler/integration_spec.ts
Expand Up @@ -950,6 +950,28 @@ export function main() {
});
}));


it('should prioritze hostInjector over viewInjector for the same binding',
inject([TestBed, AsyncTestCompleter], (tb, async) => {
tb.overrideView(MyComp, new viewAnn.View({
template: `
<directive-providing-injectable>
<directive-consuming-injectable #consuming>
</directive-consuming-injectable>
</directive-providing-injectable>
`,
directives:
[DirectiveProvidingInjectableInHostAndView, DirectiveConsumingInjectable]
}));
tb.createView(MyComp, {context: ctx})
.then((view) => {
var comp = view.rawView.locals.get("consuming");
expect(comp.injectable).toEqual("host");

async.done();
});
}));

it("should support viewInjector", inject([TestBed, AsyncTestCompleter], (tb, async) => {
tb.overrideView(DirectiveProvidingInjectableInView, new viewAnn.View({
template: `
Expand Down Expand Up @@ -1575,13 +1597,24 @@ class DirectiveProvidingInjectable {
class DirectiveProvidingInjectableInView {
}

@Component({
selector: 'directive-providing-injectable',
hostInjector: [new Binding(InjectableService, {toValue: 'host'})],
viewInjector: [new Binding(InjectableService, {toValue: 'view'})]
})
@View({template: ''})
@Injectable()
class DirectiveProvidingInjectableInHostAndView {
}


@Component({selector: 'directive-consuming-injectable'})
@View({template: ''})
@Injectable()
class DirectiveConsumingInjectable {
injectable;

constructor(@Ancestor() injectable: InjectableService) { this.injectable = injectable; }
constructor(@Ancestor() @Inject(InjectableService) injectable) { this.injectable = injectable; }
}


Expand Down