feat(injector): implement a different approach to injector scopes#50
feat(injector): implement a different approach to injector scopes#50rodyhaddad wants to merge 1 commit intoangular:masterfrom
Conversation
|
This PR makes this test pass (discussed with vojta, having this test pass is not set in stone): it('should do stuff and work', function () {
class RouteScope extends ScopeAnnotation {}
function Compiler() {}
@Inject(Compiler)
function Foo(compiler) {
this.compiler = compiler;
}
@RouteScope
@Inject(Foo, Compiler)
function RequestHandler(foo, compiler) {
this.foo = foo;
this.compiler = compiler;
}
@Provide(Compiler)
function RouteCompiler() {}
var injector = new Injector(),
child = injector.createChild([RouteCompiler], [RouteScope]);
var reqHandler = child.get(RequestHandler);
expect(reqHandler.compiler).not.toBe(reqHandler.foo.compiler);
expect(reqHandler.compiler).toBeInstanceOf(RouteCompiler);
expect(reqHandler.foo.compiler).toBeInstanceOf(Compiler);
});I'll amend the PR to add this test if that's the behavior that we want. |
There was a problem hiding this comment.
Let's call it Scope to make it consistent. Or DefaultScope, BaseScope or something?
I think you are right and we might rename all of the annotations to *Annotation but that can be done in a separate commit. The reason why I didn't do that already was that then people have to import it as import {InjectAnnotation as Inject} from 'di';... and that kind of sucks....
We should probably separate all these annotations to a separate module because often projects only need the annotations (so that their code can be used with DI) but they don't need the actual DI. Then, I think Inject, Scope, etc. will be fine...
What do you think?
|
@rodyhaddad this is awesome. Thank you! I left some subtle comments, nothing serious... |
There was a problem hiding this comment.
Can you change this fancy indentation to:
var a = b;
var c = d;
var e = f;I know we use this in Angular codebase, but I prefer this old-school style, it's much easier to add new var or re-order...
There was a problem hiding this comment.
Same for other places....
e46889e to
e4c8659
Compare
An easy way to review this is to read the tests and see if that's what we want