From 0cc1e6cc461a4640618e2bb594566551c54834e2 Mon Sep 17 00:00:00 2001 From: christopherthielen Date: Sun, 5 Oct 2014 20:52:23 -0500 Subject: [PATCH 1/7] feat($urlMatcherFactory): Made a Params and ParamSet class - make state.ownParams and params use Param type --- src/common.js | 14 ++++ src/state.js | 45 ++++-------- src/urlMatcherFactory.js | 143 ++++++++++++++++++++++++++++----------- 3 files changed, 132 insertions(+), 70 deletions(-) diff --git a/src/common.js b/src/common.js index 1dc0172ee..f3010ccd5 100644 --- a/src/common.js +++ b/src/common.js @@ -61,6 +61,20 @@ function objectKeys(object) { return result; } +/** + * like objectKeys, but includes keys from prototype chain. + * @param object the object whose prototypal keys will be returned + * @param ignoreKeys an array of keys to ignore + */ +function protoKeys(object, ignoreKeys) { + var result = []; + for (var key in object) { + if (!ignoreKeys || ignoreKeys.indexOf(key) === -1) + result.push(key); + } + return result; +} + /** * IE8-safe wrapper for `Array.prototype.indexOf()`. * diff --git a/src/state.js b/src/state.js index b5b56da9c..3257d3244 100644 --- a/src/state.js +++ b/src/state.js @@ -64,12 +64,19 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { return state.url ? state : (state.parent ? state.parent.navigable : null); }, + // Own parameters for this state. state.url.params is already built at this point. Create and add non-url params + ownParams: function(state) { + var params = state.url && state.url.params || new $$UrlMatcherFactoryProvider.ParamSet(); + forEach(state.params || {}, function(config, id) { + if (!params[id]) params[id] = new $$UrlMatcherFactoryProvider.Param(id, null, config); + }); + return params; + }, + // Derive parameters for this state and ensure they're a super-set of parent's parameters params: function(state) { - if (!state.params) { - return state.url ? state.url.params : state.parent.params; - } - return state.params; + var parentParams = state.parent && state.parent.params || new $$UrlMatcherFactoryProvider.ParamSet(); + return inherit(parentParams, state.ownParams); }, // If there is no explicit multi-view configuration, make one up so we don't have @@ -87,28 +94,6 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { return views; }, - ownParams: function(state) { - state.params = state.params || {}; - - if (!state.parent) { - return objectKeys(state.params); - } - var paramNames = {}; forEach(state.params, function (v, k) { paramNames[k] = true; }); - - forEach(state.parent.params, function (v, k) { - if (!paramNames[k]) { - throw new Error("Missing required parameter '" + k + "' in state '" + state.name + "'"); - } - paramNames[k] = false; - }); - var ownParams = []; - - forEach(paramNames, function (own, p) { - if (own) ownParams.push(p); - }); - return ownParams; - }, - // Keep a full path from the root down to this state as this is needed for state activation. path: function(state) { return state.parent ? state.parent.path.concat(state) : []; // exclude root from path @@ -801,7 +786,7 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { var keep = 0, state = toPath[keep], locals = root.locals, toLocals = []; if (!options.reload) { - while (state && state === fromPath[keep] && equalForKeys(toParams, fromParams, state.ownParams)) { + while (state && state === fromPath[keep] && equalForKeys(toParams, fromParams, state.ownParams.$$keys())) { locals = toLocals[keep] = state.locals; keep++; state = toPath[keep]; @@ -820,7 +805,7 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { } // Filter parameters before we pass them to event handlers etc. - toParams = filterByKeys(objectKeys(to.params), toParams || {}); + toParams = filterByKeys(to.params.$$keys(), toParams || {}); // Broadcast start event and cancel the transition if requested if (options.notify) { @@ -1133,7 +1118,7 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { if (!nav || nav.url === undefined || nav.url === null) { return null; } - return $urlRouter.href(nav.url, filterByKeys(objectKeys(state.params), params || {}), { + return $urlRouter.href(nav.url, filterByKeys(state.params.$$keys(), params || {}), { absolute: options.absolute }); }; @@ -1162,7 +1147,7 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { // necessary. In addition to being available to the controller and onEnter/onExit callbacks, // we also need $stateParams to be available for any $injector calls we make during the // dependency resolution process. - var $stateParams = (paramsAreFiltered) ? params : filterByKeys(objectKeys(state.params), params); + var $stateParams = (paramsAreFiltered) ? params : filterByKeys(state.params.$$keys(), params); var locals = { $stateParams: $stateParams }; // Resolve 'global' dependencies for the state, i.e. those not specific to a view. diff --git a/src/urlMatcherFactory.js b/src/urlMatcherFactory.js index 31d9d41ad..524c2db81 100644 --- a/src/urlMatcherFactory.js +++ b/src/urlMatcherFactory.js @@ -60,7 +60,7 @@ * @returns {Object} New `UrlMatcher` object */ function UrlMatcher(pattern, config) { - config = angular.isObject(config) ? config : {}; + config = extend({ params: {} }, isObject(config) ? config : {}); // Find all placeholders and create a compiled pattern, using either classic or curly syntax: // '*' name @@ -78,21 +78,13 @@ function UrlMatcher(pattern, config) { var placeholder = /([:*])(\w+)|\{(\w+)(?:\:((?:[^{}\\]+|\\.|\{(?:[^{}\\]+|\\.)*\})+))?\}/g, compiled = '^', last = 0, m, segments = this.segments = [], - params = this.params = {}; - - /** - * [Internal] Gets the decoded representation of a value if the value is defined, otherwise, returns the - * default value, which may be the result of an injectable function. - */ - function $value(value) { - /*jshint validthis: true */ - return isDefined(value) ? this.type.decode(value) : $UrlMatcherFactory.$$getDefaultValue(this); - } + params = this.params = new $$UrlMatcherFactoryProvider.ParamSet(); function addParameter(id, type, config) { if (!/^\w+(-+\w+)*$/.test(id)) throw new Error("Invalid parameter name '" + id + "' in pattern '" + pattern + "'"); if (params[id]) throw new Error("Duplicate parameter name '" + id + "' in pattern '" + pattern + "'"); - params[id] = extend({ type: type || new Type(), $value: $value }, config); + params[id] = new $$UrlMatcherFactoryProvider.Param(id, type, config); + return params[id]; } function quoteRegExp(string, pattern, isOptional) { @@ -102,12 +94,6 @@ function UrlMatcher(pattern, config) { return result + flag + '(' + pattern + ')' + flag; } - function paramConfig(param) { - if (!config.params || !config.params[param]) return {}; - var cfg = config.params[param]; - return isObject(cfg) ? cfg : { value: cfg }; - } - this.source = pattern; // Split into static segments separated by path parameter placeholders. @@ -119,12 +105,12 @@ function UrlMatcher(pattern, config) { regexp = m[4] || (m[1] == '*' ? '.*' : '[^/]*'); segment = pattern.substring(last, m.index); type = this.$types[regexp] || new Type({ pattern: new RegExp(regexp) }); - cfg = paramConfig(id); + cfg = config.params[id]; if (segment.indexOf('?') >= 0) break; // we're into the search part - compiled += quoteRegExp(segment, type.$subPattern(), isDefined(cfg.value)); - addParameter(id, type, cfg); + var param = addParameter(id, type, cfg); + compiled += quoteRegExp(segment, type.$subPattern(), param.isOptional); segments.push(segment); last = placeholder.lastIndex; } @@ -140,7 +126,7 @@ function UrlMatcher(pattern, config) { // Allow parameters to be separated by '?' as well as '&' to make concat() easier forEach(search.substring(1).split(/[&?]/), function(key) { - addParameter(key, null, paramConfig(key)); + addParameter(key, null, config.params[key]); }); } else { this.sourcePath = pattern; @@ -180,7 +166,7 @@ UrlMatcher.prototype.concat = function (pattern, config) { // Because order of search parameters is irrelevant, we can add our own search // parameters to the end of the new pattern. Parse the new pattern by itself // and then join the bits together, but it's much easier to do this on a string level. - return new $$UrlMatcherFactoryProvider.compile(this.sourcePath + pattern + this.sourceSearch, config); + return $$UrlMatcherFactoryProvider.compile(this.sourcePath + pattern + this.sourceSearch, config); }; UrlMatcher.prototype.toString = function () { @@ -216,21 +202,19 @@ UrlMatcher.prototype.exec = function (path, searchParams) { if (!m) return null; searchParams = searchParams || {}; - var params = this.parameters(), nTotal = params.length, + var paramNames = this.parameters(), nTotal = paramNames.length, nPath = this.segments.length - 1, - values = {}, i, cfg, param; + values = {}, i, cfg, paramName; if (nPath !== m.length - 1) throw new Error("Unbalanced capture group in route '" + this.source + "'"); for (i = 0; i < nPath; i++) { - param = params[i]; - cfg = this.params[param]; - values[param] = cfg.$value(m[i + 1]); + paramName = paramNames[i]; + values[paramName] = this.params[paramName].value(m[i + 1]); } for (/**/; i < nTotal; i++) { - param = params[i]; - cfg = this.params[param]; - values[param] = cfg.$value(searchParams[param]); + paramName = paramNames[i]; + values[paramName] = this.params[paramName].value(searchParams[paramName]); } return values; @@ -265,15 +249,7 @@ UrlMatcher.prototype.parameters = function (param) { * @returns {boolean} Returns `true` if `params` validates, otherwise `false`. */ UrlMatcher.prototype.validates = function (params) { - var result = true, isOptional, cfg, self = this; - - forEach(params, function(val, key) { - if (!self.params[key]) return; - cfg = self.params[key]; - isOptional = !val && isDefined(cfg.value); - result = result && (isOptional || cfg.type.is(val)); - }); - return result; + return this.params.$$validates(params); }; /** @@ -717,7 +693,94 @@ function $UrlMatcherFactory() { UrlMatcher.prototype.$types[type.name] = def; }); } + + this.Param = function Param(id, type, config) { + var self = this; + var defaultValueConfig = getDefaultValueConfig(config); + config = config || {}; + type = getType(config, type); + + function getDefaultValueConfig(config) { + var keys = isObject(config) ? objectKeys(config) : []; + var isShorthand = keys.indexOf("value") === -1 && keys.indexOf("type") === -1; + var configValue = isShorthand ? config : config.value; + return { + fn: isInjectable(configValue) ? configValue : function () { return configValue; }, + value: configValue + }; + } + + function getType(config, urlType) { + if (config.type && urlType) throw new Error("Param '"+id+"' has two type configurations."); + if (urlType && !config.type) return urlType; + return config.type instanceof Type ? config.type : new Type(config.type || {}); + } + + /** + * [Internal] Get the default value of a parameter, which may be an injectable function. + */ + function $$getDefaultValue() { + if (!injector) throw new Error("Injectable functions cannot be called at configuration time"); + return injector.invoke(defaultValueConfig.fn); + } + + /** + * [Internal] Gets the decoded representation of a value if the value is defined, otherwise, returns the + * default value, which may be the result of an injectable function. + */ + function $value(value) { + return isDefined(value) ? self.type.decode(value) : $$getDefaultValue(); + } + + extend(this, { + id: id, + type: type, + config: config, + dynamic: undefined, + isOptional: defaultValueConfig.value !== undefined, + value: $value + }); + }; + + function ParamSet(params) { + extend(this, params || {}); + } + + ParamSet.prototype = { + $$keys: function () { + return protoKeys(this, ["$$keys", "$$values", "$$equals", "$$validates"]); + }, + $$values: function(paramValues) { + var values = {}, self = this; + forEach(self.$$keys(), function(key) { + values[key] = self[key].value(paramValues && paramValues[key]); + }); + return values; + }, + $$equals: function(paramValues1, paramValues2) { + var equal = true; self = this; + forEach(self.$$keys(), function(key) { + var left = paramValues1 && paramValues1[key], right = paramValues2 && paramValues2[key]; + if (!self[key].type.equals(left, right)) equal = false; + }); + return equal; + }, + $$validates: function $$validate(paramValues) { + var result = true, isOptional, val, param, self = this; + + forEach(this.$$keys(), function(key) { + param = self[key]; + val = paramValues[key]; + isOptional = !val && param.isOptional; + result = result && (isOptional || param.type.is(val)); + }); + return result; + } + }; + + this.ParamSet = ParamSet; } // Register as a provider so it's available to other providers angular.module('ui.router.util').provider('$urlMatcherFactory', $UrlMatcherFactory); +angular.module('ui.router.util').run(['$urlMatcherFactory', function($urlMatcherFactory) { }]); From 3f60fbe6d65ebeca8d97952c05aa1d269f1b7ba1 Mon Sep 17 00:00:00 2001 From: christopherthielen Date: Wed, 8 Oct 2014 07:14:44 -0500 Subject: [PATCH 2/7] fix(): populate default params in .transitionTo. closes #1396 --- src/state.js | 2 ++ test/stateSpec.js | 33 ++++++++++++++++++++++++++++----- 2 files changed, 30 insertions(+), 5 deletions(-) diff --git a/src/state.js b/src/state.js index 3257d3244..146cce228 100644 --- a/src/state.js +++ b/src/state.js @@ -778,6 +778,8 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { } if (toState[abstractKey]) throw new Error("Cannot transition to abstract state '" + to + "'"); if (options.inherit) toParams = inheritParams($stateParams, toParams || {}, $state.$current, toState); + var defaultParams = toState.params.$$values(); + toParams = extend(defaultParams, toParams); to = toState; var toPath = to.path; diff --git a/test/stateSpec.js b/test/stateSpec.js index 7dbbfff5a..ed7c3a3a0 100644 --- a/test/stateSpec.js +++ b/test/stateSpec.js @@ -20,13 +20,14 @@ describe('state', function () { var A = { data: {} }, B = {}, C = {}, - D = { params: { x: {}, y: {} } }, - DD = { parent: D, params: { x: {}, y: {}, z: {} } }, + D = { params: { x: null, y: null } }, + DD = { parent: D, params: { x: null, y: null, z: null } }, E = { params: { i: {} } }, H = { data: {propA: 'propA', propB: 'propB'} }, HH = { parent: H }, HHH = {parent: HH, data: {propA: 'overriddenA', propC: 'propC'} }, RS = { url: '^/search?term', reloadOnSearch: false }, + OPT = { url: '/opt/:param', params: { param: 100 } }, AppInjectable = {}; beforeEach(module(function ($stateProvider, $provide) { @@ -46,6 +47,7 @@ describe('state', function () { .state('H', H) .state('HH', HH) .state('HHH', HHH) + .state('OPT', OPT) .state('RS', RS) .state('home', { url: "/" }) @@ -101,8 +103,8 @@ describe('state', function () { // State param inheritance tests. param1 is inherited by sub1 & sub2; // param2 should not be transferred (unless explicitly set). .state('root', { url: '^/root?param1' }) - .state('root.sub1', {url: '/1?param2' }) - .state('root.sub2', {url: '/2?param2' }); + .state('root.sub1', {url: '/1?param2' }); + $stateProvider.state('root.sub2', {url: '/2?param2' }); $provide.value('AppInjectable', AppInjectable); })); @@ -642,7 +644,7 @@ describe('state', function () { it('contains the parameter values for the current state', inject(function ($state, $q) { initStateTo(D, { x: 'x value', z: 'invalid value' }); - expect($state.params).toEqual({ x: 'x value', y: undefined }); + expect($state.params).toEqual({ x: 'x value', y: null }); })); }); @@ -745,6 +747,7 @@ describe('state', function () { 'H', 'HH', 'HHH', + 'OPT', 'RS', 'about', 'about.person', @@ -786,6 +789,26 @@ describe('state', function () { })); }); + describe('optional parameters', function() { + it("should be populated during transition, if unspecified", inject(function($state, $q) { + var stateParams; + $state.get("OPT").onEnter = function($stateParams) { stateParams = $stateParams; }; + $state.go("OPT"); $q.flush(); + expect($state.current.name).toBe("OPT"); + expect($state.params).toEqual({ param: 100 }); + expect(stateParams).toEqual({ param: 100 }); + })); + + it("should be populated during primary transition, if unspecified", inject(function($state, $q) { + var count = 0; + $state.get("OPT").onEnter = function($stateParams) { count++; }; + $state.go("OPT"); $q.flush(); + expect($state.current.name).toBe("OPT"); + expect($state.params).toEqual({ param: 100 }); + expect(count).toEqual(1); + })); + }); + describe('url handling', function () { it('should transition to the same state with different parameters', inject(function ($state, $rootScope, $location) { $location.path("/about/bob"); From b1379e6a4d38f7ed7436e05873932d7c279af578 Mon Sep 17 00:00:00 2001 From: christopherthielen Date: Wed, 8 Oct 2014 07:00:02 -0500 Subject: [PATCH 3/7] feat($state): add state params validation Closes #1433 feat($urlMatcherFactory): add function to get a RegExp Type --- src/state.js | 2 ++ src/urlMatcherFactory.js | 10 +++++++++- test/stateSpec.js | 27 +++++++++++++++++++++++++++ 3 files changed, 38 insertions(+), 1 deletion(-) diff --git a/src/state.js b/src/state.js index 146cce228..24756a10b 100644 --- a/src/state.js +++ b/src/state.js @@ -778,6 +778,8 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { } if (toState[abstractKey]) throw new Error("Cannot transition to abstract state '" + to + "'"); if (options.inherit) toParams = inheritParams($stateParams, toParams || {}, $state.$current, toState); + if (!toState.params.$$validates(toParams)) return TransitionFailed; + var defaultParams = toState.params.$$values(); toParams = extend(defaultParams, toParams); to = toState; diff --git a/src/urlMatcherFactory.js b/src/urlMatcherFactory.js index 524c2db81..ebda4ca82 100644 --- a/src/urlMatcherFactory.js +++ b/src/urlMatcherFactory.js @@ -94,6 +94,14 @@ function UrlMatcher(pattern, config) { return result + flag + '(' + pattern + ')' + flag; } + function regexpType(regexp) { + var type = new Type({ + pattern: new RegExp(regexp), + is: function(value) { return type.pattern.exec(type.encode(value)); } + }); + return type; + } + this.source = pattern; // Split into static segments separated by path parameter placeholders. @@ -104,7 +112,7 @@ function UrlMatcher(pattern, config) { id = m[2] || m[3]; // IE[78] returns '' for unmatched groups instead of null regexp = m[4] || (m[1] == '*' ? '.*' : '[^/]*'); segment = pattern.substring(last, m.index); - type = this.$types[regexp] || new Type({ pattern: new RegExp(regexp) }); + type = this.$types[regexp] || regexpType(regexp); cfg = config.params[id]; if (segment.indexOf('?') >= 0) break; // we're into the search part diff --git a/test/stateSpec.js b/test/stateSpec.js index ed7c3a3a0..cd217dbfe 100644 --- a/test/stateSpec.js +++ b/test/stateSpec.js @@ -96,6 +96,9 @@ describe('state', function () { .state('badParam', { url: "/bad/{param:int}" }) + .state('badParam2', { + url: "/bad2/{param:[0-9]{5}}" + }) .state('first', { url: '^/first/subpath' }) .state('second', { url: '^/second' }) @@ -755,6 +758,7 @@ describe('state', function () { 'about.sidebar', 'about.sidebar.item', 'badParam', + 'badParam2', 'dynamicController', 'first', 'home', @@ -875,6 +879,29 @@ describe('state', function () { $rootScope.$apply(); expect($state.current.name).toBe("about"); })); + + it('should ignore bad state parameters', inject(function ($state, $rootScope, $location, $stateParams) { + $state.go("badParam", { param: 5 }); + $rootScope.$apply(); + expect($state.current.name).toBe("badParam"); + expect($stateParams).toEqual({param: 5}); + + $state.go("badParam2", { param: '12345' }); // must be 5 digits + $rootScope.$apply(); + expect($state.current.name).toBe("badParam2"); + + $state.go("about"); + $rootScope.$apply(); + expect($state.current.name).toBe('about'); + + $state.go("badParam", { param: 'foo' }); + $rootScope.$apply(); + expect($state.current.name).toBe("about"); + + $state.go("badParam2", { param: '1234' }); // must be 5 digits + $rootScope.$apply(); + expect($state.current.name).toBe("about"); + })); }); it('should revert to last known working url on state change failure', inject(function ($state, $rootScope, $location, $q) { From 838b747664a6687360ea588b1d77faa32e4a7650 Mon Sep 17 00:00:00 2001 From: christopherthielen Date: Wed, 8 Oct 2014 18:15:40 -0500 Subject: [PATCH 4/7] fix($state): queue all state registration until runtime. closes #1435 - param Type(s) are not available until runtime because they can be injected. UrlMatcher requires Type. StateBuilder creates UrlMatchers. Therefore, states must be built JIT at runtime. --- src/state.js | 25 ++++++++++++++++--------- test/stateSpec.js | 4 ++-- 2 files changed, 18 insertions(+), 11 deletions(-) diff --git a/src/state.js b/src/state.js index 24756a10b..f8d85346f 100644 --- a/src/state.js +++ b/src/state.js @@ -22,7 +22,7 @@ $StateProvider.$inject = ['$urlRouterProvider', '$urlMatcherFactoryProvider']; function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { - var root, states = {}, $state, queue = {}, abstractKey = 'abstract'; + var root, states = {}, $state, queue = {}, abstractKey = 'abstract', isRuntime = false; // Builds state properties from definition passed to registerState() var stateBuilder = { @@ -156,6 +156,13 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { queue[parentName].push(state); } + function flushQueuedChildren(parentName) { + var queued = queue[parentName] || []; + while(queued.length) { + registerState(queued.shift()); + } + } + function registerState(state) { // Wrap a new object around the state so we can store our private details easily. state = inherit(state, { @@ -171,10 +178,11 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { // Get parent name var parentName = (name.indexOf('.') !== -1) ? name.substring(0, name.lastIndexOf('.')) : (isString(state.parent)) ? state.parent + : (isObject(state.parent) && isString(state.parent.name)) ? state.parent.name : ''; // If parent is not registered yet, add state to queue and register later - if (parentName && !states[parentName]) { + if (name !== "" && (!isRuntime || !states[parentName])) { return queueState(parentName, state.self); } @@ -193,11 +201,7 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { } // Register any queued children - if (queue[name]) { - for (var i = 0; i < queue[name].length; i++) { - registerState(queue[name][i]); - } - } + flushQueuedChildren(name); return state; } @@ -523,8 +527,8 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { * you're coming from. */ this.$get = $get; - $get.$inject = ['$rootScope', '$q', '$view', '$injector', '$resolve', '$stateParams', '$urlRouter']; - function $get( $rootScope, $q, $view, $injector, $resolve, $stateParams, $urlRouter) { + $get.$inject = ['$rootScope', '$q', '$view', '$injector', '$resolve', '$stateParams', '$urlRouter', '$location', '$urlMatcherFactory']; + function $get( $rootScope, $q, $view, $injector, $resolve, $stateParams, $urlRouter, $location, $urlMatcherFactory) { var TransitionSuperseded = $q.reject(new Error('transition superseded')); var TransitionPrevented = $q.reject(new Error('transition prevented')); @@ -1192,6 +1196,9 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { }); } + isRuntime = true; + flushQueuedChildren(""); + return $state; } diff --git a/test/stateSpec.js b/test/stateSpec.js index cd217dbfe..cfd9302fd 100644 --- a/test/stateSpec.js +++ b/test/stateSpec.js @@ -1049,9 +1049,9 @@ describe('state', function () { describe('provider decorators', function () { - it('should return built-in decorators', function () { + it('should return built-in decorators', inject(function ($state) { expect(stateProvider.decorator('parent')({ parent: A }).self.name).toBe("A"); - }); + })); it('should allow built-in decorators to be overridden', inject(function ($state, $q) { stateProvider.decorator('data', function(state) { From 13a468a7d54c2fb0751b94c0c1841d580b71e6dc Mon Sep 17 00:00:00 2001 From: christopherthielen Date: Mon, 13 Oct 2014 13:02:47 -0500 Subject: [PATCH 5/7] fix($urlMatcherFactory): default to parameter string coersion. feat($urlMatcherFactory): unify params handling code for path/search feat($urlMatcherFactory): add a defaultType that does string coersion and supports arrays (for params) feat($urlMatcherFactory): separate default Type(s) for path/query params Closes #1414 --- src/state.js | 5 +-- src/urlMatcherFactory.js | 90 ++++++++++++++++++++++++++----------- test/stateDirectivesSpec.js | 6 +-- test/stateSpec.js | 59 +++++++++++++++++++----- 4 files changed, 118 insertions(+), 42 deletions(-) diff --git a/src/state.js b/src/state.js index f8d85346f..1c5679ed6 100644 --- a/src/state.js +++ b/src/state.js @@ -784,8 +784,7 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { if (options.inherit) toParams = inheritParams($stateParams, toParams || {}, $state.$current, toState); if (!toState.params.$$validates(toParams)) return TransitionFailed; - var defaultParams = toState.params.$$values(); - toParams = extend(defaultParams, toParams); + toParams = toState.params.$$values(toParams); to = toState; var toPath = to.path; @@ -794,7 +793,7 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { var keep = 0, state = toPath[keep], locals = root.locals, toLocals = []; if (!options.reload) { - while (state && state === fromPath[keep] && equalForKeys(toParams, fromParams, state.ownParams.$$keys())) { + while (state && state === fromPath[keep] && state.ownParams.$$equals(toParams, fromParams)) { locals = toLocals[keep] = state.locals; keep++; state = toPath[keep]; diff --git a/src/urlMatcherFactory.js b/src/urlMatcherFactory.js index ebda4ca82..517a25d7a 100644 --- a/src/urlMatcherFactory.js +++ b/src/urlMatcherFactory.js @@ -70,12 +70,14 @@ function UrlMatcher(pattern, config) { // The regular expression is somewhat complicated due to the need to allow curly braces // inside the regular expression. The placeholder regexp breaks down as follows: // ([:*])(\w+) classic placeholder ($1 / $2) - // \{(\w+)(?:\:( ... ))?\} curly brace placeholder ($3) with optional regexp ... ($4) + // ([:]?)([\w-]+) classic search placeholder (supports snake-case-params) ($1 / $2) + // \{(\w+)(?:\:( ... ))?\} curly brace placeholder ($3) with optional regexp/type ... ($4) // (?: ... | ... | ... )+ the regexp consists of any number of atoms, an atom being either // [^{}\\]+ - anything other than curly braces or backslash // \\. - a backslash escape // \{(?:[^{}\\]+|\\.)*\} - a matched set of curly braces containing other atoms var placeholder = /([:*])(\w+)|\{(\w+)(?:\:((?:[^{}\\]+|\\.|\{(?:[^{}\\]+|\\.)*\})+))?\}/g, + searchPlaceholder = /([:]?)([\w-]+)|\{(\w+)(?:\:((?:[^{}\\]+|\\.|\{(?:[^{}\\]+|\\.)*\})+))?\}/g, compiled = '^', last = 0, m, segments = this.segments = [], params = this.params = new $$UrlMatcherFactoryProvider.ParamSet(); @@ -94,32 +96,33 @@ function UrlMatcher(pattern, config) { return result + flag + '(' + pattern + ')' + flag; } - function regexpType(regexp) { - var type = new Type({ - pattern: new RegExp(regexp), - is: function(value) { return type.pattern.exec(type.encode(value)); } - }); - return type; - } - this.source = pattern; // Split into static segments separated by path parameter placeholders. // The number of segments is always 1 more than the number of parameters. - var id, regexp, segment, type, cfg; - - while ((m = placeholder.exec(pattern))) { + function matchDetails(m, isSearch) { + var id, regexp, segment, type, typeId, cfg; + var $types = UrlMatcher.prototype.$types; + var defaultTypeId = (isSearch ? "searchParam" : "pathParam"); id = m[2] || m[3]; // IE[78] returns '' for unmatched groups instead of null - regexp = m[4] || (m[1] == '*' ? '.*' : '[^/]*'); segment = pattern.substring(last, m.index); - type = this.$types[regexp] || regexpType(regexp); + regexp = isSearch ? m[4] : m[4] || (m[1] == '*' ? '.*' : null); + typeId = regexp || defaultTypeId; + type = $types[typeId] || extend({}, $types[defaultTypeId], { pattern: new RegExp(regexp) }); cfg = config.params[id]; + return { + id: id, regexp: regexp, segment: segment, type: type, cfg: cfg + }; + } - if (segment.indexOf('?') >= 0) break; // we're into the search part + var p, param, segment; + while ((m = placeholder.exec(pattern))) { + p = matchDetails(m, false); + if (p.segment.indexOf('?') >= 0) break; // we're into the search part - var param = addParameter(id, type, cfg); - compiled += quoteRegExp(segment, type.$subPattern(), param.isOptional); - segments.push(segment); + param = addParameter(p.id, p.type, p.cfg); + compiled += quoteRegExp(p.segment, param.type.pattern.source, param.isOptional); + segments.push(p.segment); last = placeholder.lastIndex; } segment = pattern.substring(last); @@ -132,10 +135,15 @@ function UrlMatcher(pattern, config) { segment = segment.substring(0, i); this.sourcePath = pattern.substring(0, last + i); - // Allow parameters to be separated by '?' as well as '&' to make concat() easier - forEach(search.substring(1).split(/[&?]/), function(key) { - addParameter(key, null, config.params[key]); - }); + if (search.length > 0) { + last = 0; + while ((m = searchPlaceholder.exec(search))) { + p = matchDetails(m, true); + param = addParameter(p.id, p.type, p.cfg); + last = placeholder.lastIndex; + // check if ?& + } + } } else { this.sourcePath = pattern; this.sourceSearch = ''; @@ -385,7 +393,7 @@ Type.prototype.encode = function(val, key) { * @methodOf ui.router.util.type:Type * * @description - * Converts a string URL parameter value to a custom/native value. + * Converts a parameter value (from URL string or transition param) to a custom/native value. * * @param {string} val The URL parameter value to decode. * @param {string} key The name of the parameter in which `val` is stored. Can be used for @@ -433,7 +441,36 @@ function $UrlMatcherFactory() { var isCaseInsensitive = false, isStrictMode = true; + function safeString(val) { return isDefined(val) ? val.toString() : val; } + function coerceEquals(left, right) { return left == right; } + function angularEquals(left, right) { return angular.equals(left, right); } +// TODO: function regexpMatches(val) { return isDefined(val) && this.pattern.test(val); } + function regexpMatches(val) { /*jshint validthis:true */ return this.pattern.test(val); } + function normalizeStringOrArray(val) { + if (isArray(val)) { + var encoded = []; + forEach(val, function(item) { encoded.push(safeString(item)); }); + return encoded; + } else { + return safeString(val); + } + } + var enqueue = true, typeQueue = [], injector, defaultTypes = { + "searchParam": { + encode: normalizeStringOrArray, + decode: normalizeStringOrArray, + equals: angularEquals, + is: regexpMatches, + pattern: /[^&?]*/ + }, + "pathParam": { + encode: safeString, + decode: safeString, + equals: coerceEquals, + is: regexpMatches, + pattern: /[^/]*/ + }, int: { decode: function(val) { return parseInt(val, 10); @@ -449,7 +486,7 @@ function $UrlMatcherFactory() { return val ? 1 : 0; }, decode: function(val) { - return parseInt(val, 10) === 0 ? false : true; + return parseInt(val, 10) !== 0; }, is: function(val) { return val === true || val === false; @@ -720,8 +757,9 @@ function $UrlMatcherFactory() { function getType(config, urlType) { if (config.type && urlType) throw new Error("Param '"+id+"' has two type configurations."); - if (urlType && !config.type) return urlType; - return config.type instanceof Type ? config.type : new Type(config.type || {}); + if (urlType) return urlType; + if (!config.type) return UrlMatcher.prototype.$types.pathParam; + return config.type instanceof Type ? config.type : new Type(config.type); } /** diff --git a/test/stateDirectivesSpec.js b/test/stateDirectivesSpec.js index da000b603..f44d1f478 100644 --- a/test/stateDirectivesSpec.js +++ b/test/stateDirectivesSpec.js @@ -130,7 +130,7 @@ describe('uiStateRef', function() { $q.flush(); expect($state.current.name).toEqual('contacts.item.detail'); - expect($stateParams).toEqual({ id: 5 }); + expect($stateParams).toEqual({ id: "5" }); })); it('should transition when given a click that contains no data (fake-click)', inject(function($state, $stateParams, $q) { @@ -147,7 +147,7 @@ describe('uiStateRef', function() { $q.flush(); expect($state.current.name).toEqual('contacts.item.detail'); - expect($stateParams).toEqual({ id: 5 }); + expect($stateParams).toEqual({ id: "5" }); })); it('should not transition states when ctrl-clicked', inject(function($state, $stateParams, $q) { @@ -328,7 +328,7 @@ describe('uiStateRef', function() { $q.flush(); expect($state.$current.name).toBe("contacts.item.detail"); - expect($state.params).toEqual({ id: 5 }); + expect($state.params).toEqual({ id: "5" }); })); it('should resolve states from parent uiView', inject(function ($state, $stateParams, $q, $timeout) { diff --git a/test/stateSpec.js b/test/stateSpec.js index cfd9302fd..3b42f41f2 100644 --- a/test/stateSpec.js +++ b/test/stateSpec.js @@ -27,7 +27,8 @@ describe('state', function () { HH = { parent: H }, HHH = {parent: HH, data: {propA: 'overriddenA', propC: 'propC'} }, RS = { url: '^/search?term', reloadOnSearch: false }, - OPT = { url: '/opt/:param', params: { param: 100 } }, + OPT = { url: '/opt/:param', params: { param: "100" } }, + OPT2 = { url: '/opt2/:param2/:param3', params: { param3: "300", param4: "400" } }, AppInjectable = {}; beforeEach(module(function ($stateProvider, $provide) { @@ -48,6 +49,7 @@ describe('state', function () { .state('HH', HH) .state('HHH', HHH) .state('OPT', OPT) + .state('OPT.OPT2', OPT2) .state('RS', RS) .state('home', { url: "/" }) @@ -273,7 +275,7 @@ describe('state', function () { $q.flush(); expect(called).toBeTruthy(); expect($state.current.name).toEqual('DDD'); - expect($state.params).toEqual({ x: 1, y: 2, z: 3, w: 4 }); + expect($state.params).toEqual({ x: "1", y: "2", z: "3", w: "4" }); })); it('can defer a state transition in $stateNotFound', inject(function ($state, $q, $rootScope) { @@ -290,7 +292,7 @@ describe('state', function () { $q.flush(); expect(called).toBeTruthy(); expect($state.current.name).toEqual('AA'); - expect($state.params).toEqual({ a: 1 }); + expect($state.params).toEqual({ a: "1" }); })); it('can defer and supersede a state transition in $stateNotFound', inject(function ($state, $q, $rootScope) { @@ -479,11 +481,11 @@ describe('state', function () { $state.transitionTo('about.person', { person: 'bob' }); $q.flush(); - $state.go('.item', { id: 5 }); + $state.go('.item', { id: "5" }); $q.flush(); expect($state.$current.name).toBe('about.person.item'); - expect($stateParams).toEqual({ person: 'bob', id: 5 }); + expect($stateParams).toEqual({ person: 'bob', id: "5" }); $state.go('^.^.sidebar'); $q.flush(); @@ -751,6 +753,7 @@ describe('state', function () { 'HH', 'HHH', 'OPT', + 'OPT.OPT2', 'RS', 'about', 'about.person', @@ -799,8 +802,8 @@ describe('state', function () { $state.get("OPT").onEnter = function($stateParams) { stateParams = $stateParams; }; $state.go("OPT"); $q.flush(); expect($state.current.name).toBe("OPT"); - expect($state.params).toEqual({ param: 100 }); - expect(stateParams).toEqual({ param: 100 }); + expect($state.params).toEqual({ param: "100" }); + expect(stateParams).toEqual({ param: "100" }); })); it("should be populated during primary transition, if unspecified", inject(function($state, $q) { @@ -808,7 +811,43 @@ describe('state', function () { $state.get("OPT").onEnter = function($stateParams) { count++; }; $state.go("OPT"); $q.flush(); expect($state.current.name).toBe("OPT"); - expect($state.params).toEqual({ param: 100 }); + expect($state.params).toEqual({ param: "100" }); + expect(count).toEqual(1); + })); + + it("should allow mixed URL and config params", inject(function($state, $q) { + var count = 0; + $state.get("OPT").onEnter = function($stateParams) { count++; }; + $state.get("OPT.OPT2").onEnter = function($stateParams) { count++; }; + $state.go("OPT"); $q.flush(); + expect($state.current.name).toBe("OPT"); + expect($state.params).toEqual({ param: "100" }); + expect(count).toEqual(1); + + $state.go("OPT.OPT2", { param2: 200 }); $q.flush(); + expect($state.current.name).toBe("OPT.OPT2"); + expect($state.params).toEqual({ param: "100", param2: "200", param3: "300", param4: "400" }); + expect(count).toEqual(2); + })); + }); + + // TODO: Enforce by default in next major release (1.0.0) + xdescribe('non-optional parameters', function() { + it("should cause transition failure, when unspecified.", inject(function($state, $q) { + var count = 0; + $state.get("OPT").onEnter = function() { count++; }; + $state.get("OPT.OPT2").onEnter = function() { count++; }; + $state.go("OPT"); $q.flush(); + expect($state.current.name).toBe("OPT"); + expect($state.params).toEqual({ param: "100" }); + expect(count).toEqual(1); + + var result; + $state.go("OPT.OPT2").then(function(data) { result = data; }); + $q.flush(); + expect($state.current.name).toBe("OPT"); + expect($state.params).toEqual({ param: "100" }); + expect(result).toEqual("asdfasdf"); expect(count).toEqual(1); })); }); @@ -996,7 +1035,7 @@ describe('state', function () { $state.go('root.sub1', { param2: 2 }); $q.flush(); expect($state.current.name).toEqual('root.sub1'); - expect($stateParams).toEqual({ param1: 1, param2: 2 }); + expect($stateParams).toEqual({ param1: "1", param2: "2" }); })); it('should not inherit siblings\' states', inject(function ($state, $stateParams, $q) { @@ -1009,7 +1048,7 @@ describe('state', function () { $q.flush(); expect($state.current.name).toEqual('root.sub2'); - expect($stateParams).toEqual({ param1: 1, param2: undefined }); + expect($stateParams).toEqual({ param1: "1", param2: undefined }); })); }); From e756e083306478cc97e97fdb7dd1c9c8a8c5c75a Mon Sep 17 00:00:00 2001 From: christopherthielen Date: Mon, 20 Oct 2014 20:15:30 -0500 Subject: [PATCH 6/7] - rename $$UrlMatcherFactoryProvider to $$UMFP - move $types from UrlMatcher.prototype to $UrlMatcherFactoryProvider closure --- src/state.js | 6 +++--- src/urlMatcherFactory.js | 33 +++++++++++++++++---------------- 2 files changed, 20 insertions(+), 19 deletions(-) diff --git a/src/state.js b/src/state.js index 1c5679ed6..35f3ed7ee 100644 --- a/src/state.js +++ b/src/state.js @@ -66,16 +66,16 @@ function $StateProvider( $urlRouterProvider, $urlMatcherFactory) { // Own parameters for this state. state.url.params is already built at this point. Create and add non-url params ownParams: function(state) { - var params = state.url && state.url.params || new $$UrlMatcherFactoryProvider.ParamSet(); + var params = state.url && state.url.params || new $$UMFP.ParamSet(); forEach(state.params || {}, function(config, id) { - if (!params[id]) params[id] = new $$UrlMatcherFactoryProvider.Param(id, null, config); + if (!params[id]) params[id] = new $$UMFP.Param(id, null, config); }); return params; }, // Derive parameters for this state and ensure they're a super-set of parent's parameters params: function(state) { - var parentParams = state.parent && state.parent.params || new $$UrlMatcherFactoryProvider.ParamSet(); + var parentParams = state.parent && state.parent.params || new $$UMFP.ParamSet(); return inherit(parentParams, state.ownParams); }, diff --git a/src/urlMatcherFactory.js b/src/urlMatcherFactory.js index 517a25d7a..53d778f47 100644 --- a/src/urlMatcherFactory.js +++ b/src/urlMatcherFactory.js @@ -1,3 +1,5 @@ +var $$UMFP; // reference to $UrlMatcherFactoryProvider + /** * @ngdoc object * @name ui.router.util.type:UrlMatcher @@ -80,12 +82,12 @@ function UrlMatcher(pattern, config) { searchPlaceholder = /([:]?)([\w-]+)|\{(\w+)(?:\:((?:[^{}\\]+|\\.|\{(?:[^{}\\]+|\\.)*\})+))?\}/g, compiled = '^', last = 0, m, segments = this.segments = [], - params = this.params = new $$UrlMatcherFactoryProvider.ParamSet(); + params = this.params = new $$UMFP.ParamSet(); function addParameter(id, type, config) { if (!/^\w+(-+\w+)*$/.test(id)) throw new Error("Invalid parameter name '" + id + "' in pattern '" + pattern + "'"); if (params[id]) throw new Error("Duplicate parameter name '" + id + "' in pattern '" + pattern + "'"); - params[id] = new $$UrlMatcherFactoryProvider.Param(id, type, config); + params[id] = new $$UMFP.Param(id, type, config); return params[id]; } @@ -102,13 +104,12 @@ function UrlMatcher(pattern, config) { // The number of segments is always 1 more than the number of parameters. function matchDetails(m, isSearch) { var id, regexp, segment, type, typeId, cfg; - var $types = UrlMatcher.prototype.$types; var defaultTypeId = (isSearch ? "searchParam" : "pathParam"); id = m[2] || m[3]; // IE[78] returns '' for unmatched groups instead of null segment = pattern.substring(last, m.index); regexp = isSearch ? m[4] : m[4] || (m[1] == '*' ? '.*' : null); typeId = regexp || defaultTypeId; - type = $types[typeId] || extend({}, $types[defaultTypeId], { pattern: new RegExp(regexp) }); + type = $$UMFP.type(typeId) || extend({}, $$UMFP.type(defaultTypeId), { pattern: new RegExp(regexp) }); cfg = config.params[id]; return { id: id, regexp: regexp, segment: segment, type: type, cfg: cfg @@ -182,7 +183,7 @@ UrlMatcher.prototype.concat = function (pattern, config) { // Because order of search parameters is irrelevant, we can add our own search // parameters to the end of the new pattern. Parse the new pattern by itself // and then join the bits together, but it's much easier to do this on a string level. - return $$UrlMatcherFactoryProvider.compile(this.sourcePath + pattern + this.sourceSearch, config); + return $$UMFP.compile(this.sourcePath + pattern + this.sourceSearch, config); }; UrlMatcher.prototype.toString = function () { @@ -322,8 +323,6 @@ UrlMatcher.prototype.format = function (values) { return result.replace('//', '/'); }; -UrlMatcher.prototype.$types = {}; - /** * @ngdoc object * @name ui.router.util.type:Type @@ -427,7 +426,6 @@ Type.prototype.$subPattern = function() { Type.prototype.pattern = /.*/; -var $$UrlMatcherFactoryProvider; /** * @ngdoc object * @name ui.router.util.$urlMatcherFactory @@ -437,7 +435,7 @@ var $$UrlMatcherFactoryProvider; * is also available to providers under the name `$urlMatcherFactoryProvider`. */ function $UrlMatcherFactory() { - $$UrlMatcherFactoryProvider = this; + $$UMFP = this; var isCaseInsensitive = false, isStrictMode = true; @@ -456,7 +454,7 @@ function $UrlMatcherFactory() { } } - var enqueue = true, typeQueue = [], injector, defaultTypes = { + var $types, enqueue = true, typeQueue = [], injector, defaultTypes = { "searchParam": { encode: normalizeStringOrArray, decode: normalizeStringOrArray, @@ -707,7 +705,10 @@ function $UrlMatcherFactory() { * */ this.type = function (name, def) { - if (!isDefined(def)) return UrlMatcher.prototype.$types[name]; + if (!isDefined(def)) { + if (!isDefined($types)) throw new Error("Please wait until runtime to retrieve types."); + return $types[name]; + } typeQueue.push({ name: name, def: def }); if (!enqueue) flushTypeQueue(); return this; @@ -717,11 +718,11 @@ function $UrlMatcherFactory() { this.$get = ['$injector', function ($injector) { injector = $injector; enqueue = false; - UrlMatcher.prototype.$types = {}; + $types = {}; flushTypeQueue(); forEach(defaultTypes, function(type, name) { - if (!UrlMatcher.prototype.$types[name]) UrlMatcher.prototype.$types[name] = new Type(type); + if (!$types[name]) $types[name] = new Type(type); }); return this; }]; @@ -731,11 +732,11 @@ function $UrlMatcherFactory() { // before actually wiring up and assigning type definitions function flushTypeQueue() { forEach(typeQueue, function(type) { - if (UrlMatcher.prototype.$types[type.name]) { + if ($types[type.name]) { throw new Error("A type named '" + type.name + "' has already been defined."); } var def = new Type(isInjectable(type.def) ? injector.invoke(type.def) : type.def); - UrlMatcher.prototype.$types[type.name] = def; + $types[type.name] = def; }); } @@ -758,7 +759,7 @@ function $UrlMatcherFactory() { function getType(config, urlType) { if (config.type && urlType) throw new Error("Param '"+id+"' has two type configurations."); if (urlType) return urlType; - if (!config.type) return UrlMatcher.prototype.$types.pathParam; + if (!config.type) return $types.pathParam; return config.type instanceof Type ? config.type : new Type(config.type); } From 97f8d9034bddffc4da3240b2cf004a6f8b2f5631 Mon Sep 17 00:00:00 2001 From: christopherthielen Date: Mon, 20 Oct 2014 21:02:16 -0500 Subject: [PATCH 7/7] made .when defer till runtime --- src/urlRouter.js | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/src/urlRouter.js b/src/urlRouter.js index 4e5fbc4a2..5a69e82d9 100644 --- a/src/urlRouter.js +++ b/src/urlRouter.js @@ -16,7 +16,7 @@ */ $UrlRouterProvider.$inject = ['$locationProvider', '$urlMatcherFactoryProvider']; function $UrlRouterProvider( $locationProvider, $urlMatcherFactory) { - var rules = [], otherwise = null, interceptDeferred = false, listener; + var self = this, rules = [], otherwise = null, interceptDeferred = false, listener; // Returns a string that is a prefix of all strings matching the RegExp function regExpPrefix(re) { @@ -154,6 +154,20 @@ function $UrlRouterProvider( $locationProvider, $urlMatcherFactory) { * @param {string|object} handler The path you want to redirect your user to. */ this.when = function (what, handler) { + if (whenQueue) + whenQueue.push({ what: what, handler: handler }); + else + _when(what, handler); + return self; + }; + + var whenQueue = []; + function flushWhenQueue() { + forEach(whenQueue, function(queuedWhen) { _when(queuedWhen.what, queuedWhen.handler); }); + whenQueue = null; + } + + function _when(what, handler) { var redirect, handlerIsString = isString(handler); if (isString(what)) what = $urlMatcherFactory.compile(what); @@ -190,11 +204,11 @@ function $UrlRouterProvider( $locationProvider, $urlMatcherFactory) { var check = { matcher: $urlMatcherFactory.isMatcher(what), regex: what instanceof RegExp }; for (var n in check) { - if (check[n]) return this.rule(strategies[n](what, handler)); + if (check[n]) return self.rule(strategies[n](what, handler)); } throw new Error("invalid 'what' in when()"); - }; + } /** * @ngdoc function @@ -264,7 +278,7 @@ function $UrlRouterProvider( $locationProvider, $urlMatcherFactory) { this.$get = $get; $get.$inject = ['$location', '$rootScope', '$injector', '$browser']; function $get( $location, $rootScope, $injector, $browser) { - + flushWhenQueue(); var baseHref = $browser.baseHref(), location = $location.url(); function appendBasePath(url, isHtml5, absolute) {