Skip to content

fix($state): populate default params in .transitionTo. closes #1396 #1432

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 8 commits into from
Closed

fix($state): populate default params in .transitionTo. closes #1396 #1432

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
967e49b
fix($urlMatcherFactory): Made a Params class
christopherthielen Oct 6, 2014
2f7eb4f
fix($state): make ownParams and params use Param type
christopherthielen Oct 7, 2014
75a0ed3
feat($urlMatcherFactory): add Param.isOptional
christopherthielen Oct 8, 2014
2541023
fix($urlMatcherFactory): always initalize $urlMatcherFactory at runtime.
christopherthielen Oct 8, 2014
9499093
feat($urlMatcherFactory): add ParamSet class
christopherthielen Oct 8, 2014
231b221
fix(): fix 86137values logic
christopherthielen Oct 8, 2014
0630562
fix(): use ownParams.86137keys()
christopherthielen Oct 8, 2014
a31c381
fix(): populate default params in .transitionTo. closes #1396
christopherthielen Oct 8, 2014
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions src/common.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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()`.
*
Expand Down
47 changes: 17 additions & 30 deletions src/state.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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
Expand Down Expand Up @@ -793,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;
Expand All @@ -801,7 +788,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];
Expand All @@ -820,7 +807,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) {
Expand Down Expand Up @@ -1133,7 +1120,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
});
};
Expand Down Expand Up @@ -1162,7 +1149,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.
Expand Down
135 changes: 95 additions & 40 deletions src/urlMatcherFactory.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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) {
Expand All @@ -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.
Expand All @@ -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;
}
Expand All @@ -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;
Expand Down Expand Up @@ -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 () {
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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);
};

/**
Expand Down Expand Up @@ -717,7 +693,86 @@ 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,
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not yet satisfied with this logic. this means paramName: undefined is not optional but paramName: null is optional

value: $value
});
};

function ParamSet(params) {
extend(this, params || {});
}

ParamSet.prototype = {
$$keys: function () {
return protoKeys(this, ["$$keys", "$$values", "$$validates"]);
},
$$values: function(paramValues) {
var values = {}, self = this;
forEach(self.$$keys(), function(key) {
values[key] = self[key].value(paramValues && paramValues[key]);
});
return values;
},
$$validates: function $$validate(paramValues) {
var result = true, isOptional, param, self = this;

forEach(paramValues, function (val, key) {
if (!self[key]) return;
param = self[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) { }]);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a pattern I use in ui-router-extras to make sure that provider.$get() is called

Loading