Skip to content
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

Jump to bottom

Revert "Support trailing slashes, not extraneous ones" #3280

Merged
merged 1 commit into from
Apr 11, 2016
Merged

Revert "Support trailing slashes, not extraneous ones" #3280

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
42 changes: 23 additions & 19 deletions modules/PatternUtils.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,10 @@ function escapeRegExp(string) {
return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&')
}

function escapeSource(string) {
return escapeRegExp(string).replace(/\/+/g, '/+')
}

function _compilePattern(pattern) {
let regexpSource = ''
const paramNames = []
Expand All @@ -13,7 +17,7 @@ function _compilePattern(pattern) {
while ((match = matcher.exec(pattern))) {
if (match.index !== lastIndex) {
tokens.push(pattern.slice(lastIndex, match.index))
regexpSource += escapeRegExp(pattern.slice(lastIndex, match.index))
regexpSource += escapeSource(pattern.slice(lastIndex, match.index))
}

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

if (lastIndex !== pattern.length) {
tokens.push(pattern.slice(lastIndex, pattern.length))
regexpSource += escapeRegExp(pattern.slice(lastIndex, pattern.length))
regexpSource += escapeSource(pattern.slice(lastIndex, pattern.length))
}

return {
Expand Down Expand Up @@ -78,15 +82,17 @@ export function compilePattern(pattern) {
* - paramValues
*/
export function matchPattern(pattern, pathname) {
// Ensure pattern starts with leading slash for consistency with pathname.
// Make leading slashes consistent between pattern and pathname.
if (pattern.charAt(0) !== '/') {
pattern = `/${pattern}`
}
if (pathname.charAt(0) !== '/') {
pathname = `/${pathname}`
}

let { regexpSource, paramNames, tokens } = compilePattern(pattern)

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

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

if (remainingPathname) {
// Require that the match ends at a path separator, if we didn't match
// the full path, so any remaining pathname is a new path segment.
if (matchedPath.charAt(matchedPath.length - 1) !== '/') {
return {
remainingPathname: null,
paramNames,
paramValues: null
}
// If we didn't match the entire pathname, then make sure that the match we
// did get ends at a path separator (potentially the one we added above at
// the beginning of the path, if the actual match was empty).
if (
remainingPathname &&
matchedPath.charAt(matchedPath.length - 1) !== '/'
) {
return {
remainingPathname: null,
paramNames,
paramValues: null
}

// If there is a remaining pathname, treat the path separator as part of
// the remaining pathname for properly continuing the match.
remainingPathname = `/${remainingPathname}`
}

paramValues = match.slice(1).map(v => v && decodeURIComponent(v))
Expand Down
2 changes: 1 addition & 1 deletion modules/__tests__/_bc-Link-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ describe('v1 Link', function () {
Michael
</Link>
<Link
to="/hello/ryan" query={{ the: 'query' }}
to="hello/ryan" query={{ the: 'query' }}
activeClassName="active"
>
Ryan
Expand Down
16 changes: 8 additions & 8 deletions modules/__tests__/_bc-isActive-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -95,15 +95,15 @@ describe('v1 isActive', function () {
})
})

it('is not active with extraneous slashes', function (done) {
it('is active with extraneous slashes', function (done) {
render((
<Router history={createHistory('/parent/child')}>
<Route path="/parent">
<Route path="child" />
</Route>
</Router>
), node, function () {
expect(this.history.isActive('/parent////child////')).toBe(false)
expect(this.history.isActive('/parent////child////')).toBe(true)
done()
})
})
Expand Down Expand Up @@ -293,7 +293,7 @@ describe('v1 isActive', function () {
})
})

it('is not active with extraneous slashes', function (done) {
it('is active with extraneous slashes', function (done) {
render((
<Router history={createHistory('/parent/child')}>
<Route path="/parent">
Expand All @@ -303,8 +303,8 @@ describe('v1 isActive', function () {
</Route>
</Router>
), node, function () {
expect(this.history.isActive('/parent///child///', null)).toBe(false)
expect(this.history.isActive('/parent///child///', null, true)).toBe(false)
expect(this.history.isActive('/parent///child///', null)).toBe(true)
expect(this.history.isActive('/parent///child///', null, true)).toBe(true)
done()
})
})
Expand All @@ -329,7 +329,7 @@ describe('v1 isActive', function () {
})
})

it('is not active with extraneous slashes', function (done) {
it('is active with extraneous slashes', function (done) {
render((
<Router history={createHistory('/parent/child')}>
<Route path="/parent">
Expand All @@ -341,8 +341,8 @@ describe('v1 isActive', function () {
</Route>
</Router>
), node, function () {
expect(this.history.isActive('/parent///child///', null)).toBe(false)
expect(this.history.isActive('/parent///child///', null, true)).toBe(false)
expect(this.history.isActive('/parent///child///', null)).toBe(true)
expect(this.history.isActive('/parent///child///', null, true)).toBe(true)
done()
})
})
Expand Down
120 changes: 8 additions & 112 deletions modules/__tests__/isActive-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -114,43 +114,15 @@ describe('isActive', function () {
})
})

it('is active with trailing slash on pattern', function (done) {
it('is active with extraneous slashes', function (done) {
render((
<Router history={createHistory('/parent/child')}>
<Route path="/parent">
<Route path="child" />
</Route>
</Router>
), node, function () {
expect(this.router.isActive('/parent/child/')).toBe(true)
done()
})
})

it('is active with trailing slash on location', function (done) {
render((
<Router history={createHistory('/parent/child/')}>
<Route path="/parent">
<Route path="child" />
</Route>
</Router>
), node, function () {
expect(this.router.isActive('/parent/child')).toBe(true)
expect(this.router.isActive('/parent/child/')).toBe(true)
done()
})
})

it('is not active with extraneous slashes', function (done) {
render((
<Router history={createHistory('/parent/child')}>
<Route path="/parent">
<Route path="child" />
</Route>
</Router>
), node, function () {
expect(this.router.isActive('/parent//child')).toBe(false)
expect(this.router.isActive('/parent/child//')).toBe(false)
expect(this.router.isActive('/parent////child////')).toBe(true)
done()
})
})
Expand Down Expand Up @@ -364,41 +336,7 @@ describe('isActive', function () {
})
})

it('is active with trailing slash on pattern', function (done) {
render((
<Router history={createHistory('/parent/child')}>
<Route path="/parent">
<Route path="child">
<IndexRoute />
</Route>
</Route>
</Router>
), node, function () {
expect(this.router.isActive('/parent/child/')).toBe(true)
expect(this.router.isActive('/parent/child/', true)).toBe(true)
done()
})
})

it('is active with trailing slash on location', function (done) {
render((
<Router history={createHistory('/parent/child/')}>
<Route path="/parent">
<Route path="child">
<IndexRoute />
</Route>
</Route>
</Router>
), node, function () {
expect(this.router.isActive('/parent/child')).toBe(true)
expect(this.router.isActive('/parent/child', true)).toBe(true)
expect(this.router.isActive('/parent/child/')).toBe(true)
expect(this.router.isActive('/parent/child/', true)).toBe(true)
done()
})
})

it('is not active with extraneous slashes', function (done) {
it('is active with extraneous slashes', function (done) {
render((
<Router history={createHistory('/parent/child')}>
<Route path="/parent">
Expand All @@ -408,10 +346,8 @@ describe('isActive', function () {
</Route>
</Router>
), node, function () {
expect(this.router.isActive('/parent//child')).toBe(false)
expect(this.router.isActive('/parent/child//')).toBe(false)
expect(this.router.isActive('/parent//child', true)).toBe(false)
expect(this.router.isActive('/parent/child//', true)).toBe(false)
expect(this.router.isActive('/parent///child///')).toBe(true)
expect(this.router.isActive('/parent///child///', true)).toBe(true)
done()
})
})
Expand All @@ -436,45 +372,7 @@ describe('isActive', function () {
})
})

it('is active with trailing slash on pattern', function (done) {
render((
<Router history={createHistory('/parent/child')}>
<Route path="/parent">
<Route path="child">
<Route>
<IndexRoute />
</Route>
</Route>
</Route>
</Router>
), node, function () {
expect(this.router.isActive('/parent/child/')).toBe(true)
expect(this.router.isActive('/parent/child/', true)).toBe(true)
done()
})
})

it('is active with trailing slash on location', function (done) {
render((
<Router history={createHistory('/parent/child/')}>
<Route path="/parent">
<Route path="child">
<Route>
<IndexRoute />
</Route>
</Route>
</Route>
</Router>
), node, function () {
expect(this.router.isActive('/parent/child')).toBe(true)
expect(this.router.isActive('/parent/child', true)).toBe(true)
expect(this.router.isActive('/parent/child/')).toBe(true)
expect(this.router.isActive('/parent/child/', true)).toBe(true)
done()
})
})

it('is not active with extraneous slashes', function (done) {
it('is active with extraneous slashes', function (done) {
render((
<Router history={createHistory('/parent/child')}>
<Route path="/parent">
Expand All @@ -486,10 +384,8 @@ describe('isActive', function () {
</Route>
</Router>
), node, function () {
expect(this.router.isActive('/parent//child')).toBe(false)
expect(this.router.isActive('/parent/child//')).toBe(false)
expect(this.router.isActive('/parent//child', true)).toBe(false)
expect(this.router.isActive('/parent/child//', true)).toBe(false)
expect(this.router.isActive('/parent///child///')).toBe(true)
expect(this.router.isActive('/parent///child///', true)).toBe(true)
done()
})
})
Expand Down
2 changes: 1 addition & 1 deletion modules/__tests__/matchPattern-test.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ describe('matchPattern', function () {
}

it('works without params', function () {
assertMatch('/', '/path', '/path', [], [])
assertMatch('/', '/path', 'path', [], [])
})

it('works with named params', function () {
Expand Down