Skip to content

Commit

Permalink
fix($urlMatcherFactory): default to parameter string coersion.
Browse files Browse the repository at this point in the history 
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
  • Loading branch information
@christopherthielen
christopherthielen committed Oct 13, 2014
1 parent 8dd978d commit 2371ba9
Show file tree
Hide file tree
Showing 4 changed files with 69 additions and 27 deletions.
5 changes: 2 additions & 3 deletions src/state.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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];
Expand Down
79 changes: 61 additions & 18 deletions src/urlMatcherFactory.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down Expand Up @@ -106,20 +108,26 @@ function UrlMatcher(pattern, config) {

// 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, cfg;
id = m[2] || m[3]; // IE[78] returns '' for unmatched groups instead of null
regexp = m[4] || (m[1] == '*' ? '.*' : '[^/]*');
regexp = isSearch ? m[4] : m[4] || (m[1] == '*' ? '.*' : '[^/]*');
segment = pattern.substring(last, m.index);
type = this.$types[regexp] || regexpType(regexp);
type = regexp ? UrlMatcher.prototype.$types[regexp] || regexpType(regexp) : undefined;
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, p.type.$subPattern(), param.isOptional);
segments.push(p.segment);
last = placeholder.lastIndex;
}
segment = pattern.substring(last);
Expand All @@ -132,10 +140,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 = '';
Expand Down Expand Up @@ -385,7 +398,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
Expand Down Expand Up @@ -433,7 +446,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": {
encde: safeString,
decode: safeString,
equals: coerceEquals,
is: regexpMatches,
pattern: /[^/]*/
},
int: {
decode: function(val) {
return parseInt(val, 10);
Expand All @@ -449,7 +491,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;
Expand Down Expand Up @@ -720,8 +762,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);
}

/**
Expand Down
4 changes: 2 additions & 2 deletions test/stateDirectivesSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand All @@ -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) {
Expand Down
8 changes: 4 additions & 4 deletions test/stateSpec.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -273,7 +273,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) {
Expand All @@ -290,7 +290,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) {
Expand Down Expand Up @@ -996,7 +996,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) {
Expand All @@ -1009,7 +1009,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 });
}));
});

Expand Down

0 comments on commit 2371ba9

Please sign in to comment.