Skip to content

Commit

Permalink
fix($urlMatcherFactory): fix mixed path/query params ordering problem
Browse files Browse the repository at this point in the history
If a state defined a query param and a substate defined a path param, the URLs were generated and matched in the wrong order.  A substate creates its UrlMatcher using .concat() and a concat()'d UrlMatcher assumed the parent params order mattered.  Switched to populating an array of params in the order they were parsed in UrlMatcher constructor.

Closes #1543
  • Loading branch information
christopherthielen committed Nov 17, 2014
1 parent 5e6783b commit a479fbd
Show file tree
Hide file tree
Showing 2 changed files with 12 additions and 3 deletions.
9 changes: 6 additions & 3 deletions src/urlMatcherFactory.js
Original file line number Diff line number Diff line change
Expand Up @@ -86,9 +86,11 @@ function UrlMatcher(pattern, config, parentMatcher) {
compiled = '^', last = 0, m,
segments = this.segments = [],
parentParams = parentMatcher ? parentMatcher.params : {},
params = this.params = parentMatcher ? parentMatcher.params.$$new() : new $$UMFP.ParamSet();
params = this.params = parentMatcher ? parentMatcher.params.$$new() : new $$UMFP.ParamSet(),
paramNames = [];

function addParameter(id, type, config, location) {
paramNames.push(id);
if (parentParams[id]) return parentParams[id];
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 + "'");
Expand Down Expand Up @@ -162,6 +164,7 @@ function UrlMatcher(pattern, config, parentMatcher) {

this.regexp = new RegExp(compiled, config.caseInsensitive ? 'i' : undefined);
this.prefix = segments[0];
this.$$paramNames = paramNames;
}

/**
Expand Down Expand Up @@ -277,7 +280,7 @@ UrlMatcher.prototype.exec = function (path, searchParams) {
* pattern has no parameters, an empty array is returned.
*/
UrlMatcher.prototype.parameters = function (param) {
if (!isDefined(param)) return this.params.$$keys();
if (!isDefined(param)) return this.$$paramNames;
return this.params[param] || null;
};

Expand Down Expand Up @@ -499,7 +502,7 @@ Type.prototype.$asArray = function(mode, isSearch) {
};
}

function toArray(val) { return isArray(val) ? val : [ val ] }
function toArray(val) { return isArray(val) ? val : [ val ]; }
function fromArray(val) { return mode === "auto" && val && val.length === 1 ? val[0] : val; }
function falsey(val) { return !val; }

Expand Down
6 changes: 6 additions & 0 deletions test/urlMatcherFactorySpec.js
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,12 @@ describe("UrlMatcher", function () {
expect(m.exec("/foo")).toEqual({});
expect(m.exec("/FOO")).toEqual({});
});

it("should generate/match params in the proper order", function() {
var m = new UrlMatcher('/foo?queryparam');
m = m.concat("/bar/:pathparam");
expect(m.exec("/foo/bar/pathval", { queryparam: "queryval" })).toEqual({ pathparam: "pathval", queryparam: "queryval"});
});
});


Expand Down

0 comments on commit a479fbd

Please sign in to comment.