From 1eea2b254eed84d0447bf386e41a868b197fb379 Mon Sep 17 00:00:00 2001 From: Misko Hevery Date: Wed, 13 May 2015 15:54:46 -0700 Subject: [PATCH] feat: allow for forward references in injection It is possible for a class defined first to be referencing a class defined later, and as a result at the time of the definition it is not possible to access the later's class reference. This allows to refer to the later defined class through a closure.Closes #1891 --- modules/angular2/angular2.js | 1 + modules/angular2/di.ts | 1 + .../angular2/src/core/compiler/compiler.js | 4 +- .../src/core/compiler/directive_resolver.js | 3 +- .../src/core/compiler/element_injector.js | 4 +- modules/angular2/src/di/binding.ts | 14 +++- modules/angular2/src/di/forward_ref.dart | 17 ++++ modules/angular2/src/di/forward_ref.ts | 50 +++++++++++ modules/angular2/src/di/injector.ts | 3 +- modules/angular2/src/di/key.ts | 5 +- .../core/forward_ref_integration_spec.dart | 6 ++ .../core/forward_ref_integration_spec.es6 | 84 +++++++++++++++++++ modules/angular2/test/di/forward_ref_spec.js | 23 +++++ modules/angular2/test/di/injector_spec.js | 23 ++++- 14 files changed, 225 insertions(+), 13 deletions(-) create mode 100644 modules/angular2/src/di/forward_ref.dart create mode 100644 modules/angular2/src/di/forward_ref.ts create mode 100644 modules/angular2/test/core/forward_ref_integration_spec.dart create mode 100644 modules/angular2/test/core/forward_ref_integration_spec.es6 create mode 100644 modules/angular2/test/di/forward_ref_spec.js diff --git a/modules/angular2/angular2.js b/modules/angular2/angular2.js index facf3da73a..53db8d8359 100644 --- a/modules/angular2/angular2.js +++ b/modules/angular2/angular2.js @@ -3,6 +3,7 @@ export * from './core'; export * from './annotations'; export * from './directives'; export * from './forms'; +export * from './di'; export {Observable, EventEmitter} from 'angular2/src/facade/async'; export * from 'angular2/src/render/api'; export {DomRenderer, DOCUMENT_TOKEN} from 'angular2/src/render/dom/dom_renderer'; diff --git a/modules/angular2/di.ts b/modules/angular2/di.ts index bfcb3b5bfd..045cce6627 100644 --- a/modules/angular2/di.ts +++ b/modules/angular2/di.ts @@ -7,6 +7,7 @@ export * from './src/di/annotations'; export * from './src/di/decorators'; +export * from './src/di/forward_ref'; export {Injector} from './src/di/injector'; export {Binding, ResolvedBinding, Dependency, bind} from './src/di/binding'; export {Key, KeyRegistry, TypeLiteral} from './src/di/key'; diff --git a/modules/angular2/src/core/compiler/compiler.js b/modules/angular2/src/core/compiler/compiler.js index 6683638b42..617ba5585a 100644 --- a/modules/angular2/src/core/compiler/compiler.js +++ b/modules/angular2/src/core/compiler/compiler.js @@ -1,4 +1,4 @@ -import {Binding} from 'angular2/di'; +import {Binding, resolveForwardRef} from 'angular2/di'; import {Injectable} from 'angular2/src/di/annotations_impl'; import {Type, isBlank, isPresent, BaseException, normalizeBlank, stringify} from 'angular2/src/facade/lang'; import {Promise, PromiseWrapper} from 'angular2/src/facade/async'; @@ -237,7 +237,7 @@ export class Compiler { _flattenList(tree:List, out:List /**/):void { for (var i = 0; i < tree.length; i++) { - var item = tree[i]; + var item = resolveForwardRef(tree[i]); if (ListWrapper.isList(item)) { this._flattenList(item, out); } else { diff --git a/modules/angular2/src/core/compiler/directive_resolver.js b/modules/angular2/src/core/compiler/directive_resolver.js index 38d751a3e4..4795e7745d 100644 --- a/modules/angular2/src/core/compiler/directive_resolver.js +++ b/modules/angular2/src/core/compiler/directive_resolver.js @@ -1,4 +1,5 @@ import {Injectable} from 'angular2/src/di/annotations_impl'; +import {resolveForwardRef} from 'angular2/di'; import {Type, isPresent, BaseException, stringify} from 'angular2/src/facade/lang'; import {Directive} from '../annotations_impl/annotations'; import {reflector} from 'angular2/src/reflection/reflection'; @@ -6,7 +7,7 @@ import {reflector} from 'angular2/src/reflection/reflection'; @Injectable() export class DirectiveResolver { resolve(type:Type):Directive { - var annotations = reflector.annotations(type); + var annotations = reflector.annotations(resolveForwardRef(type)); if (isPresent(annotations)) { for (var i=0; i p instanceof Query); - return isPresent(p) ? p.directive : null; + return isPresent(p) ? resolveForwardRef(p.directive) : null; } } diff --git a/modules/angular2/src/di/binding.ts b/modules/angular2/src/di/binding.ts index 5e09ffd6d9..e771229889 100644 --- a/modules/angular2/src/di/binding.ts +++ b/modules/angular2/src/di/binding.ts @@ -10,6 +10,7 @@ import { DependencyAnnotation } from './annotations_impl'; import {NoAnnotationError} from './exceptions'; +import {resolveForwardRef} from './forward_ref'; /** * @private @@ -217,8 +218,9 @@ export class Binding { var resolvedDeps; var isAsync = false; if (isPresent(this.toClass)) { - factoryFn = reflector.factory(this.toClass); - resolvedDeps = _dependenciesFor(this.toClass); + var toClass = resolveForwardRef(this.toClass); + factoryFn = reflector.factory(toClass); + resolvedDeps = _dependenciesFor(toClass); } else if (isPresent(this.toAlias)) { factoryFn = (aliasInstance) => aliasInstance; resolvedDeps = [Dependency.fromKey(Key.get(this.toAlias))]; @@ -234,7 +236,8 @@ export class Binding { resolvedDeps = _EMPTY_LIST; } - return new ResolvedBinding(Key.get(this.token), factoryFn, resolvedDeps, isAsync); + return new ResolvedBinding(Key.get(resolveForwardRef(this.token)), factoryFn, resolvedDeps, + isAsync); } } @@ -428,7 +431,8 @@ export class BindingBuilder { function _constructDependencies(factoryFunction: Function, dependencies: List) { return isBlank(dependencies) ? _dependenciesFor(factoryFunction) : - ListWrapper.map(dependencies, (t) => Dependency.fromKey(Key.get(t))); + ListWrapper.map(dependencies, + (t) => Dependency.fromKey(Key.get(resolveForwardRef(t)))); } function _dependenciesFor(typeOrFunc): List { @@ -475,6 +479,8 @@ function _extractToken(typeOrFunc, annotations) { } } + token = resolveForwardRef(token); + if (isPresent(token)) { return _createDependency(token, asPromise, lazy, optional, depProps); } else { diff --git a/modules/angular2/src/di/forward_ref.dart b/modules/angular2/src/di/forward_ref.dart new file mode 100644 index 0000000000..2fef206e9d --- /dev/null +++ b/modules/angular2/src/di/forward_ref.dart @@ -0,0 +1,17 @@ +library angular2.di.forward_ref; + +typedef Type ForwardRefFn(); + +/** + * Dart does not have the forward ref problem, so this function is a noop. + */ +forwardRef(ForwardRefFn forwardRefFn) => forwardRefFn(); + +/** + * Lazily retrieve the reference value. + * + * See: {@link forwardRef} + * + * @exportedAs angular2/di + */ +resolveForwardRef(type) => type; diff --git a/modules/angular2/src/di/forward_ref.ts b/modules/angular2/src/di/forward_ref.ts new file mode 100644 index 0000000000..0c393d63fc --- /dev/null +++ b/modules/angular2/src/di/forward_ref.ts @@ -0,0 +1,50 @@ +import {Type} from 'angular2/src/facade/lang'; + +export interface ForwardRefFn { (): Type; } + +/** + * Allows to refer to references which are not yet defined. + * + * This situation arises when the key which we need te refer to for the purposes of DI is declared, + * but not yet defined. + * + * ## Example: + * + * ``` + * class Door { + * // Incorrect way to refer to a reference which is defined later. + * // This fails because `Lock` is undefined at this point. + * constructor(lock:Lock) { } + * + * // Correct way to refer to a reference which is defined later. + * // The reference needs to be captured in a closure. + * constructor(@Inject(forwardRef(() => Lock)) lock:Lock) { } + * } + * + * // Only at this point the lock is defined. + * class Lock { + * } + * ``` + * + * @exportedAs angular2/di + */ +export function forwardRef(forwardRefFn: ForwardRefFn): Type { + (forwardRefFn).__forward_ref__ = forwardRef; + return (forwardRefFn); +} + +/** + * Lazily retrieve the reference value. + * + * See: {@link forwardRef} + * + * @exportedAs angular2/di + */ +export function resolveForwardRef(type: any): any { + if (typeof type == 'function' && type.hasOwnProperty('__forward_ref__') && + type.__forward_ref__ === forwardRef) { + return (type)(); + } else { + return type; + } +} diff --git a/modules/angular2/src/di/injector.ts b/modules/angular2/src/di/injector.ts index 652c01eed9..99b13a4be5 100644 --- a/modules/angular2/src/di/injector.ts +++ b/modules/angular2/src/di/injector.ts @@ -13,6 +13,7 @@ import { import {FunctionWrapper, Type, isPresent, isBlank} from 'angular2/src/facade/lang'; import {PromiseWrapper, Promise} from 'angular2/src/facade/async'; import {Key} from './key'; +import {resolveForwardRef} from './forward_ref'; var _constructing = new Object(); var _notFound = new Object(); @@ -370,7 +371,7 @@ class _AsyncInjectorStrategy { function _resolveBindings(bindings: List): List { var resolvedList = ListWrapper.createFixedSize(bindings.length); for (var i = 0; i < bindings.length; i++) { - var unresolved = bindings[i]; + var unresolved = resolveForwardRef(bindings[i]); var resolved; if (unresolved instanceof ResolvedBinding) { resolved = unresolved; // ha-ha! I'm easily amused diff --git a/modules/angular2/src/di/key.ts b/modules/angular2/src/di/key.ts index 8e48244e89..fdc3d0d8e1 100644 --- a/modules/angular2/src/di/key.ts +++ b/modules/angular2/src/di/key.ts @@ -1,5 +1,5 @@ import {MapWrapper} from 'angular2/src/facade/collection'; -import {stringify, CONST, Type} from 'angular2/src/facade/lang'; +import {stringify, CONST, Type, isBlank, BaseException} from 'angular2/src/facade/lang'; import {TypeLiteral} from './type_literal'; export {TypeLiteral} from './type_literal'; @@ -26,6 +26,9 @@ export class Key { * @private */ constructor(token: Object, id: number) { + if (isBlank(token)) { + throw new BaseException('Token must be defined!'); + } this.token = token; this.id = id; } diff --git a/modules/angular2/test/core/forward_ref_integration_spec.dart b/modules/angular2/test/core/forward_ref_integration_spec.dart new file mode 100644 index 0000000000..d7e9e0c673 --- /dev/null +++ b/modules/angular2/test/core/forward_ref_integration_spec.dart @@ -0,0 +1,6 @@ +/// This file contains tests that make sense only in Dart +library angular2.test.core.forward_ref_integration_spec; + +main() { + // Don't run in Dart as it is not relevant, and Dart const rules prevent us from expressing it. +} diff --git a/modules/angular2/test/core/forward_ref_integration_spec.es6 b/modules/angular2/test/core/forward_ref_integration_spec.es6 new file mode 100644 index 0000000000..c06835af89 --- /dev/null +++ b/modules/angular2/test/core/forward_ref_integration_spec.es6 @@ -0,0 +1,84 @@ +import { + AsyncTestCompleter, + beforeEach, + ddescribe, + describe, + expect, + iit, + inject, + it, + xit +} from 'angular2/test_lib'; +import {TestBed} from 'angular2/src/test_lib/test_bed'; +import {Directive, Component} from 'angular2/src/core/annotations_impl/annotations'; +import {Query} from 'angular2/src/core/annotations_impl/di'; +import {View} from 'angular2/src/core/annotations_impl/view'; +import {QueryList, NgFor} from 'angular2/angular2'; +import {Inject} from 'angular2/src/di/annotations_impl'; +import {forwardRef, resolveForwardRef, bind} from 'angular2/di'; +import {Type} from 'angular2/src/facade/lang'; + +export function main() { + describe("forwardRef integration", function () { + it('should instantiate components which are declared using forwardRef', inject( + [TestBed, AsyncTestCompleter], + (tb, async) => { + tb.createView(App).then((view) => { + view.detectChanges(); + expect(view.rootNodes).toHaveText('frame(lock)'); + async.done(); + }); + }) + ); + }); +} + +@Component({ + selector: 'app', + injectables: [ + forwardRef(() => Frame) + ] +}) +@View({ + template: ``, + directives: [ + bind(forwardRef(() => Door)).toClass(forwardRef(() => Door)), + bind(forwardRef(() => Lock)).toClass(forwardRef(() => Lock)) + ] +}) +class App { +} + +@Component({ + selector: 'Lock' +}) +@View({ + directives: [NgFor], + template: `{{frame.name}}({{lock.name}})` +}) +class Door { + locks: QueryList; + frame: Frame; + + constructor(@Query(forwardRef(() => Lock)) locks: QueryList, @Inject(forwardRef(() => Frame)) frame:Frame) { + this.frame = frame; + this.locks = locks; + } +} + +class Frame { + name: string; + constructor() { + this.name = 'frame'; + } +} + +@Directive({ + selector: 'lock' +}) +class Lock { + name: string; + constructor() { + this.name = 'lock'; + } +} diff --git a/modules/angular2/test/di/forward_ref_spec.js b/modules/angular2/test/di/forward_ref_spec.js new file mode 100644 index 0000000000..df00947c45 --- /dev/null +++ b/modules/angular2/test/di/forward_ref_spec.js @@ -0,0 +1,23 @@ +import { + AsyncTestCompleter, + beforeEach, + ddescribe, + describe, + expect, + iit, + inject, + it, + xit, +} from 'angular2/test_lib'; +import {forwardRef, resolveForwardRef} from 'angular2/di'; +import {Type} from 'angular2/src/facade/lang'; + +export function main() { + describe("forwardRef", function () { + it('should wrap and unwrap the reference', () => { + var ref = forwardRef(() => String); + expect(ref instanceof Type).toBe(true); + expect(resolveForwardRef(ref)).toBe(String); + }); + }); +} diff --git a/modules/angular2/test/di/injector_spec.js b/modules/angular2/test/di/injector_spec.js index 039c9344b0..93daaacd95 100644 --- a/modules/angular2/test/di/injector_spec.js +++ b/modules/angular2/test/di/injector_spec.js @@ -1,6 +1,6 @@ import {isBlank, BaseException} from 'angular2/src/facade/lang'; import {describe, ddescribe, it, iit, expect, beforeEach} from 'angular2/test_lib'; -import {Injector, bind, ResolvedBinding} from 'angular2/di'; +import {Injector, bind, ResolvedBinding, Key, forwardRef} from 'angular2/di'; import {Optional, Inject, InjectLazy} from 'angular2/src/di/annotations_impl'; @@ -382,13 +382,32 @@ export function main() { }); describe('resolve', function() { - it('should resolve and flatten', function() { + it('should resolve and flatten', () => { var bindings = Injector.resolve([Engine, [BrokenEngine]]); bindings.forEach(function(b) { if (isBlank(b)) return; // the result is a sparse array expect(b instanceof ResolvedBinding).toBe(true); }); }); + + it('should resolve forward references', () => { + var bindings = Injector.resolve([ + forwardRef(() => Engine), + [ bind(forwardRef(() => BrokenEngine)).toClass(forwardRef(() => Engine)) ], + bind(forwardRef(() => String)).toFactory(() => 'OK', [forwardRef(() => Engine)]), + bind(forwardRef(() => DashboardSoftware)).toAsyncFactory(() => 123, [forwardRef(() => BrokenEngine)]) + ]); + + var engineBinding = bindings[Key.get(Engine).id]; + var brokenEngineBinding = bindings[Key.get(BrokenEngine).id]; + var stringBinding = bindings[Key.get(String).id]; + var dashboardSoftwareBinding = bindings[Key.get(DashboardSoftware).id]; + + expect(engineBinding.factory() instanceof Engine).toBe(true); + expect(brokenEngineBinding.factory() instanceof Engine).toBe(true); + expect(stringBinding.dependencies[0].key).toEqual(Key.get(Engine)); + expect(dashboardSoftwareBinding.dependencies[0].key).toEqual(Key.get(BrokenEngine)); + }); }); }); }