Skip to content

Commit a4af7a6

Browse files
mjacksonmjackson
committed
Use () for optional params instead of ?
Fixes #960
1 parent ec0bc5e commit a4af7a6

File tree

2 files changed

+87
-61
lines changed

2 files changed

+87
-61
lines changed

‎modules/PathUtils.js

Lines changed: 79 additions & 45 deletions
Original file line numberDiff line numberDiff line change
@@ -1,35 +1,62 @@
1+
/* jshint -W084 */
12
var invariant = require('react/lib/invariant');
23
var assign = require('object-assign');
34
var qs = require('qs');
45

5-
var paramCompileMatcher = /:([a-zA-Z_$][a-zA-Z0-9_$]*)|[*.()\[\]\\+|{}^$]/g;
6-
var paramInjectMatcher = /:([a-zA-Z_$][a-zA-Z0-9_$?]*[?]?)|[*]/g;
7-
var paramInjectTrailingSlashMatcher = /\/\/\?|\/\?\/|\/\?/g;
86
var queryMatcher = /\?(.*)$/;
97

10-
var _compiledPatterns = {};
8+
function escapeRegExp(string) {
9+
return string.replace(/[.*+?^${}()|[\]\\]/g, "\\$&");
10+
}
1111

12-
function compilePattern(pattern) {
13-
if (!(pattern in _compiledPatterns)) {
14-
var paramNames = [];
15-
var source = pattern.replace(paramCompileMatcher, function (match, paramName) {
16-
if (paramName) {
17-
paramNames.push(paramName);
18-
return '([^/?#]+)';
19-
} else if (match === '*') {
20-
paramNames.push('splat');
21-
return '(.*?)';
22-
} else {
23-
return '\\' + match;
24-
}
25-
});
12+
function _compilePattern(pattern) {
13+
var escapedSource = '';
14+
var paramNames = [];
15+
var tokens = [];
16+
17+
var match, lastIndex = 0, matcher = /:([a-zA-Z_$][a-zA-Z0-9_$]*)|\*|\(|\)/g;
18+
while (match = matcher.exec(pattern)) {
19+
if (match.index !== lastIndex) {
20+
tokens.push(pattern.slice(lastIndex, match.index));
21+
escapedSource += escapeRegExp(pattern.slice(lastIndex, match.index));
22+
}
23+
24+
if (match[1]) {
25+
escapedSource += '([^/?#]+)';
26+
paramNames.push(match[1]);
27+
} else if (match[0] === '*') {
28+
escapedSource += '(.*?)';
29+
paramNames.push('splat');
30+
} else if (match[0] === '(') {
31+
escapedSource += '(?:';
32+
} else if (match[0] === ')') {
33+
escapedSource += ')?';
34+
}
35+
36+
tokens.push(match[0]);
37+
38+
lastIndex = matcher.lastIndex;
39+
}
2640

27-
_compiledPatterns[pattern] = {
28-
matcher: new RegExp('^' + source + '$', 'i'),
29-
paramNames: paramNames
30-
};
41+
if (lastIndex !== pattern.length) {
42+
tokens.push(pattern.slice(lastIndex, pattern.length));
43+
escapedSource += escapeRegExp(pattern.slice(lastIndex, pattern.length));
3144
}
3245

46+
return {
47+
pattern,
48+
escapedSource,
49+
paramNames,
50+
tokens
51+
};
52+
}
53+
54+
var _compiledPatterns = {};
55+
56+
function compilePattern(pattern) {
57+
if (!(pattern in _compiledPatterns))
58+
_compiledPatterns[pattern] = _compilePattern(pattern);
59+
3360
return _compiledPatterns[pattern];
3461
}
3562

@@ -62,7 +89,8 @@ var PathUtils = {
6289
* pattern does not match the given path.
6390
*/
6491
extractParams: function (pattern, path) {
65-
var { matcher, paramNames } = compilePattern(pattern);
92+
var { escapedSource, paramNames } = compilePattern(pattern);
93+
var matcher = new RegExp('^' + escapedSource + '$', 'i');
6694
var match = path.match(matcher);
6795

6896
if (!match)
@@ -84,40 +112,46 @@ var PathUtils = {
84112
injectParams: function (pattern, params) {
85113
params = params || {};
86114

87-
var splatIndex = 0;
115+
var { tokens } = compilePattern(pattern);
116+
var parenCount = 0, pathname = '', splatIndex = 0;
88117

89-
return pattern.replace(paramInjectMatcher, function (match, paramName) {
90-
paramName = paramName || 'splat';
118+
var token, paramName, paramValue;
119+
for (var i = 0, len = tokens.length; i < len; ++i) {
120+
token = tokens[i];
91121

92-
// If param is optional don't check for existence
93-
if (paramName.slice(-1) === '?') {
94-
paramName = paramName.slice(0, -1);
122+
if (token === '*') {
123+
paramValue = Array.isArray(params.splat) ? params.splat[splatIndex++] : params.splat;
95124

96-
if (params[paramName] == null)
97-
return '';
98-
} else {
99125
invariant(
100-
params[paramName] != null,
101-
'Missing "%s" parameter for path "%s"',
102-
paramName, pattern
126+
paramValue != null || parenCount > 0,
127+
'Missing splat #%s for path "%s"',
128+
splatIndex, pattern
103129
);
104-
}
105130

106-
var segment;
107-
if (paramName === 'splat' && Array.isArray(params[paramName])) {
108-
segment = params[paramName][splatIndex++];
131+
if (paramValue != null)
132+
pathname += paramValue;
133+
} else if (token === '(') {
134+
parenCount += 1;
135+
} else if (token === ')') {
136+
parenCount -= 1;
137+
} else if (token.charAt(0) === ':') {
138+
paramName = token.substring(1);
139+
paramValue = params[paramName];
109140

110141
invariant(
111-
segment != null,
112-
'Missing splat # %s for path "%s"',
113-
splatIndex, pattern
142+
paramValue != null || parenCount > 0,
143+
'Missing "%s" parameter for path "%s"',
144+
paramName, pattern
114145
);
146+
147+
if (paramValue != null)
148+
pathname += paramValue;
115149
} else {
116-
segment = params[paramName];
150+
pathname += token;
117151
}
152+
}
118153

119-
return segment;
120-
}).replace(paramInjectTrailingSlashMatcher, '/');
154+
return pathname.replace(/\/+/g, '/');
121155
},
122156

123157
/**

‎modules/__tests__/PathUtils-test.js

Lines changed: 8 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -48,7 +48,7 @@ describe('PathUtils.extractParams', function () {
4848
});
4949

5050
describe('and the pattern is optional', function () {
51-
var pattern = 'comments/:id?/edit';
51+
var pattern = 'comments/(:id)/edit';
5252

5353
describe('and the path matches with supplied param', function () {
5454
it('returns an object with the params', function () {
@@ -64,7 +64,7 @@ describe('PathUtils.extractParams', function () {
6464
});
6565

6666
describe('and the pattern and forward slash are optional', function () {
67-
var pattern = 'comments/:id?/?edit';
67+
var pattern = 'comments(/:id)/edit';
6868

6969
describe('and the path matches with supplied param', function () {
7070
it('returns an object with the params', function () {
@@ -140,15 +140,13 @@ describe('PathUtils.extractParams', function () {
140140
});
141141
});
142142

143-
describe('when a pattern has a ?', function () {
144-
var pattern = '/archive/?:name?';
143+
describe('when a pattern has an optional group', function () {
144+
var pattern = '/archive(/:name)';
145145

146146
describe('and the path matches', function () {
147147
it('returns an object with the params', function () {
148-
expect(PathUtils.extractParams(pattern, '/archive')).toEqual({ name: undefined });
149-
expect(PathUtils.extractParams(pattern, '/archive/')).toEqual({ name: undefined });
150148
expect(PathUtils.extractParams(pattern, '/archive/foo')).toEqual({ name: 'foo' });
151-
expect(PathUtils.extractParams(pattern, '/archivefoo')).toEqual({ name: 'foo' });
149+
expect(PathUtils.extractParams(pattern, '/archive')).toEqual({ name: undefined });
152150
});
153151
});
154152

@@ -199,19 +197,19 @@ describe('PathUtils.injectParams', function () {
199197
});
200198

201199
describe('and a param is optional', function () {
202-
var pattern = 'comments/:id?/edit';
200+
var pattern = 'comments/(:id)/edit';
203201

204202
it('returns the correct path when param is supplied', function () {
205203
expect(PathUtils.injectParams(pattern, { id:'123' })).toEqual('comments/123/edit');
206204
});
207205

208206
it('returns the correct path when param is not supplied', function () {
209-
expect(PathUtils.injectParams(pattern, {})).toEqual('comments//edit');
207+
expect(PathUtils.injectParams(pattern, {})).toEqual('comments/edit');
210208
});
211209
});
212210

213211
describe('and a param and forward slash are optional', function () {
214-
var pattern = 'comments/:id?/?edit';
212+
var pattern = 'comments(/:id)/edit';
215213

216214
it('returns the correct path when param is supplied', function () {
217215
expect(PathUtils.injectParams(pattern, { id:'123' })).toEqual('comments/123/edit');
@@ -274,12 +272,6 @@ describe('PathUtils.injectParams', function () {
274272
expect(PathUtils.injectParams('/foo.bar.baz')).toEqual('/foo.bar.baz');
275273
});
276274
});
277-
278-
describe('when a pattern has optional slashes', function () {
279-
it('returns the correct path', function () {
280-
expect(PathUtils.injectParams('/foo/?/bar/?/baz/?')).toEqual('/foo/bar/baz/');
281-
});
282-
});
283275
});
284276

285277
describe('PathUtils.extractQuery', function () {

0 commit comments

Comments
 (0)