From 872eed7e1174c5b5997b4b4eb685784283fea44b Mon Sep 17 00:00:00 2001 From: Trey Date: Fri, 30 Nov 2012 10:55:28 -0500 Subject: [PATCH 1/2] This generally improves Angular routes in a backwards compatible way. Addresses: - A TODO where most routes was being compiled and processed every single route change. Routes are now processed up front and simply matched - Routes are now processed in the order in which they are added instead of arbitrarily. - Helps with #1147 - Helps with #1159 - Makes #918 easier - It is now possible to match the forwardslash character. - It is now possible to restrict your match to string, int or path. --- src/ng/route.js | 108 ++++++++++++++++++++++++++++-------------------- 1 file changed, 64 insertions(+), 44 deletions(-) diff --git a/src/ng/route.js b/src/ng/route.js index e2a9c6333743..7a5e39e078d8 100644 --- a/src/ng/route.js +++ b/src/ng/route.js @@ -11,7 +11,43 @@ * Used for configuring routes. See {@link ng.$route $route} for an example. */ function $RouteProvider(){ - var routes = {}; + var routes = []; + var nullRoute; + + var routeNewRe = new RegExp('\\<(\\w+)\\:(\\w+)\\>'); + var pathTypes = { + 'string': '\\w+', + 'int': '\\d+', + 'path': '[\\w\\/]+' + }; + + function compileRoute(path, route) { + var reNames = []; + var reParts = []; + var routeParts = path.split('/'); + forEach(routeParts, function(part) { + if (part[0] === '<') { + // This is a new-style route part. + var match = part.match(routeNewRe); + reParts.push('(' + pathTypes[match[1]] + ')'); + reNames.push(match[2]); + } else if (part[0] == ':') { + // Assume old style route part + reParts.push('(' + pathTypes['string'] + ')'); + reNames.push(part.slice(1)) + } else { + // Plain string match, not captured. + reParts.push(part.replace(/([\.\\\(\)\^\$])/g, "\\$1")); + //reNames.push(null); + } + }); + // Join things together into a spec that we can reuse. + return { + re: new RegExp('^' + reParts.join('\\/') + '$'), + matches: reNames, + route: route + } + } /** * @ngdoc method @@ -72,17 +108,12 @@ function $RouteProvider(){ * Adds a new route definition to the `$route` service. */ this.when = function(path, route) { - routes[path] = extend({reloadOnSearch: true}, route); - + routes.push(compileRoute(path, extend({reloadOnSearch: true}, route))); // create redirection for trailing slashes - if (path) { - var redirectPath = (path[path.length-1] == '/') - ? path.substr(0, path.length-1) - : path +'/'; - - routes[redirectPath] = {redirectTo: path}; - } - + var redirectPath = (path[path.length-1] == '/') + ? path.substr(0, path.length-1) + : path +'/'; + routes.push(compileRoute(redirectPath, {redirectTo: path})); return this; }; @@ -99,7 +130,8 @@ function $RouteProvider(){ * @returns {Object} self */ this.otherwise = function(params) { - this.when(null, params); + //this.when(null, params); + nullRoute = params; return this; }; @@ -316,24 +348,16 @@ function $RouteProvider(){ ///////////////////////////////////////////////////// function switchRouteMatcher(on, when) { - // TODO(i): this code is convoluted and inefficient, we should construct the route matching - // regex only once and then reuse it - var regex = '^' + when.replace(/([\.\\\(\)\^\$])/g, "\\$1") + '$', - params = [], - dst = {}; - forEach(when.split(/\W/), function(param) { - if (param) { - var paramRegExp = new RegExp(":" + param + "([\\W])"); - if (regex.match(paramRegExp)) { - regex = regex.replace(paramRegExp, "([^\\/]*)$1"); - params.push(param); - } - } - }); - var match = on.match(new RegExp(regex)); + var params = when.matches, + dst = {}, + match; + match = on.match(when.re); + if (match) { forEach(params, function(name, index) { - dst[name] = match[index + 1]; + if (name) { + dst[name] = match[index + 1]; + } }); } return match ? dst : null; @@ -410,42 +434,38 @@ function $RouteProvider(){ } } - /** * @returns the current active route, by matching it against the URL */ function parseRoute() { // Match a route var params, match; - forEach(routes, function(route, path) { - if (!match && (params = matcher($location.path(), path))) { - match = inherit(route, { + forEach(routes, function(pathMatch) { + if (!match && (params = matcher($location.path(), pathMatch))) { + match = inherit(pathMatch.route, { params: extend({}, $location.search(), params), pathParams: params}); - match.$route = route; + match.$route = pathMatch.route; } }); // No route matched; fallback to "otherwise" route - return match || routes[null] && inherit(routes[null], {params: {}, pathParams:{}}); + return match || nullRoute && inherit(nullRoute, {params: {}, pathParams:{}}); } /** * @returns interpolation of the redirect path with the parametrs */ function interpolate(string, params) { - var result = []; - forEach((string||'').split(':'), function(segment, i) { - if (i == 0) { - result.push(segment); + var result = [], inx, param; + forEach((string || '').split('/'), function(part) { + if ((inx = part.indexOf(':')) >= 0) { + param = part.slice(inx+1, part[part.length-1] == '>' ? -1 : undefined); + result.push(params[param]); } else { - var segmentMatch = segment.match(/(\w+)(.*)/); - var key = segmentMatch[1]; - result.push(params[key]); - result.push(segmentMatch[2] || ''); - delete params[key]; + result.push(part); } }); - return result.join(''); + return result.join('/'); } }]; } From 2d8cd06b27761e47f2214398647d65922611bfb4 Mon Sep 17 00:00:00 2001 From: Trey Date: Fri, 30 Nov 2012 12:03:24 -0500 Subject: [PATCH 2/2] Adding tests to support the advanced routing, fixing a small search bug. --- src/ng/route.js | 13 ++++----- test/ng/routeSpec.js | 63 ++++++++++++++++++++++++++++++++++++++++++++ 2 files changed, 70 insertions(+), 6 deletions(-) diff --git a/src/ng/route.js b/src/ng/route.js index 7a5e39e078d8..c430acc61f1d 100644 --- a/src/ng/route.js +++ b/src/ng/route.js @@ -31,10 +31,10 @@ function $RouteProvider(){ var match = part.match(routeNewRe); reParts.push('(' + pathTypes[match[1]] + ')'); reNames.push(match[2]); - } else if (part[0] == ':') { + } else if (part[0] === ':') { // Assume old style route part - reParts.push('(' + pathTypes['string'] + ')'); - reNames.push(part.slice(1)) + reParts.push('(' + pathTypes.string + ')'); + reNames.push(part.slice(1)); } else { // Plain string match, not captured. reParts.push(part.replace(/([\.\\\(\)\^\$])/g, "\\$1")); @@ -46,7 +46,7 @@ function $RouteProvider(){ re: new RegExp('^' + reParts.join('\\/') + '$'), matches: reNames, route: route - } + }; } /** @@ -110,7 +110,7 @@ function $RouteProvider(){ this.when = function(path, route) { routes.push(compileRoute(path, extend({reloadOnSearch: true}, route))); // create redirection for trailing slashes - var redirectPath = (path[path.length-1] == '/') + var redirectPath = (path[path.length-1] === '/') ? path.substr(0, path.length-1) : path +'/'; routes.push(compileRoute(redirectPath, {redirectTo: path})); @@ -459,8 +459,9 @@ function $RouteProvider(){ var result = [], inx, param; forEach((string || '').split('/'), function(part) { if ((inx = part.indexOf(':')) >= 0) { - param = part.slice(inx+1, part[part.length-1] == '>' ? -1 : undefined); + param = part.slice(inx+1, part[part.length-1] === '>' ? -1 : undefined); result.push(params[param]); + delete params[param]; } else { result.push(part); } diff --git a/test/ng/routeSpec.js b/test/ng/routeSpec.js index 52fffa406b69..6623b2c71442 100644 --- a/test/ng/routeSpec.js +++ b/test/ng/routeSpec.js @@ -733,5 +733,68 @@ describe('$route', function() { }); }); }); + + describe('advancedMatching', function() { + + it('should properly match path parts', function() { + module(function($routeProvider) { + $routeProvider.when('/foo//', {templateUrl: 'foo.html'}); + }); + + inject(function($route, $location, $rootScope) { + $location.path('/foo/id3/eId/rest/of/url/'); + $rootScope.$digest(); + + expect($location.path()).toEqual('/foo/id3/eId/rest/of/url'); + expect($route.current.params).toEqual({subid: 'id3/eId/rest/of', subsubid: 'url'}); + expect($route.current.templateUrl).toEqual('foo.html'); + }); + }); + + it('should properly match trailing paths', function() { + module(function($routeProvider) { + $routeProvider.when('/bar/:id//', {templateUrl: 'bar.html'}); + }); + + inject(function($route, $location, $rootScope) { + $location.path('/bar/id3/eId/rest/of/url/'); + $rootScope.$digest(); + + expect($location.path()).toEqual('/bar/id3/eId/rest/of/url/'); + expect($route.current.params).toEqual({ id : 'id3', subid : 'eId', subsubid : 'rest/of/url/' }); + expect($route.current.templateUrl).toEqual('bar.html'); + }); + }); + + it('should properly filter based on match type', function() { + module(function($routeProvider) { + $routeProvider.when('/bar//:astring', {templateUrl: 'bar.html'}); + }); + + inject(function($route, $location, $rootScope) { + $location.path('/bar/word/trailing'); + $rootScope.$digest(); + + expect($location.path()).toEqual('/bar/word/trailing'); + expect($route.current).not.toBeDefined(); + }); + }); + + it('should properly resolve ambiguous urls in order', function() { + module(function($routeProvider) { + $routeProvider.when('/bar//const', {templateUrl: 'bar.html'}); + $routeProvider.when('/bar//:astring', {templateUrl: 'foo.html'}); + }); + + inject(function($route, $location, $rootScope) { + $location.path('/bar/500/const'); + $rootScope.$digest(); + + expect($route.current.templateUrl).toEqual('bar.html'); + }); + }); + + }); + }); });