From f55782eaa774181a1ee8b64f16d31fe605830b8f Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Sun, 31 Jan 2016 15:35:23 +0000 Subject: [PATCH 1/5] fix(angular1-router): add support for using the component helper In Angular 1.5 there is a new helper method for creating component directives. See https://docs.angularjs.org/guide/component for more information about components. These kind of directives only match the `E` element form and the previously component router only created HTML that matched directives that matched the `A` attribute form. This commit changes the `` directive so that it generates custom HTML elements rather divs with custom attributes to trigger the relevant component to appear in the DOM. Going forward, Angular 1.5 users are encouraged to create their router components using the following style: ``` myModule.componnet('component-name', { // component definition object }); ``` Closes angular/angular.js#13860 Closes #6076 Closes #5278 BREAKING CHANGE: The component router now creates custom element HTML rather than custom attribute HTML, in order to create a new component. So rather than ```html
``` it now creates ```html ``` If you defined you router components using the `directive()` helper and specified the `restrict` properties such that element matching was not allowed, e.g. `restrict: 'A'` then these components will no longer be instantiated by the component router and the outlet will be empty. The fix is to include `E` in the `restrict` property. `restrict: 'EA'` Note that this does not affect directives that did not specify the `restrict` property as the default for this property is already `EA`. --- modules/angular1_router/src/ng_outlet.ts | 3 +- .../test/integration/navigation_spec.js | 87 +++++++++++++------ npm-shrinkwrap.clean.json | 8 +- npm-shrinkwrap.json | 34 ++++---- package.json | 6 +- 5 files changed, 88 insertions(+), 50 deletions(-) diff --git a/modules/angular1_router/src/ng_outlet.ts b/modules/angular1_router/src/ng_outlet.ts index 6184f7655a3e..7c450bb40d2f 100644 --- a/modules/angular1_router/src/ng_outlet.ts +++ b/modules/angular1_router/src/ng_outlet.ts @@ -155,7 +155,8 @@ function ngOutletDirective($animate, $q: ng.IQService, $router) { } this.controller.$$routeParams = instruction.params; - this.controller.$$template = '
'; + this.controller.$$template = + '<' + dashCase(componentName) + '>'; this.controller.$$router = this.router.childRouter(instruction.componentType); let newScope = scope.$new(); diff --git a/modules/angular1_router/test/integration/navigation_spec.js b/modules/angular1_router/test/integration/navigation_spec.js index 3d230c4b1290..51be632a7209 100644 --- a/modules/angular1_router/test/integration/navigation_spec.js +++ b/modules/angular1_router/test/integration/navigation_spec.js @@ -21,17 +21,21 @@ describe('navigation', function () { $router = _$router_; }); - registerComponent('userCmp', { + registerDirective('userCmp', { template: '
hello {{userCmp.$routeParams.name}}
' }); - registerComponent('oneCmp', { + registerDirective('oneCmp', { template: '
{{oneCmp.number}}
', controller: function () {this.number = 'one'} }); - registerComponent('twoCmp', { + registerDirective('twoCmp', { template: '
{{twoCmp.number}}
', controller: function () {this.number = 'two'} }); + registerComponent('threeCmp', { + template: '
{{$ctrl.number}}
', + controller: function () {this.number = 'three'} + }); }); it('should work in a simple case', function () { @@ -47,6 +51,21 @@ describe('navigation', function () { expect(elt.text()).toBe('one'); }); + + it('should work with components created by the `mod.component()` helper', function () { + compile(''); + + $router.config([ + { path: '/', component: 'threeCmp' } + ]); + + $router.navigateByUrl('/'); + $rootScope.$digest(); + + expect(elt.text()).toBe('three'); + }); + + it('should navigate between components with different parameters', function () { $router.config([ { path: '/user/:name', component: 'userCmp' } @@ -68,7 +87,7 @@ describe('navigation', function () { function ParentController() { instanceCount += 1; } - registerComponent('parentCmp', { + registerDirective('parentCmp', { template: 'parent { }', $routeConfig: [ { path: '/user/:name', component: 'userCmp' } @@ -94,7 +113,7 @@ describe('navigation', function () { it('should work with nested outlets', function () { - registerComponent('childCmp', { + registerDirective('childCmp', { template: '
inner {
}
', $routeConfig: [ { path: '/b', component: 'oneCmp' } @@ -114,7 +133,7 @@ describe('navigation', function () { it('should work with recursive nested outlets', function () { - registerComponent('recurCmp', { + registerDirective('recurCmp', { template: '
recur {
}
', $routeConfig: [ { path: '/recur', component: 'recurCmp' }, @@ -163,7 +182,7 @@ describe('navigation', function () { it('should change location to the canonical route with nested components', inject(function ($location) { - registerComponent('childRouter', { + registerDirective('childRouter', { template: '
inner {
}
', $routeConfig: [ { path: '/new-child', component: 'oneCmp', name: 'NewChild'}, @@ -208,7 +227,7 @@ describe('navigation', function () { it('should expose a "navigating" property on $router', inject(function ($q) { var defer; - registerComponent('pendingActivate', { + registerDirective('pendingActivate', { $canActivate: function () { defer = $q.defer(); return defer.promise; @@ -227,31 +246,26 @@ describe('navigation', function () { expect($router.navigating).toBe(false); })); - function registerComponent(name, options) { - var controller = options.controller || function () {}; - - ['$routerOnActivate', '$routerOnDeactivate', '$routerOnReuse', '$routerCanReuse', '$routerCanDeactivate'].forEach(function (hookName) { - if (options[hookName]) { - controller.prototype[hookName] = options[hookName]; - } - }); - + function registerDirective(name, options) { function factory() { return { template: options.template || '', controllerAs: name, - controller: controller + controller: getController(options) }; } + applyStaticProperties(factory, options); + $compileProvider.directive(name, factory); + } - if (options.$canActivate) { - factory.$canActivate = options.$canActivate; - } - if (options.$routeConfig) { - factory.$routeConfig = options.$routeConfig; - } + function registerComponent(name, options) { - $compileProvider.directive(name, factory); + var definition = { + template: options.template || '', + controller: getController(options), + } + applyStaticProperties(definition, options); + $compileProvider.component(name, definition); } function compile(template) { @@ -259,4 +273,27 @@ describe('navigation', function () { $rootScope.$digest(); return elt; } + + function getController(options) { + var controller = options.controller || function () {}; + [ + '$routerOnActivate', '$routerOnDeactivate', + '$routerOnReuse', '$routerCanReuse', + '$routerCanDeactivate' + ].forEach(function (hookName) { + if (options[hookName]) { + controller.prototype[hookName] = options[hookName]; + } + }); + return controller; + } + + function applyStaticProperties(target, options) { + ['$canActivate', '$routeConfig'].forEach(function(property) { + if (options[property]) { + target[property] = options[property]; + } + }); + } }); + diff --git a/npm-shrinkwrap.clean.json b/npm-shrinkwrap.clean.json index 2e81c5571242..fccea7ba8bab 100644 --- a/npm-shrinkwrap.clean.json +++ b/npm-shrinkwrap.clean.json @@ -46,13 +46,13 @@ "version": "1.0.0" }, "angular": { - "version": "1.4.8" + "version": "1.5.0" }, "angular-animate": { - "version": "1.4.8" + "version": "1.5.0" }, "angular-mocks": { - "version": "1.4.8" + "version": "1.5.0" }, "ansi": { "version": "0.3.0" @@ -5828,5 +5828,5 @@ } }, "name": "angular-srcs", - "version": "2.0.0-beta.2" + "version": "2.0.0-beta.3" } diff --git a/npm-shrinkwrap.json b/npm-shrinkwrap.json index dbcceace50b0..def5bc54f362 100644 --- a/npm-shrinkwrap.json +++ b/npm-shrinkwrap.json @@ -1,6 +1,6 @@ { "name": "angular-srcs", - "version": "2.0.0-beta.2", + "version": "2.0.0-beta.3", "dependencies": { "abbrev": { "version": "1.0.7", @@ -74,19 +74,19 @@ "resolved": "https://registry.npmjs.org/amdefine/-/amdefine-1.0.0.tgz" }, "angular": { - "version": "1.4.8", - "from": "angular@>=1.4.7 <2.0.0", - "resolved": "https://registry.npmjs.org/angular/-/angular-1.4.8.tgz" + "version": "1.5.0", + "from": "angular@1.5.0", + "resolved": "https://registry.npmjs.org/angular/-/angular-1.5.0.tgz" }, "angular-animate": { - "version": "1.4.8", - "from": "angular-animate@>=1.4.7 <2.0.0", - "resolved": "https://registry.npmjs.org/angular-animate/-/angular-animate-1.4.8.tgz" + "version": "1.5.0", + "from": "angular-animate@1.5.0", + "resolved": "https://registry.npmjs.org/angular-animate/-/angular-animate-1.5.0.tgz" }, "angular-mocks": { - "version": "1.4.8", - "from": "angular-mocks@>=1.4.7 <2.0.0", - "resolved": "https://registry.npmjs.org/angular-mocks/-/angular-mocks-1.4.8.tgz" + "version": "1.5.0", + "from": "angular-mocks@1.5.0", + "resolved": "https://registry.npmjs.org/angular-mocks/-/angular-mocks-1.5.0.tgz" }, "ansi": { "version": "0.3.0", @@ -7668,7 +7668,7 @@ }, "rxjs": { "version": "5.0.0-beta.0", - "from": "rxjs@5.0.0-beta.0", + "from": "https://registry.npmjs.org/rxjs/-/rxjs-5.0.0-beta.0.tgz", "resolved": "https://registry.npmjs.org/rxjs/-/rxjs-5.0.0-beta.0.tgz" }, "sass-graph": { @@ -8519,29 +8519,29 @@ }, "ts2dart": { "version": "0.7.22", - "from": "ts2dart@>=0.7.22 <0.8.0", + "from": "https://registry.npmjs.org/ts2dart/-/ts2dart-0.7.22.tgz", "resolved": "https://registry.npmjs.org/ts2dart/-/ts2dart-0.7.22.tgz", "dependencies": { "source-map": { "version": "0.4.4", - "from": "source-map@>=0.4.2 <0.5.0", + "from": "https://registry.npmjs.org/source-map/-/source-map-0.4.4.tgz", "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.4.4.tgz" }, "source-map-support": { "version": "0.3.3", - "from": "source-map-support@>=0.3.1 <0.4.0", + "from": "https://registry.npmjs.org/source-map-support/-/source-map-support-0.3.3.tgz", "resolved": "https://registry.npmjs.org/source-map-support/-/source-map-support-0.3.3.tgz", "dependencies": { "source-map": { "version": "0.1.32", - "from": "source-map@0.1.32", + "from": "https://registry.npmjs.org/source-map/-/source-map-0.1.32.tgz", "resolved": "https://registry.npmjs.org/source-map/-/source-map-0.1.32.tgz" } } }, "typescript": { "version": "1.7.3", - "from": "typescript@1.7.3", + "from": "https://registry.npmjs.org/typescript/-/typescript-1.7.3.tgz", "resolved": "https://registry.npmjs.org/typescript/-/typescript-1.7.3.tgz" } } @@ -9292,7 +9292,7 @@ }, "zone.js": { "version": "0.5.13", - "from": "zone.js@0.5.13", + "from": "https://registry.npmjs.org/zone.js/-/zone.js-0.5.13.tgz", "resolved": "https://registry.npmjs.org/zone.js/-/zone.js-0.5.13.tgz" } } diff --git a/package.json b/package.json index 93c8e91f1878..e659aa913668 100644 --- a/package.json +++ b/package.json @@ -39,9 +39,9 @@ "zone.js": "0.5.13" }, "devDependencies": { - "angular": "^1.4.7", - "angular-animate": "^1.4.7", - "angular-mocks": "^1.4.7", + "angular": "^1.5.0", + "angular-animate": "^1.5.0", + "angular-mocks": "^1.5.0", "base64-js": "^0.0.8", "bower": "^1.3.12", "broccoli": "^0.16.9", From 814cf0b6e626b07d992ef414a080d49dd53d252d Mon Sep 17 00:00:00 2001 From: Alexander Bachmann Date: Wed, 27 Jan 2016 21:05:34 +0100 Subject: [PATCH 2/5] fix(router): don't prepend `/` unnecessarily to Location paths Closes #6729 Closes #5502 --- .../test/integration/navigation_spec.js | 20 +++++++++++++++++++ modules/angular2/src/router/router.ts | 4 ++-- .../router/integration/navigation_spec.ts | 14 +++++++++++++ 3 files changed, 36 insertions(+), 2 deletions(-) diff --git a/modules/angular1_router/test/integration/navigation_spec.js b/modules/angular1_router/test/integration/navigation_spec.js index 51be632a7209..a26670327041 100644 --- a/modules/angular1_router/test/integration/navigation_spec.js +++ b/modules/angular1_router/test/integration/navigation_spec.js @@ -131,6 +131,26 @@ describe('navigation', function () { expect(elt.text()).toBe('outer { inner { one } }'); }); + it('should work when parent route has empty path', inject(function ($location) { + registerComponent('childCmp', { + template: '
inner {
}
', + $routeConfig: [ + { path: '/b', component: 'oneCmp' } + ] + }); + + $router.config([ + { path: '/...', component: 'childCmp' } + ]); + compile('
outer {
}
'); + + $router.navigateByUrl('/b'); + $rootScope.$digest(); + + expect(elt.text()).toBe('outer { inner { one } }'); + expect($location.path()).toBe('/b'); + })); + it('should work with recursive nested outlets', function () { registerDirective('recurCmp', { diff --git a/modules/angular2/src/router/router.ts b/modules/angular2/src/router/router.ts index a1bd9b041606..d0f2ce3d16aa 100644 --- a/modules/angular2/src/router/router.ts +++ b/modules/angular2/src/router/router.ts @@ -438,7 +438,7 @@ export class RootRouter extends Router { } var emitPath = instruction.toUrlPath(); var emitQuery = instruction.toUrlQuery(); - if (emitPath.length > 0) { + if (emitPath.length > 0 && emitPath[0] != '/') { emitPath = '/' + emitPath; } @@ -465,7 +465,7 @@ export class RootRouter extends Router { commit(instruction: Instruction, _skipLocationChange: boolean = false): Promise { var emitPath = instruction.toUrlPath(); var emitQuery = instruction.toUrlQuery(); - if (emitPath.length > 0) { + if (emitPath.length > 0 && emitPath[0] != '/') { emitPath = '/' + emitPath; } var promise = super.commit(instruction); diff --git a/modules/angular2/test/router/integration/navigation_spec.ts b/modules/angular2/test/router/integration/navigation_spec.ts index 8e649ec9c26e..78c07cfb7d02 100644 --- a/modules/angular2/test/router/integration/navigation_spec.ts +++ b/modules/angular2/test/router/integration/navigation_spec.ts @@ -105,6 +105,20 @@ export function main() { }); })); + it('should navigate to child routes when the root component has an empty path', + inject([AsyncTestCompleter, Location], (async, location) => { + compile(tcb, 'outer { }') + .then((rtc) => {fixture = rtc}) + .then((_) => rtr.config([new Route({path: '/...', component: ParentCmp})])) + .then((_) => rtr.navigateByUrl('/b')) + .then((_) => { + fixture.detectChanges(); + expect(fixture.debugElement.nativeElement).toHaveText('outer { inner { hello } }'); + expect(location.urlChanges).toEqual(['/b']); + async.done(); + }); + })); + it('should navigate to child routes of async routes', inject([AsyncTestCompleter], (async) => { compile(tcb, 'outer { }') .then((rtc) => {fixture = rtc}) From d7c5aeb6b598e04ea1bec444e4d43a5a37f27ede Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Sun, 7 Feb 2016 20:40:00 +0000 Subject: [PATCH 3/5] feat(angular1_router): allow component to bind to router --- modules/angular1_router/src/ng_outlet.ts | 3 +- .../test/integration/router_spec.js | 78 +++++++++++++++---- 2 files changed, 65 insertions(+), 16 deletions(-) diff --git a/modules/angular1_router/src/ng_outlet.ts b/modules/angular1_router/src/ng_outlet.ts index 7c450bb40d2f..62bfe20635fd 100644 --- a/modules/angular1_router/src/ng_outlet.ts +++ b/modules/angular1_router/src/ng_outlet.ts @@ -156,10 +156,11 @@ function ngOutletDirective($animate, $q: ng.IQService, $router) { this.controller.$$routeParams = instruction.params; this.controller.$$template = - '<' + dashCase(componentName) + '>'; + '<' + dashCase(componentName) + ' router="$$router">'; this.controller.$$router = this.router.childRouter(instruction.componentType); let newScope = scope.$new(); + newScope.$$router = this.controller.$$router; let clone = $transclude(newScope, clone => { $animate.enter(clone, null, this.currentElement || element); diff --git a/modules/angular1_router/test/integration/router_spec.js b/modules/angular1_router/test/integration/router_spec.js index c302c21cea62..bea802f366b6 100644 --- a/modules/angular1_router/test/integration/router_spec.js +++ b/modules/angular1_router/test/integration/router_spec.js @@ -44,31 +44,57 @@ describe('router', function () { expect(elt.text()).toBe('Home'); })); - function registerComponent(name, options) { - var controller = options.controller || function () {}; - ['$onActivate', '$onDeactivate', '$onReuse', '$canReuse', '$canDeactivate'].forEach(function (hookName) { - if (options[hookName]) { - controller.prototype[hookName] = options[hookName]; - } + it('should bind the component to the current router', inject(function($location) { + var router; + registerComponent('homeCmp', { + bindings: { router: '=' }, + controller: function($scope, $element) { + this.$routerOnActivate = function() { + router = this.router; + }; + }, + template: 'Home' }); + registerComponent('app', { + template: '
', + $routeConfig: [ + { path: '/', component: 'homeCmp' } + ] + }); + + compile(''); + + $location.path('/'); + $rootScope.$digest(); + var homeElement = elt.find('home-cmp'); + expect(homeElement.text()).toBe('Home'); + expect(homeElement.isolateScope().$ctrl.router).toBeDefined(); + expect(router).toBeDefined(); + })); + + function registerDirective(name, options) { function factory() { return { template: options.template || '', controllerAs: name, - controller: controller + controller: getController(options) }; } + applyStaticProperties(factory, options); + $compileProvider.directive(name, factory); + } - if (options.$canActivate) { - factory.$canActivate = options.$canActivate; - } - if (options.$routeConfig) { - factory.$routeConfig = options.$routeConfig; - } + function registerComponent(name, options) { - $compileProvider.directive(name, factory); + var definition = { + bindings: options.bindings, + template: options.template || '', + controller: getController(options), + } + applyStaticProperties(definition, options); + $compileProvider.component(name, definition); } function compile(template) { @@ -76,4 +102,26 @@ describe('router', function () { $rootScope.$digest(); return elt; } -}); + + function getController(options) { + var controller = options.controller || function () {}; + [ + '$routerOnActivate', '$routerOnDeactivate', + '$routerOnReuse', '$routerCanReuse', + '$routerCanDeactivate' + ].forEach(function (hookName) { + if (options[hookName]) { + controller.prototype[hookName] = options[hookName]; + } + }); + return controller; + } + + function applyStaticProperties(target, options) { + ['$canActivate', '$routeConfig'].forEach(function(property) { + if (options[property]) { + target[property] = options[property]; + } + }); + } +}); \ No newline at end of file From 35efe5158bc1f939ba61df0b885ac5437b560936 Mon Sep 17 00:00:00 2001 From: Peter Bacon Darwin Date: Sun, 7 Feb 2016 22:09:06 +0000 Subject: [PATCH 4/5] test(angular1_router): test that location handles query strings See 6698 --- .../test/integration/navigation_spec.js | 36 +++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/modules/angular1_router/test/integration/navigation_spec.js b/modules/angular1_router/test/integration/navigation_spec.js index a26670327041..70bcf2eeee04 100644 --- a/modules/angular1_router/test/integration/navigation_spec.js +++ b/modules/angular1_router/test/integration/navigation_spec.js @@ -36,6 +36,14 @@ describe('navigation', function () { template: '
{{$ctrl.number}}
', controller: function () {this.number = 'three'} }); + registerComponent('getParams', { + template: '
{{$ctrl.params.x}}
', + controller: function () { + this.$routerOnActivate = function(next) { + this.params = next.params; + }; + } + }) }); it('should work in a simple case', function () { @@ -186,6 +194,21 @@ describe('navigation', function () { })); + it('should pass through query terms to the location', inject(function ($location) { + $router.config([ + { path: '/user', component: 'userCmp' } + ]); + + compile('
'); + + $router.navigateByUrl('/user?x=y'); + $rootScope.$digest(); + + expect($location.path()).toBe('/user'); + expect($location.search()).toEqual({ x: 'y'}); + })); + + it('should change location to the canonical route', inject(function ($location) { compile('
'); @@ -245,6 +268,19 @@ describe('navigation', function () { })); + it('should navigate when the location query changes', inject(function ($location) { + $router.config([ + { path: '/get/params', component: 'getParams' } + ]); + compile('
'); + + $location.url('/get/params?x=y'); + $rootScope.$digest(); + + expect(elt.text()).toBe('y'); + })); + + it('should expose a "navigating" property on $router', inject(function ($q) { var defer; registerDirective('pendingActivate', { From 321f200bba84d947247d781c86368d868ce9f49f Mon Sep 17 00:00:00 2001 From: David Reher Date: Tue, 26 Jan 2016 13:51:50 +0100 Subject: [PATCH 5/5] fix(router): fixed the location wrapper for angular1 In angular2 `Location.path()` returns the complete path including query string. In angular1 the query parameters are missing. Similar to this `Location.go` does accept two parameters (path *and query*). --- modules/angular1_router/lib/facades.es5 | 6 +++--- modules/angular1_router/src/module_template.js | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/modules/angular1_router/lib/facades.es5 b/modules/angular1_router/lib/facades.es5 index f419cdef5ff5..b84d86bf361a 100644 --- a/modules/angular1_router/lib/facades.es5 +++ b/modules/angular1_router/lib/facades.es5 @@ -303,8 +303,8 @@ Location.prototype.subscribe = function () { //TODO: implement }; Location.prototype.path = function () { - return $location.path(); + return $location.url(); }; -Location.prototype.go = function (url) { - return $location.path(url); +Location.prototype.go = function (path, query) { + return $location.url(path + query); }; diff --git a/modules/angular1_router/src/module_template.js b/modules/angular1_router/src/module_template.js index 43415af4d5b3..0139664bc752 100644 --- a/modules/angular1_router/src/module_template.js +++ b/modules/angular1_router/src/module_template.js @@ -57,7 +57,7 @@ function routerFactory($q, $location, $$directiveIntrospector, $browser, $rootSc }); var router = new RootRouter(registry, location, $routerRootComponent); - $rootScope.$watch(function () { return $location.path(); }, function (path) { + $rootScope.$watch(function () { return $location.url(); }, function (path) { if (router.lastNavigationAttempt !== path) { router.navigateByUrl(path); }