Skip to content

Commit 2158a9d

Browse files
christopherthielenchristopherthielen
committed
fix($urlMatcherFactory): add explicit placeholder Type
 - when using {param:regex} format placeholder, now use separate areas for regexp and Type, i.e., {param:regex:Type}
1 parent 2371ba9 commit 2158a9d

File tree

5 files changed

+72
-28
lines changed

5 files changed

+72
-28
lines changed

‎src/urlMatcherFactory.js

+15-10
Original file line numberDiff line numberDiff line change
@@ -72,19 +72,22 @@ function UrlMatcher(pattern, config) {
7272
// ([:*])(\w+) classic placeholder ($1 / $2)
7373
// ([:]?)([\w-]+) classic search placeholder (supports snake-case-params) ($1 / $2)
7474
// \{(\w+)(?:\:( ... ))?\} curly brace placeholder ($3) with optional regexp/type ... ($4)
75-
// (?: ... | ... | ... )+ the regexp consists of any number of atoms, an atom being either
76-
// [^{}\\]+ - anything other than curly braces or backslash
75+
// (?: ... | ... | ... | ... )+ the regexp consists of any number of atoms, an atom being either
76+
// (?=:) - positive lookahead for colon, to match type portion
77+
// [^:{}\\]+ - anything other than colon, curly braces or backslash
7778
// \\. - a backslash escape
7879
// \{(?:[^{}\\]+|\\.)*\} - a matched set of curly braces containing other atoms
79-
var placeholder = /([:*])(\w+)|\{(\w+)(?:\:((?:[^{}\\]+|\\.|\{(?:[^{}\\]+|\\.)*\})+))?\}/g,
80-
searchPlaceholder = /([:]?)([\w-]+)|\{(\w+)(?:\:((?:[^{}\\]+|\\.|\{(?:[^{}\\]+|\\.)*\})+))?\}/g,
80+
// (?:\:([A-Za-z0-9_-]+))? the placeholder's data Type
81+
var placeholder = /([:*])(\w+)|\{(\w+)(?:\:((?:(?=:)|[^:{}\\]+|\\.|\{(?:[^{}\\]+|\\.)*\})+))?(?:\:([A-Za-z0-9_-]+))?\}/g,
82+
searchPlaceholder = /([:]?)([\w-]+)|\{(\w+)(?:\:((?:(?=:)|[^:{}\\]+|\\.|\{(?:[^{}\\]+|\\.)*\})+))?(?:\:([A-Za-z0-9_-]+))?\}/g,
8183
compiled = '^', last = 0, m,
8284
segments = this.segments = [],
8385
params = this.params = new $$UrlMatcherFactoryProvider.ParamSet();
8486

85-
function addParameter(id, type, config) {
87+
function addParameter(id, type, config, regexp) {
8688
if (!/^\w+(-+\w+)*$/.test(id)) throw new Error("Invalid parameter name '" + id + "' in pattern '" + pattern + "'");
8789
if (params[id]) throw new Error("Duplicate parameter name '" + id + "' in pattern '" + pattern + "'");
90+
if (regexp) type = inherit(type, { pattern: new RegExp(regexp) });
8891
params[id] = new $$UrlMatcherFactoryProvider.Param(id, type, config);
8992
return params[id];
9093
}
@@ -111,9 +114,11 @@ function UrlMatcher(pattern, config) {
111114
function matchDetails(m, isSearch) {
112115
var id, regexp, segment, type, cfg;
113116
id = m[2] || m[3]; // IE[78] returns '' for unmatched groups instead of null
114-
regexp = isSearch ? m[4] : m[4] || (m[1] == '*' ? '.*' : '[^/]*');
117+
regexp = isSearch ? m[4] : m[4] || (m[1] == '*' ? '.*' : undefined);
118+
var typeIdentifier = m[5] || (isSearch ? "searchParam" : "pathParam");
119+
type = UrlMatcher.prototype.$types[typeIdentifier];
120+
if (!type) throw new Error("Unregistered type identifier: " + typeIdentifier);
115121
segment = pattern.substring(last, m.index);
116-
type = regexp ? UrlMatcher.prototype.$types[regexp] || regexpType(regexp) : undefined;
117122
cfg = config.params[id];
118123
return {
119124
id: id, regexp: regexp, segment: segment, type: type, cfg: cfg
@@ -125,8 +130,8 @@ function UrlMatcher(pattern, config) {
125130
p = matchDetails(m, false);
126131
if (p.segment.indexOf('?') >= 0) break; // we're into the search part
127132

128-
param = addParameter(p.id, p.type, p.cfg);
129-
compiled += quoteRegExp(p.segment, p.type.$subPattern(), param.isOptional);
133+
param = addParameter(p.id, p.type, p.cfg, p.regexp);
134+
compiled += quoteRegExp(p.segment, param.type.pattern.source, param.isOptional);
130135
segments.push(p.segment);
131136
last = placeholder.lastIndex;
132137
}
@@ -144,7 +149,7 @@ function UrlMatcher(pattern, config) {
144149
last = 0;
145150
while ((m = searchPlaceholder.exec(search))) {
146151
p = matchDetails(m, true);
147-
param = addParameter(p.id, p.type, p.cfg);
152+
param = addParameter(p.id, p.type, p.cfg, p.regexp);
148153
last = placeholder.lastIndex;
149154
// check if ?&
150155
}

‎test/stateDirectivesSpec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -328,7 +328,7 @@ describe('uiStateRef', function() {
328328
$q.flush();
329329

330330
expect($state.$current.name).toBe("contacts.item.detail");
331-
expect($state.params).toEqual({ id: 5 });
331+
expect($state.params).toEqual({ id: "5" });
332332
}));
333333

334334
it('should resolve states from parent uiView', inject(function ($state, $stateParams, $q, $timeout) {

‎test/stateSpec.js

+46-7
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,8 @@ describe('state', function () {
2727
HH = { parent: H },
2828
HHH = {parent: HH, data: {propA: 'overriddenA', propC: 'propC'} },
2929
RS = { url: '^/search?term', reloadOnSearch: false },
30-
OPT = { url: '/opt/:param', params: { param: 100 } },
30+
OPT = { url: '/opt/:param', params: { param: "100" } },
31+
OPT2 = { url: '/opt2/:param2/:param3', params: { param3: "300", param4: "400" } },
3132
AppInjectable = {};
3233

3334
beforeEach(module(function ($stateProvider, $provide) {
@@ -48,6 +49,7 @@ describe('state', function () {
4849
.state('HH', HH)
4950
.state('HHH', HHH)
5051
.state('OPT', OPT)
52+
.state('OPT.OPT2', OPT2)
5153
.state('RS', RS)
5254

5355
.state('home', { url: "/" })
@@ -94,7 +96,7 @@ describe('state', function () {
9496
}
9597
})
9698
.state('badParam', {
97-
url: "/bad/{param:int}"
99+
url: "/bad/{param::int}"
98100
})
99101
.state('badParam2', {
100102
url: "/bad2/{param:[0-9]{5}}"
@@ -479,11 +481,11 @@ describe('state', function () {
479481
$state.transitionTo('about.person', { person: 'bob' });
480482
$q.flush();
481483

482-
$state.go('.item', { id: 5 });
484+
$state.go('.item', { id: "5" });
483485
$q.flush();
484486

485487
expect($state.$current.name).toBe('about.person.item');
486-
expect($stateParams).toEqual({ person: 'bob', id: 5 });
488+
expect($stateParams).toEqual({ person: 'bob', id: "5" });
487489

488490
$state.go('^.^.sidebar');
489491
$q.flush();
@@ -751,6 +753,7 @@ describe('state', function () {
751753
'HH',
752754
'HHH',
753755
'OPT',
756+
'OPT.OPT2',
754757
'RS',
755758
'about',
756759
'about.person',
@@ -799,16 +802,52 @@ describe('state', function () {
799802
$state.get("OPT").onEnter = function($stateParams) { stateParams = $stateParams; };
800803
$state.go("OPT"); $q.flush();
801804
expect($state.current.name).toBe("OPT");
802-
expect($state.params).toEqual({ param: 100 });
803-
expect(stateParams).toEqual({ param: 100 });
805+
expect($state.params).toEqual({ param: "100" });
806+
expect(stateParams).toEqual({ param: "100" });
804807
}));
805808

806809
it("should be populated during primary transition, if unspecified", inject(function($state, $q) {
807810
var count = 0;
808811
$state.get("OPT").onEnter = function($stateParams) { count++; };
809812
$state.go("OPT"); $q.flush();
810813
expect($state.current.name).toBe("OPT");
811-
expect($state.params).toEqual({ param: 100 });
814+
expect($state.params).toEqual({ param: "100" });
815+
expect(count).toEqual(1);
816+
}));
817+
818+
it("should allow mixed URL and config params", inject(function($state, $q) {
819+
var count = 0;
820+
$state.get("OPT").onEnter = function($stateParams) { count++; };
821+
$state.get("OPT.OPT2").onEnter = function($stateParams) { count++; };
822+
$state.go("OPT"); $q.flush();
823+
expect($state.current.name).toBe("OPT");
824+
expect($state.params).toEqual({ param: "100" });
825+
expect(count).toEqual(1);
826+
827+
$state.go("OPT.OPT2", { param2: 200 }); $q.flush();
828+
expect($state.current.name).toBe("OPT.OPT2");
829+
expect($state.params).toEqual({ param: "100", param2: "200", param3: "300", param4: "400" });
830+
expect(count).toEqual(2);
831+
}));
832+
});
833+
834+
// TODO: Enforce by default in next major release (1.0.0)
835+
xdescribe('non-optional parameters', function() {
836+
it("should cause transition failure, when unspecified.", inject(function($state, $q) {
837+
var count = 0;
838+
$state.get("OPT").onEnter = function() { count++; };
839+
$state.get("OPT.OPT2").onEnter = function() { count++; };
840+
$state.go("OPT"); $q.flush();
841+
expect($state.current.name).toBe("OPT");
842+
expect($state.params).toEqual({ param: "100" });
843+
expect(count).toEqual(1);
844+
845+
var result;
846+
$state.go("OPT.OPT2").then(function(data) { result = data; });
847+
$q.flush();
848+
expect($state.current.name).toBe("OPT");
849+
expect($state.params).toEqual({ param: "100" });
850+
expect(result).toEqual("asdfasdf");
812851
expect(count).toEqual(1);
813852
}));
814853
});

‎test/urlMatcherFactorySpec.js

+9-9
Original file line numberDiff line numberDiff line change
@@ -252,13 +252,13 @@ describe("urlMatcherFactory", function () {
252252
}));
253253

254254
it("should match built-in types", function () {
255-
var m = new UrlMatcher("/{foo:int}/{flag:bool}");
255+
var m = new UrlMatcher("/{foo::int}/{flag::bool}");
256256
expect(m.exec("/1138/1")).toEqual({ foo: 1138, flag: true });
257257
expect(m.format({ foo: 5, flag: true })).toBe("/5/1");
258258
});
259259

260260
it("should encode/decode dates", function () {
261-
var m = new UrlMatcher("/calendar/{date:date}"),
261+
var m = new UrlMatcher("/calendar/{date::date}"),
262262
result = m.exec("/calendar/2014-03-26");
263263

264264
expect(result.date instanceof Date).toBe(true);
@@ -267,7 +267,7 @@ describe("urlMatcherFactory", function () {
267267
});
268268

269269
it("should not match invalid typed parameter values", function() {
270-
var m = new UrlMatcher('/users/{id:int}');
270+
var m = new UrlMatcher('/users/{id::int}');
271271

272272
expect(m.exec('/users/1138').id).toBe(1138);
273273
expect(m.exec('/users/alpha')).toBeNull();
@@ -279,7 +279,7 @@ describe("urlMatcherFactory", function () {
279279

280280
describe("optional parameters", function() {
281281
it("should match with or without values", function () {
282-
var m = new UrlMatcher('/users/{id:int}', {
282+
var m = new UrlMatcher('/users/{id::int}', {
283283
params: { id: { value: null } }
284284
});
285285
expect(m.exec('/users/1138')).toEqual({ id: 1138 });
@@ -288,7 +288,7 @@ describe("urlMatcherFactory", function () {
288288
});
289289

290290
it("should correctly match multiple", function() {
291-
var m = new UrlMatcher('/users/{id:int}/{state:[A-Z]+}', {
291+
var m = new UrlMatcher('/users/{id::int}/{state:[A-Z]+}', {
292292
params: { id: { value: null }, state: { value: null } }
293293
});
294294
expect(m.exec('/users/1138')).toEqual({ id: 1138, state: null });
@@ -305,15 +305,15 @@ describe("urlMatcherFactory", function () {
305305
});
306306

307307
it("should correctly format with or without values", function() {
308-
var m = new UrlMatcher('/users/{id:int}', {
308+
var m = new UrlMatcher('/users/{id::int}', {
309309
params: { id: { value: null } }
310310
});
311311
expect(m.format()).toBe('/users/');
312312
expect(m.format({ id: 1138 })).toBe('/users/1138');
313313
});
314314

315315
it("should correctly format multiple", function() {
316-
var m = new UrlMatcher('/users/{id:int}/{state:[A-Z]+}', {
316+
var m = new UrlMatcher('/users/{id::int}/{state:[A-Z]+}', {
317317
params: { id: { value: null }, state: { value: null } }
318318
});
319319

@@ -324,7 +324,7 @@ describe("urlMatcherFactory", function () {
324324
});
325325

326326
it("should match in between static segments", function() {
327-
var m = new UrlMatcher('/users/{user:int}/photos', {
327+
var m = new UrlMatcher('/users/{user::int}/photos', {
328328
params: { user: 5 }
329329
});
330330
expect(m.exec('/users/photos').user).toBe(5);
@@ -346,7 +346,7 @@ describe("urlMatcherFactory", function () {
346346

347347
describe("default values", function() {
348348
it("should populate if not supplied in URL", function() {
349-
var m = new UrlMatcher('/users/{id:int}/{test}', {
349+
var m = new UrlMatcher('/users/{id::int}/{test}', {
350350
params: { id: { value: 0 }, test: { value: "foo" } }
351351
});
352352
expect(m.exec('/users')).toEqual({ id: 0, test: "foo" });

‎test/urlRouterSpec.js

+1-1
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,7 @@ describe("UrlRouter", function () {
194194
return val === 1138;
195195
}
196196
});
197-
var matcher = new UrlMatcher("/foo/{param:custom}");
197+
var matcher = new UrlMatcher("/foo/{param::custom}");
198198

199199
expect($urlRouter.href(matcher, { param: 1138 })).toBe('#/foo/1138');
200200
expect($urlRouter.href(matcher, { param: 5 })).toBeNull();

0 commit comments

Comments
 (0)