From adef68b4d6ab46f37a5b82679d0afe24ccbda49b Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Thu, 25 Feb 2016 13:03:29 +0000 Subject: [PATCH] refactor(angular_1_router): remove directiveIntrospector The directiveIntrospector was a bit of a hack to allow the router to read the `$routeConfig` annocation and `$routerCanActivate` hook from directives when they were registered. It turns out that if we put these properties on the component controller's constructor function (i.e. as static class methods) then we can simply use the `$injector` to access it as required. Currently, people put the properties directly on their component definition objects. In Angular 1.5.1, we will copy these properties onto the controller constructor to maintain a simple migration path. But going forward it may be better to encourage people to add the properties directly to the controller constructor. --- .../angular1_router/src/module_template.js | 54 ++++++++++++++----- modules/angular1_router/src/ng_outlet.ts | 52 ------------------ .../test/directive_introspector_spec.js | 38 ------------- 3 files changed, 41 insertions(+), 103 deletions(-) delete mode 100644 modules/angular1_router/test/directive_introspector_spec.js diff --git a/modules/angular1_router/src/module_template.js b/modules/angular1_router/src/module_template.js index 9757a0aec5..ca4352dff2 100644 --- a/modules/angular1_router/src/module_template.js +++ b/modules/angular1_router/src/module_template.js @@ -4,9 +4,9 @@ angular.module('ngComponentRouter'). // Because Angular 1 has no notion of a root component, we use an object with unique identity // to represent this. Can be overloaded with a component name value('$routerRootComponent', new Object()). - factory('$rootRouter', ['$q', '$location', '$$directiveIntrospector', '$browser', '$rootScope', '$injector', '$routerRootComponent', routerFactory]); + factory('$rootRouter', ['$q', '$location', '$browser', '$rootScope', '$injector', '$routerRootComponent', routerFactory]); -function routerFactory($q, $location, $$directiveIntrospector, $browser, $rootScope, $injector, $routerRootComponent) { +function routerFactory($q, $location, $browser, $rootScope, $injector, $routerRootComponent) { // When this file is processed, the line below is replaced with // the contents of `../lib/facades.es5`. @@ -23,11 +23,24 @@ function routerFactory($q, $location, $$directiveIntrospector, $browser, $rootSc // the contents of the compiled TypeScript classes. //{{SHARED_CODE}} + function getComponentConstructor(name) { + var serviceName = name + 'Directive'; + if ($injector.has(serviceName)) { + var definitions = $injector.get(serviceName); + if (definitions.length > 1) { + throw new BaseException('too many directives named "' + name + '"'); + } + return definitions[0].controller; + } else { + throw new BaseException('directive "' + name + '" is not registered'); + } + } + //TODO: this is a hack to replace the exiting implementation at run-time exports.getCanActivateHook = function (directiveName) { - var factory = $$directiveIntrospector.getTypeByName(directiveName); - return factory && factory.$canActivate && function (next, prev) { - return $injector.invoke(factory.$canActivate, null, { + var controller = getComponentConstructor(directiveName); + return controller.$canActivate && function (next, prev) { + return $injector.invoke(controller.$canActivate, null, { $nextInstruction: next, $prevInstruction: prev }); @@ -45,17 +58,32 @@ function routerFactory($q, $location, $$directiveIntrospector, $browser, $rootSc var RouteRegistry = exports.RouteRegistry; var RootRouter = exports.RootRouter; + // Override this method to actually get hold of the child routes + RouteRegistry.prototype.configFromComponent = function (component) { + var that = this; + if (isString(component)) { + // Don't read the annotations component a type more than once – + // this prevents an infinite loop if a component routes recursively. + if (this._rules.has(component)) { + return; + } + var controller = getComponentConstructor(component); + if (angular.isArray(controller.$routeConfig)) { + controller.$routeConfig.forEach(function (config) { + var loader = config.loader; + if (isPresent(loader)) { + config = angular.extend({}, config, { loader: () => $injector.invoke(loader) }); + } + that.config(component, config); + }); + } + } + + } + var registry = new RouteRegistry($routerRootComponent); var location = new Location(); - $$directiveIntrospector(function (name, factory) { - if (angular.isArray(factory.$routeConfig)) { - factory.$routeConfig.forEach(function (config) { - registry.config(name, config); - }); - } - }); - var router = new RootRouter(registry, location, $routerRootComponent); $rootScope.$watch(function () { return $location.url(); }, function (path) { if (router.lastNavigationAttempt !== path) { diff --git a/modules/angular1_router/src/ng_outlet.ts b/modules/angular1_router/src/ng_outlet.ts index 94ebfa8417..191bd61d0e 100644 --- a/modules/angular1_router/src/ng_outlet.ts +++ b/modules/angular1_router/src/ng_outlet.ts @@ -1,51 +1,6 @@ /// -/* - * decorates $compileProvider so that we have access to routing metadata - */ -function compilerProviderDecorator($compileProvider, - $$directiveIntrospectorProvider: DirectiveIntrospectorProvider) { - let directive = $compileProvider.directive; - $compileProvider.directive = function(name: string, factory: Function) { - $$directiveIntrospectorProvider.register(name, factory); - return directive.apply(this, arguments); - }; -} -/* - * private service that holds route mappings for each controller - */ -class DirectiveIntrospectorProvider { - private directiveBuffer: any[] = []; - private directiveFactoriesByName: {[name: string]: Function} = {}; - private onDirectiveRegistered: (name: string, factory: Function) => any = null; - - register(name: string, factory: Function) { - if (angular.isArray(factory)) { - factory = factory[factory.length - 1]; - } - this.directiveFactoriesByName[name] = factory; - if (this.onDirectiveRegistered) { - this.onDirectiveRegistered(name, factory); - } else { - this.directiveBuffer.push({name: name, factory: factory}); - } - } - - $get() { - let fn: any = newOnControllerRegistered => { - this.onDirectiveRegistered = newOnControllerRegistered; - while (this.directiveBuffer.length > 0) { - let directive = this.directiveBuffer.pop(); - this.onDirectiveRegistered(directive.name, directive.factory); - } - }; - - fn.getTypeByName = name => this.directiveFactoriesByName[name]; - - return fn; - } -} /** * @name ngOutlet @@ -303,10 +258,3 @@ angular.module('ngComponentRouter', []) .directive('ngOutlet', ['$compile', ngOutletFillContentDirective]) .directive('ngLink', ['$rootRouter', '$parse', ngLinkDirective]) .directive('$router', ['$q', routerTriggerDirective]); - -/* - * A module for inspecting controller constructors - */ -angular.module('ng') - .provider('$$directiveIntrospector', DirectiveIntrospectorProvider) - .config(['$compileProvider', '$$directiveIntrospectorProvider', compilerProviderDecorator]); diff --git a/modules/angular1_router/test/directive_introspector_spec.js b/modules/angular1_router/test/directive_introspector_spec.js deleted file mode 100644 index 5126643d66..0000000000 --- a/modules/angular1_router/test/directive_introspector_spec.js +++ /dev/null @@ -1,38 +0,0 @@ -'use strict'; - -describe('$$directiveIntrospector', function () { - - var $compileProvider; - - beforeEach(function() { - module('ng'); - module('ngComponentRouter'); - module(function(_$compileProvider_) { - $compileProvider = _$compileProvider_; - }); - }); - - it('should call the introspector function whenever a directive factory is registered', inject(function ($$directiveIntrospector) { - var spy = jasmine.createSpy(); - $$directiveIntrospector(spy); - function myDir(){} - $compileProvider.directive('myDir', myDir); - - expect(spy).toHaveBeenCalledWith('myDir', myDir); - })); - - it('should call the introspector function whenever a directive factory is registered with array annotations', inject(function ($$directiveIntrospector) { - var spy = jasmine.createSpy(); - $$directiveIntrospector(spy); - function myDir(){} - $compileProvider.directive('myDir', ['foo', myDir]); - - expect(spy).toHaveBeenCalledWith('myDir', myDir); - })); - - it('should retrieve a factory based on directive name', inject(function ($$directiveIntrospector) { - function myDir(){} - $compileProvider.directive('myDir', ['foo', myDir]); - expect($$directiveIntrospector.getTypeByName('myDir')).toBe(myDir); - })); -});