Skip to content

Commit

Permalink
fix trailing slash handling (#3923)
Browse files Browse the repository at this point in the history
- Add tests for matching pattern with trailing slash.
- Fix non-exact match of pattern with trailing slash.
- Fix incorrect match of pattern with trailing slash.
- Simplify matchPattern with pathToRegexp 'end' option.
  • Loading branch information
aaugustin authored and ryanflorence committed Oct 6, 2016
1 parent d87cae8 commit e5be03a
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 15 deletions.
79 changes: 79 additions & 0 deletions modules/__tests__/Match-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -283,6 +283,85 @@ describe('Match', () => {
})
})

describe('with a trailing slash', () => {
const TEXT = 'TEXT'
const run = (location, cb) => (
cb(renderToString(
<Match
pattern="/foo/"
location={location}
render={() => (
<div>{TEXT}</div>
)}
/>
))
)

describe('when matched exactly', () => {
it('renders', () => {
run({ pathname: '/foo/' }, (html) => {
expect(html).toContain(TEXT)
})
})
})

describe('when matched partially', () => {
it('renders', () => {
run({ pathname: '/foo/bar/' }, (html) => {
expect(html).toContain(TEXT)
})
})
})

describe('when the trailing slash is missing', () => {
it('does not renders', () => {
run({ pathname: '/foo' }, (html) => {
expect(html).toNotContain(TEXT)
})
})
})
})

describe('`exactly` prop with a trailing slash', () => {
const TEXT = 'TEXT'
const run = (location, cb) => (
cb(renderToString(
<Match
exactly
pattern="/foo/"
location={location}
render={() => (
<div>{TEXT}</div>
)}
/>
))
)

describe('when matched exactly', () => {
it('renders', () => {
run({ pathname: '/foo/' }, (html) => {
expect(html).toContain(TEXT)
})
})
})

describe('when matched partially', () => {
it('does not render', () => {
run({ pathname: '/foo/bar/' }, (html) => {
expect(html).toNotContain(TEXT)
})
})
})

describe('when the trailing slash is missing', () => {
it('does not renders', () => {
run({ pathname: '/foo' }, (html) => {
expect(html).toNotContain(TEXT)
})
})
})
})

describe('when rendered in context of a location', () => {
it('matches the location from context', () => {
const TEXT = 'TEXT'
Expand Down
27 changes: 12 additions & 15 deletions modules/matchPattern.js
Original file line number Diff line number Diff line change
@@ -1,22 +1,21 @@
import pathToRegexp from 'path-to-regexp'

const cache = {}
// cache[exactly][pattern] contains getMatcher(pattern, exactly)
const cache = {true: {}, false: {}}

const getMatcher = (pattern) => {
let matcher = cache[pattern]
const getMatcher = (pattern, exactly) => {
const exactlyStr = exactly ? 'true' : 'false'
let matcher = cache[exactlyStr][pattern]

if (!matcher) {
const keys = []
const regex = pathToRegexp(pattern, keys)
matcher = cache[pattern] = { keys, regex }
const regex = pathToRegexp(pattern, keys, { end: exactly, strict: true })
matcher = cache[exactlyStr][pattern] = { keys, regex }
}

return matcher
}

const truncatePathnameToPattern = (pathname, pattern) =>
pathname.split('/').slice(0, pattern.split('/').length).join('/')

const parseParams = (pattern, match, keys) =>
match.slice(1).reduce((params, value, index) => {
params[keys[index].name] = decodeURIComponent(value)
Expand All @@ -39,16 +38,14 @@ const matchPattern = (pattern, location, matchExactly, parent) => {
pattern
}

const matcher = getMatcher(pattern)
const pathname = matchExactly ?
location.pathname : truncatePathnameToPattern(location.pathname, pattern)
const match = matcher.regex.exec(pathname)
const matcher = getMatcher(pattern, matchExactly)
const match = matcher.regex.exec(location.pathname)

if (match) {
const params = parseParams(pattern, match, matcher.keys)
const locationLength = location.pathname.split('/').length
const patternLength = pattern.split('/').length
const isExact = locationLength === patternLength
const pathname = match[0]
const isExact = pathname === location.pathname

return { params, isExact, pathname }
} else {
return null
Expand Down

1 comment on commit e5be03a

@aaugustin
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Please sign in to comment.