Skip to content

Commit 67d347c

Browse files
timdorrtimdorr
committed
Revert "Support trailing slashes, not extraneous ones" (#3280)
1 parent 92d305a commit 67d347c

File tree

5 files changed

+41
-141
lines changed

5 files changed

+41
-141
lines changed

‎modules/PatternUtils.js‎

Lines changed: 23 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,10 @@ function escapeRegExp(string) {
44
return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
55
}
66

7+
function escapeSource(string) {
8+
return escapeRegExp(string).replace(/\/+/g, '/+')
9+
}
10+
711
function _compilePattern(pattern) {
812
let regexpSource = ''
913
const paramNames = []
@@ -13,7 +17,7 @@ function _compilePattern(pattern) {
1317
while ((match = matcher.exec(pattern))) {
1418
if (match.index !== lastIndex) {
1519
tokens.push(pattern.slice(lastIndex, match.index))
16-
regexpSource += escapeRegExp(pattern.slice(lastIndex, match.index))
20+
regexpSource += escapeSource(pattern.slice(lastIndex, match.index))
1721
}
1822

1923
if (match[1]) {
@@ -38,7 +42,7 @@ function _compilePattern(pattern) {
3842

3943
if (lastIndex !== pattern.length) {
4044
tokens.push(pattern.slice(lastIndex, pattern.length))
41-
regexpSource += escapeRegExp(pattern.slice(lastIndex, pattern.length))
45+
regexpSource += escapeSource(pattern.slice(lastIndex, pattern.length))
4246
}
4347

4448
return {
@@ -78,15 +82,17 @@ export function compilePattern(pattern) {
7882
* - paramValues
7983
*/
8084
export function matchPattern(pattern, pathname) {
81-
// Ensure pattern starts with leading slash for consistency with pathname.
85+
// Make leading slashes consistent between pattern and pathname.
8286
if (pattern.charAt(0) !== '/') {
8387
pattern = `/${pattern}`
8488
}
89+
if (pathname.charAt(0) !== '/') {
90+
pathname = `/${pathname}`
91+
}
92+
8593
let { regexpSource, paramNames, tokens } = compilePattern(pattern)
8694

87-
if (pattern.charAt(pattern.length - 1) !== '/') {
88-
regexpSource += '/?' // Allow optional path separator at end.
89-
}
95+
regexpSource += '/*' // Capture path separators
9096

9197
// Special-case patterns like '*' for catch-all routes.
9298
if (tokens[tokens.length - 1] === '*') {
@@ -100,20 +106,18 @@ export function matchPattern(pattern, pathname) {
100106
const matchedPath = match[0]
101107
remainingPathname = pathname.substr(matchedPath.length)
102108

103-
if (remainingPathname) {
104-
// Require that the match ends at a path separator, if we didn't match
105-
// the full path, so any remaining pathname is a new path segment.
106-
if (matchedPath.charAt(matchedPath.length - 1) !== '/') {
107-
return {
108-
remainingPathname: null,
109-
paramNames,
110-
paramValues: null
111-
}
109+
// If we didn't match the entire pathname, then make sure that the match we
110+
// did get ends at a path separator (potentially the one we added above at
111+
// the beginning of the path, if the actual match was empty).
112+
if (
113+
remainingPathname &&
114+
matchedPath.charAt(matchedPath.length - 1) !== '/'
115+
) {
116+
return {
117+
remainingPathname: null,
118+
paramNames,
119+
paramValues: null
112120
}
113-
114-
// If there is a remaining pathname, treat the path separator as part of
115-
// the remaining pathname for properly continuing the match.
116-
remainingPathname = `/${remainingPathname}`
117121
}
118122

119123
paramValues = match.slice(1).map(v => v && decodeURIComponent(v))

‎modules/__tests__/_bc-Link-test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -57,7 +57,7 @@ describe('v1 Link', function () {
5757
Michael
5858
</Link>
5959
<Link
60-
to="/hello/ryan" query={{ the: 'query' }}
60+
to="hello/ryan" query={{ the: 'query' }}
6161
activeClassName="active"
6262
>
6363
Ryan

‎modules/__tests__/_bc-isActive-test.js‎

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -95,15 +95,15 @@ describe('v1 isActive', function () {
9595
})
9696
})
9797

98-
it('is not active with extraneous slashes', function (done) {
98+
it('is active with extraneous slashes', function (done) {
9999
render((
100100
<Router history={createHistory('/parent/child')}>
101101
<Route path="/parent">
102102
<Route path="child" />
103103
</Route>
104104
</Router>
105105
), node, function () {
106-
expect(this.history.isActive('/parent////child////')).toBe(false)
106+
expect(this.history.isActive('/parent////child////')).toBe(true)
107107
done()
108108
})
109109
})
@@ -293,7 +293,7 @@ describe('v1 isActive', function () {
293293
})
294294
})
295295

296-
it('is not active with extraneous slashes', function (done) {
296+
it('is active with extraneous slashes', function (done) {
297297
render((
298298
<Router history={createHistory('/parent/child')}>
299299
<Route path="/parent">
@@ -303,8 +303,8 @@ describe('v1 isActive', function () {
303303
</Route>
304304
</Router>
305305
), node, function () {
306-
expect(this.history.isActive('/parent///child///', null)).toBe(false)
307-
expect(this.history.isActive('/parent///child///', null, true)).toBe(false)
306+
expect(this.history.isActive('/parent///child///', null)).toBe(true)
307+
expect(this.history.isActive('/parent///child///', null, true)).toBe(true)
308308
done()
309309
})
310310
})
@@ -329,7 +329,7 @@ describe('v1 isActive', function () {
329329
})
330330
})
331331

332-
it('is not active with extraneous slashes', function (done) {
332+
it('is active with extraneous slashes', function (done) {
333333
render((
334334
<Router history={createHistory('/parent/child')}>
335335
<Route path="/parent">
@@ -341,8 +341,8 @@ describe('v1 isActive', function () {
341341
</Route>
342342
</Router>
343343
), node, function () {
344-
expect(this.history.isActive('/parent///child///', null)).toBe(false)
345-
expect(this.history.isActive('/parent///child///', null, true)).toBe(false)
344+
expect(this.history.isActive('/parent///child///', null)).toBe(true)
345+
expect(this.history.isActive('/parent///child///', null, true)).toBe(true)
346346
done()
347347
})
348348
})

‎modules/__tests__/isActive-test.js‎

Lines changed: 8 additions & 112 deletions
Original file line numberDiff line numberDiff line change
@@ -114,43 +114,15 @@ describe('isActive', function () {
114114
})
115115
})
116116

117-
it('is active with trailing slash on pattern', function (done) {
117+
it('is active with extraneous slashes', function (done) {
118118
render((
119119
<Router history={createHistory('/parent/child')}>
120120
<Route path="/parent">
121121
<Route path="child" />
122122
</Route>
123123
</Router>
124124
), node, function () {
125-
expect(this.router.isActive('/parent/child/')).toBe(true)
126-
done()
127-
})
128-
})
129-
130-
it('is active with trailing slash on location', function (done) {
131-
render((
132-
<Router history={createHistory('/parent/child/')}>
133-
<Route path="/parent">
134-
<Route path="child" />
135-
</Route>
136-
</Router>
137-
), node, function () {
138-
expect(this.router.isActive('/parent/child')).toBe(true)
139-
expect(this.router.isActive('/parent/child/')).toBe(true)
140-
done()
141-
})
142-
})
143-
144-
it('is not active with extraneous slashes', function (done) {
145-
render((
146-
<Router history={createHistory('/parent/child')}>
147-
<Route path="/parent">
148-
<Route path="child" />
149-
</Route>
150-
</Router>
151-
), node, function () {
152-
expect(this.router.isActive('/parent//child')).toBe(false)
153-
expect(this.router.isActive('/parent/child//')).toBe(false)
125+
expect(this.router.isActive('/parent////child////')).toBe(true)
154126
done()
155127
})
156128
})
@@ -364,41 +336,7 @@ describe('isActive', function () {
364336
})
365337
})
366338

367-
it('is active with trailing slash on pattern', function (done) {
368-
render((
369-
<Router history={createHistory('/parent/child')}>
370-
<Route path="/parent">
371-
<Route path="child">
372-
<IndexRoute />
373-
</Route>
374-
</Route>
375-
</Router>
376-
), node, function () {
377-
expect(this.router.isActive('/parent/child/')).toBe(true)
378-
expect(this.router.isActive('/parent/child/', true)).toBe(true)
379-
done()
380-
})
381-
})
382-
383-
it('is active with trailing slash on location', function (done) {
384-
render((
385-
<Router history={createHistory('/parent/child/')}>
386-
<Route path="/parent">
387-
<Route path="child">
388-
<IndexRoute />
389-
</Route>
390-
</Route>
391-
</Router>
392-
), node, function () {
393-
expect(this.router.isActive('/parent/child')).toBe(true)
394-
expect(this.router.isActive('/parent/child', true)).toBe(true)
395-
expect(this.router.isActive('/parent/child/')).toBe(true)
396-
expect(this.router.isActive('/parent/child/', true)).toBe(true)
397-
done()
398-
})
399-
})
400-
401-
it('is not active with extraneous slashes', function (done) {
339+
it('is active with extraneous slashes', function (done) {
402340
render((
403341
<Router history={createHistory('/parent/child')}>
404342
<Route path="/parent">
@@ -408,10 +346,8 @@ describe('isActive', function () {
408346
</Route>
409347
</Router>
410348
), node, function () {
411-
expect(this.router.isActive('/parent//child')).toBe(false)
412-
expect(this.router.isActive('/parent/child//')).toBe(false)
413-
expect(this.router.isActive('/parent//child', true)).toBe(false)
414-
expect(this.router.isActive('/parent/child//', true)).toBe(false)
349+
expect(this.router.isActive('/parent///child///')).toBe(true)
350+
expect(this.router.isActive('/parent///child///', true)).toBe(true)
415351
done()
416352
})
417353
})
@@ -436,45 +372,7 @@ describe('isActive', function () {
436372
})
437373
})
438374

439-
it('is active with trailing slash on pattern', function (done) {
440-
render((
441-
<Router history={createHistory('/parent/child')}>
442-
<Route path="/parent">
443-
<Route path="child">
444-
<Route>
445-
<IndexRoute />
446-
</Route>
447-
</Route>
448-
</Route>
449-
</Router>
450-
), node, function () {
451-
expect(this.router.isActive('/parent/child/')).toBe(true)
452-
expect(this.router.isActive('/parent/child/', true)).toBe(true)
453-
done()
454-
})
455-
})
456-
457-
it('is active with trailing slash on location', function (done) {
458-
render((
459-
<Router history={createHistory('/parent/child/')}>
460-
<Route path="/parent">
461-
<Route path="child">
462-
<Route>
463-
<IndexRoute />
464-
</Route>
465-
</Route>
466-
</Route>
467-
</Router>
468-
), node, function () {
469-
expect(this.router.isActive('/parent/child')).toBe(true)
470-
expect(this.router.isActive('/parent/child', true)).toBe(true)
471-
expect(this.router.isActive('/parent/child/')).toBe(true)
472-
expect(this.router.isActive('/parent/child/', true)).toBe(true)
473-
done()
474-
})
475-
})
476-
477-
it('is not active with extraneous slashes', function (done) {
375+
it('is active with extraneous slashes', function (done) {
478376
render((
479377
<Router history={createHistory('/parent/child')}>
480378
<Route path="/parent">
@@ -486,10 +384,8 @@ describe('isActive', function () {
486384
</Route>
487385
</Router>
488386
), node, function () {
489-
expect(this.router.isActive('/parent//child')).toBe(false)
490-
expect(this.router.isActive('/parent/child//')).toBe(false)
491-
expect(this.router.isActive('/parent//child', true)).toBe(false)
492-
expect(this.router.isActive('/parent/child//', true)).toBe(false)
387+
expect(this.router.isActive('/parent///child///')).toBe(true)
388+
expect(this.router.isActive('/parent///child///', true)).toBe(true)
493389
done()
494390
})
495391
})

‎modules/__tests__/matchPattern-test.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,7 @@ describe('matchPattern', function () {
1212
}
1313

1414
it('works without params', function () {
15-
assertMatch('/', '/path', '/path', [], [])
15+
assertMatch('/', '/path', 'path', [], [])
1616
})
1717

1818
it('works with named params', function () {

0 commit comments

Comments
 (0)