From 9cd747f6d39ad52141ec856d2c1aa4c97535f42b Mon Sep 17 00:00:00 2001 From: Jimmy Jia Date: Mon, 7 Mar 2016 13:07:06 -0500 Subject: [PATCH] Support trailing slashes, not extraneous ones --- modules/PatternUtils.js | 42 ++++----- modules/__tests__/_bc-Link-test.js | 2 +- modules/__tests__/_bc-isActive-test.js | 16 ++-- modules/__tests__/isActive-test.js | 120 +++++++++++++++++++++++-- modules/__tests__/matchPattern-test.js | 2 +- 5 files changed, 141 insertions(+), 41 deletions(-) diff --git a/modules/PatternUtils.js b/modules/PatternUtils.js index b62c489d9b..2d7edec196 100644 --- a/modules/PatternUtils.js +++ b/modules/PatternUtils.js @@ -4,10 +4,6 @@ function escapeRegExp(string) { return string.replace(/[.*+?^${}()|[\]\\]/g, '\\$&') } -function escapeSource(string) { - return escapeRegExp(string).replace(/\/+/g, '/+') -} - function _compilePattern(pattern) { let regexpSource = '' const paramNames = [] @@ -17,7 +13,7 @@ function _compilePattern(pattern) { while ((match = matcher.exec(pattern))) { if (match.index !== lastIndex) { tokens.push(pattern.slice(lastIndex, match.index)) - regexpSource += escapeSource(pattern.slice(lastIndex, match.index)) + regexpSource += escapeRegExp(pattern.slice(lastIndex, match.index)) } if (match[1]) { @@ -42,7 +38,7 @@ function _compilePattern(pattern) { if (lastIndex !== pattern.length) { tokens.push(pattern.slice(lastIndex, pattern.length)) - regexpSource += escapeSource(pattern.slice(lastIndex, pattern.length)) + regexpSource += escapeRegExp(pattern.slice(lastIndex, pattern.length)) } return { @@ -82,17 +78,15 @@ export function compilePattern(pattern) { * - paramValues */ export function matchPattern(pattern, pathname) { - // Make leading slashes consistent between pattern and pathname. + // Ensure pattern starts with leading slash for consistency with pathname. if (pattern.charAt(0) !== '/') { pattern = `/${pattern}` } - if (pathname.charAt(0) !== '/') { - pathname = `/${pathname}` - } - let { regexpSource, paramNames, tokens } = compilePattern(pattern) - regexpSource += '/*' // Capture path separators + if (pattern.charAt(pattern.length - 1) !== '/') { + regexpSource += '/?' // Allow optional path separator at end. + } // Special-case patterns like '*' for catch-all routes. if (tokens[tokens.length - 1] === '*') { @@ -106,18 +100,20 @@ export function matchPattern(pattern, pathname) { const matchedPath = match[0] remainingPathname = pathname.substr(matchedPath.length) - // 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 (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 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)) diff --git a/modules/__tests__/_bc-Link-test.js b/modules/__tests__/_bc-Link-test.js index b08c21c835..ab5aaeb346 100644 --- a/modules/__tests__/_bc-Link-test.js +++ b/modules/__tests__/_bc-Link-test.js @@ -52,7 +52,7 @@ describe('v1 Link', function () { Michael Ryan diff --git a/modules/__tests__/_bc-isActive-test.js b/modules/__tests__/_bc-isActive-test.js index 1ba0e02edc..354dfe98df 100644 --- a/modules/__tests__/_bc-isActive-test.js +++ b/modules/__tests__/_bc-isActive-test.js @@ -90,7 +90,7 @@ describe('v1 isActive', function () { }) }) - it('is active with extraneous slashes', function (done) { + it('is not active with extraneous slashes', function (done) { render(( @@ -98,7 +98,7 @@ describe('v1 isActive', function () { ), node, function () { - expect(this.history.isActive('/parent////child////')).toBe(true) + expect(this.history.isActive('/parent////child////')).toBe(false) done() }) }) @@ -288,7 +288,7 @@ describe('v1 isActive', function () { }) }) - it('is active with extraneous slashes', function (done) { + it('is not active with extraneous slashes', function (done) { render(( @@ -298,8 +298,8 @@ describe('v1 isActive', function () { ), node, function () { - expect(this.history.isActive('/parent///child///', null)).toBe(true) - expect(this.history.isActive('/parent///child///', null, true)).toBe(true) + expect(this.history.isActive('/parent///child///', null)).toBe(false) + expect(this.history.isActive('/parent///child///', null, true)).toBe(false) done() }) }) @@ -324,7 +324,7 @@ describe('v1 isActive', function () { }) }) - it('is active with extraneous slashes', function (done) { + it('is not active with extraneous slashes', function (done) { render(( @@ -336,8 +336,8 @@ describe('v1 isActive', function () { ), node, function () { - expect(this.history.isActive('/parent///child///', null)).toBe(true) - expect(this.history.isActive('/parent///child///', null, true)).toBe(true) + expect(this.history.isActive('/parent///child///', null)).toBe(false) + expect(this.history.isActive('/parent///child///', null, true)).toBe(false) done() }) }) diff --git a/modules/__tests__/isActive-test.js b/modules/__tests__/isActive-test.js index f5e47bfc67..d938ebe4b0 100644 --- a/modules/__tests__/isActive-test.js +++ b/modules/__tests__/isActive-test.js @@ -114,7 +114,7 @@ describe('isActive', function () { }) }) - it('is active with extraneous slashes', function (done) { + it('is active with trailing slash on pattern', function (done) { render(( @@ -122,7 +122,35 @@ describe('isActive', function () { ), node, function () { - expect(this.router.isActive('/parent////child////')).toBe(true) + expect(this.router.isActive('/parent/child/')).toBe(true) + done() + }) + }) + + it('is active with trailing slash on location', function (done) { + render(( + + + + + + ), 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(( + + + + + + ), node, function () { + expect(this.router.isActive('/parent//child')).toBe(false) + expect(this.router.isActive('/parent/child//')).toBe(false) done() }) }) @@ -336,7 +364,41 @@ describe('isActive', function () { }) }) - it('is active with extraneous slashes', function (done) { + it('is active with trailing slash on pattern', function (done) { + render(( + + + + + + + + ), 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(( + + + + + + + + ), 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) { render(( @@ -346,8 +408,10 @@ describe('isActive', function () { ), 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(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) done() }) }) @@ -372,7 +436,45 @@ describe('isActive', function () { }) }) - it('is active with extraneous slashes', function (done) { + it('is active with trailing slash on pattern', function (done) { + render(( + + + + + + + + + + ), 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(( + + + + + + + + + + ), 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) { render(( @@ -384,8 +486,10 @@ describe('isActive', function () { ), 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(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) done() }) }) diff --git a/modules/__tests__/matchPattern-test.js b/modules/__tests__/matchPattern-test.js index 6649296797..236d952576 100644 --- a/modules/__tests__/matchPattern-test.js +++ b/modules/__tests__/matchPattern-test.js @@ -12,7 +12,7 @@ describe('matchPattern', function () { } it('works without params', function () { - assertMatch('/', '/path', 'path', [], []) + assertMatch('/', '/path', '/path', [], []) }) it('works with named params', function () {