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 }); })); });