From a110f6eeb5bebafa5d6364b65e9f3502fadf516a Mon Sep 17 00:00:00 2001 From: Paul Sherman Date: Tue, 4 Apr 2017 23:31:30 -0500 Subject: [PATCH 1/6] Smarter stripPrefix Does a case-insensitive match and will not strip the prefix if the prefix is a partial match (e.g., given the pathname "/prefix/pathname", the basename "/pre" will not be stripped. --- modules/PathUtils.js | 2 +- modules/__tests__/PathUtils-test.js | 40 +++++++++++++++++++++++++++++ 2 files changed, 41 insertions(+), 1 deletion(-) create mode 100644 modules/__tests__/PathUtils-test.js diff --git a/modules/PathUtils.js b/modules/PathUtils.js index 4382e6981..8115c8e60 100644 --- a/modules/PathUtils.js +++ b/modules/PathUtils.js @@ -5,7 +5,7 @@ export const stripLeadingSlash = (path) => path.charAt(0) === '/' ? path.substr(1) : path export const stripPrefix = (path, prefix) => - path.indexOf(prefix) === 0 ? path.substr(prefix.length) : path + (new RegExp('^' + prefix + '(\\/|\\?|#|$)', 'i')).test(path) ? path.substr(prefix.length) : path export const stripTrailingSlash = (path) => path.charAt(path.length - 1) === '/' ? path.slice(0, -1) : path diff --git a/modules/__tests__/PathUtils-test.js b/modules/__tests__/PathUtils-test.js new file mode 100644 index 000000000..1bc1d7170 --- /dev/null +++ b/modules/__tests__/PathUtils-test.js @@ -0,0 +1,40 @@ +import expect from 'expect' +import { stripPrefix } from '../PathUtils' + +describe('stripPrefix', () => { + it('strips the prefix from the pathname', () => { + const path = '/prefix/pathname' + const prefix = '/prefix' + expect(stripPrefix(path, prefix)).toEqual('/pathname') + }) + + it('is not case-sensitive', () => { + const path = '/PREFIX/pathname' + const prefix = '/prefix' + expect(stripPrefix(path, prefix)).toEqual('/pathname') + }) + + it('does not strip partial prefix matches', () => { + const path = '/prefixed/pathname' + const prefix = '/prefix' + expect(stripPrefix(path, prefix)).toEqual(path) + }) + + it('strips when path is only the prefix', () => { + const path = '/prefix' + const prefix = '/prefix' + expect(stripPrefix(path, prefix)).toEqual('') + }) + + it('strips with no pathname, but with a search string', () => { + const path = '/prefix?a=b' + const prefix = '/prefix' + expect(stripPrefix(path, prefix)).toEqual('?a=b') + }) + + it('strips with no pathname, but with a hash string', () => { + const path = '/prefix#rest' + const prefix = '/prefix' + expect(stripPrefix(path, prefix)).toEqual('#rest') + }) +}) From 37f2ce894471221f509a3fb3b11250ebb947ac4b Mon Sep 17 00:00:00 2001 From: Paul Sherman Date: Thu, 20 Apr 2017 13:02:18 -0500 Subject: [PATCH 2/6] Test public API, strip Basename --- modules/PathUtils.js | 7 ++-- modules/__tests__/BrowserHistory-test.js | 39 ++++++++++++++++++++++ modules/__tests__/HashHistory-test.js | 42 ++++++++++++++++++++++++ modules/__tests__/PathUtils-test.js | 40 ---------------------- modules/createBrowserHistory.js | 4 +-- modules/createHashHistory.js | 4 +-- 6 files changed, 90 insertions(+), 46 deletions(-) delete mode 100644 modules/__tests__/PathUtils-test.js diff --git a/modules/PathUtils.js b/modules/PathUtils.js index 8115c8e60..70e55f9ec 100644 --- a/modules/PathUtils.js +++ b/modules/PathUtils.js @@ -4,8 +4,11 @@ export const addLeadingSlash = (path) => export const stripLeadingSlash = (path) => path.charAt(0) === '/' ? path.substr(1) : path -export const stripPrefix = (path, prefix) => - (new RegExp('^' + prefix + '(\\/|\\?|#|$)', 'i')).test(path) ? path.substr(prefix.length) : path +export const hasBasename = (path, prefix) => + (new RegExp('^' + prefix + '(\\/|\\?|#|$)', 'i')).test(path) + +export const stripBasename = (path, prefix) => + hasBasename(path, prefix) ? path.substr(prefix.length) : path export const stripTrailingSlash = (path) => path.charAt(path.length - 1) === '/' ? path.slice(0, -1) : path diff --git a/modules/__tests__/BrowserHistory-test.js b/modules/__tests__/BrowserHistory-test.js index 0e088c4dc..927f71977 100644 --- a/modules/__tests__/BrowserHistory-test.js +++ b/modules/__tests__/BrowserHistory-test.js @@ -1,3 +1,4 @@ +import expect from 'expect' import createHistory from '../createBrowserHistory' import { canUseDOM, supportsHistory } from '../DOMUtils' import * as TestSequences from './TestSequences' @@ -168,4 +169,42 @@ describeHistory('a browser history', () => { TestSequences.HashChangeTransitionHook(history, done) }) }) + + describe('basename', () => { + it('strips the basename from the pathname', () => { + window.history.replaceState(null, null, '/prefix/pathname') + const history = createHistory({ basename: '/prefix' }) + expect(history.location.pathname).toEqual('/pathname') + }) + + it('is not case-sensitive', () => { + window.history.replaceState(null, null, '/PREFIX/pathname') + const history = createHistory({ basename: '/prefix' }) + expect(history.location.pathname).toEqual('/pathname') + }) + + it('does not strip partial prefix matches', () => { + window.history.replaceState(null, null, '/prefixed/pathname') + const history = createHistory({ basename: '/prefix' }) + expect(history.location.pathname).toEqual('/prefixed/pathname') + }) + + it('strips when path is only the prefix', () => { + window.history.replaceState(null, null, '/prefix') + const history = createHistory({ basename: '/prefix' }) + expect(history.location.pathname).toEqual('/') + }) + + it('strips with no pathname, but with a search string', () => { + window.history.replaceState(null, null, '/prefix?a=b') + const history = createHistory({ basename: '/prefix' }) + expect(history.location.pathname).toEqual('') + }) + + it('strips with no pathname, but with a hash string', () => { + window.history.replaceState(null, null, '/prefix#rest') + const history = createHistory({ basename: '/prefix' }) + expect(history.location.pathname).toEqual('') + }) + }) }) diff --git a/modules/__tests__/HashHistory-test.js b/modules/__tests__/HashHistory-test.js index fa9465dd2..26536df77 100644 --- a/modules/__tests__/HashHistory-test.js +++ b/modules/__tests__/HashHistory-test.js @@ -1,3 +1,4 @@ +import expect from 'expect' import createHistory from '../createHashHistory' import { canUseDOM, supportsGoWithoutReloadUsingHash } from '../DOMUtils' import * as TestSequences from './TestSequences' @@ -208,4 +209,45 @@ describeHistory('a hash history', () => { TestSequences.SlashHashPathCoding(history, done) }) }) + + + describe('basename', () => { + it('strips the basename from the pathname', () => { + window.location.hash = '#/prefix/pathname' + const history = createHistory({ basename: '/prefix' }) + expect(history.location.pathname).toEqual('/pathname') + }) + + it('is not case-sensitive', () => { + window.location.hash = '#/PREFIX/pathname' + const history = createHistory({ basename: '/prefix' }) + expect(history.location.pathname).toEqual('/pathname') + }) + + it('does not strip partial prefix matches', () => { + window.location.hash = '#/prefixed/pathname' + const history = createHistory({ basename: '/prefix' }) + expect(history.location.pathname).toEqual('/prefixed/pathname') + }) + + it('strips when path is only the prefix', () => { + window.location.hash = '#/prefix' + const history = createHistory({ basename: '/prefix' }) + expect(history.location.pathname).toEqual('/') + }) + + it('strips with no pathname, but with a search string', () => { + window.location.hash = '#/prefix?a=b' + const history = createHistory({ basename: '/prefix' }) + expect(history.location.pathname).toEqual('') + }) + + it('strips with no pathname, but with a hash string', () => { + console.log('\n\n\n\n\n\n\n\n') + window.location.hash = '#/prefix#rest' + const history = createHistory({ basename: '/prefix' }) + expect(history.location.pathname).toEqual('') + console.log('\n\n\n\n\n\n\n\n') + }) + }) }) diff --git a/modules/__tests__/PathUtils-test.js b/modules/__tests__/PathUtils-test.js deleted file mode 100644 index 1bc1d7170..000000000 --- a/modules/__tests__/PathUtils-test.js +++ /dev/null @@ -1,40 +0,0 @@ -import expect from 'expect' -import { stripPrefix } from '../PathUtils' - -describe('stripPrefix', () => { - it('strips the prefix from the pathname', () => { - const path = '/prefix/pathname' - const prefix = '/prefix' - expect(stripPrefix(path, prefix)).toEqual('/pathname') - }) - - it('is not case-sensitive', () => { - const path = '/PREFIX/pathname' - const prefix = '/prefix' - expect(stripPrefix(path, prefix)).toEqual('/pathname') - }) - - it('does not strip partial prefix matches', () => { - const path = '/prefixed/pathname' - const prefix = '/prefix' - expect(stripPrefix(path, prefix)).toEqual(path) - }) - - it('strips when path is only the prefix', () => { - const path = '/prefix' - const prefix = '/prefix' - expect(stripPrefix(path, prefix)).toEqual('') - }) - - it('strips with no pathname, but with a search string', () => { - const path = '/prefix?a=b' - const prefix = '/prefix' - expect(stripPrefix(path, prefix)).toEqual('?a=b') - }) - - it('strips with no pathname, but with a hash string', () => { - const path = '/prefix#rest' - const prefix = '/prefix' - expect(stripPrefix(path, prefix)).toEqual('#rest') - }) -}) diff --git a/modules/createBrowserHistory.js b/modules/createBrowserHistory.js index bf0f06fb9..9cfcd701a 100644 --- a/modules/createBrowserHistory.js +++ b/modules/createBrowserHistory.js @@ -4,7 +4,7 @@ import { createLocation } from './LocationUtils' import { addLeadingSlash, stripTrailingSlash, - stripPrefix, + stripBasename, parsePath, createPath } from './PathUtils' @@ -66,7 +66,7 @@ const createBrowserHistory = (props = {}) => { ) if (basename) - path = stripPrefix(path, basename) + path = stripBasename(path, basename) return { ...parsePath(path), diff --git a/modules/createHashHistory.js b/modules/createHashHistory.js index b8a841564..a352d957e 100644 --- a/modules/createHashHistory.js +++ b/modules/createHashHistory.js @@ -5,7 +5,7 @@ import { addLeadingSlash, stripLeadingSlash, stripTrailingSlash, - stripPrefix, + stripBasename, parsePath, createPath } from './PathUtils' @@ -81,7 +81,7 @@ const createHashHistory = (props = {}) => { ) if (basename) - path = stripPrefix(path, basename) + path = stripBasename(path, basename) return parsePath(path) } From 22ca7e0222c236fc2e336bf0acfbfb9678337f7c Mon Sep 17 00:00:00 2001 From: Paul Sherman Date: Thu, 20 Apr 2017 13:05:59 -0500 Subject: [PATCH 3/6] Use hasBasename --- modules/createBrowserHistory.js | 3 ++- modules/createHashHistory.js | 3 ++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/modules/createBrowserHistory.js b/modules/createBrowserHistory.js index 9cfcd701a..e71f436f0 100644 --- a/modules/createBrowserHistory.js +++ b/modules/createBrowserHistory.js @@ -4,6 +4,7 @@ import { createLocation } from './LocationUtils' import { addLeadingSlash, stripTrailingSlash, + hasBasename, stripBasename, parsePath, createPath @@ -60,7 +61,7 @@ const createBrowserHistory = (props = {}) => { let path = pathname + search + hash warning( - !(basename && path.indexOf(basename) !== 0), + !(basename && hasBasename(path, basename)), 'You are attempting to use a basename on a page whose URL path does not begin ' + 'with the basename. Expected path "' + path + '" to begin with "' + basename + '".' ) diff --git a/modules/createHashHistory.js b/modules/createHashHistory.js index a352d957e..83b3f9f2e 100644 --- a/modules/createHashHistory.js +++ b/modules/createHashHistory.js @@ -5,6 +5,7 @@ import { addLeadingSlash, stripLeadingSlash, stripTrailingSlash, + hasBasename, stripBasename, parsePath, createPath @@ -75,7 +76,7 @@ const createHashHistory = (props = {}) => { let path = decodePath(getHashPath()) warning( - !(basename && path.indexOf(basename) !== 0), + !(basename && hasBasename(path, basename)), 'You are attempting to use a basename on a page whose URL path does not begin ' + 'with the basename. Expected path "' + path + '" to begin with "' + basename + '".' ) From 89b2a660acc555e2c9b4a8a629b80edd6570044a Mon Sep 17 00:00:00 2001 From: Paul Sherman Date: Thu, 20 Apr 2017 13:40:09 -0500 Subject: [PATCH 4/6] Have getDOMLocation call createLocation --- modules/LocationUtils.js | 5 +++++ modules/PathUtils.js | 2 +- modules/__tests__/BrowserHistory-test.js | 4 ++-- modules/__tests__/HashHistory-test.js | 6 ++---- modules/__tests__/LocationUtils-test.js | 4 ++-- modules/createBrowserHistory.js | 7 +------ modules/createHashHistory.js | 3 +-- 7 files changed, 14 insertions(+), 17 deletions(-) diff --git a/modules/LocationUtils.js b/modules/LocationUtils.js index a9dd903b7..92de40a73 100644 --- a/modules/LocationUtils.js +++ b/modules/LocationUtils.js @@ -42,6 +42,11 @@ export const createLocation = (path, state, key, currentLocation) => { } else if (location.pathname.charAt(0) !== '/') { location.pathname = resolvePathname(location.pathname, currentLocation.pathname) } + } else { + // When there is no prior location and pathname is empty, set it to / + if (!location.pathname) { + location.pathname = '/' + } } return location diff --git a/modules/PathUtils.js b/modules/PathUtils.js index 70e55f9ec..e398b9e3c 100644 --- a/modules/PathUtils.js +++ b/modules/PathUtils.js @@ -29,7 +29,7 @@ export const parsePath = (path) => { search = pathname.substr(searchIndex) pathname = pathname.substr(0, searchIndex) } - + pathname = decodeURI(pathname) return { diff --git a/modules/__tests__/BrowserHistory-test.js b/modules/__tests__/BrowserHistory-test.js index 927f71977..1fe4e9ed5 100644 --- a/modules/__tests__/BrowserHistory-test.js +++ b/modules/__tests__/BrowserHistory-test.js @@ -198,13 +198,13 @@ describeHistory('a browser history', () => { it('strips with no pathname, but with a search string', () => { window.history.replaceState(null, null, '/prefix?a=b') const history = createHistory({ basename: '/prefix' }) - expect(history.location.pathname).toEqual('') + expect(history.location.pathname).toEqual('/') }) it('strips with no pathname, but with a hash string', () => { window.history.replaceState(null, null, '/prefix#rest') const history = createHistory({ basename: '/prefix' }) - expect(history.location.pathname).toEqual('') + expect(history.location.pathname).toEqual('/') }) }) }) diff --git a/modules/__tests__/HashHistory-test.js b/modules/__tests__/HashHistory-test.js index 26536df77..4acfb18b4 100644 --- a/modules/__tests__/HashHistory-test.js +++ b/modules/__tests__/HashHistory-test.js @@ -239,15 +239,13 @@ describeHistory('a hash history', () => { it('strips with no pathname, but with a search string', () => { window.location.hash = '#/prefix?a=b' const history = createHistory({ basename: '/prefix' }) - expect(history.location.pathname).toEqual('') + expect(history.location.pathname).toEqual('/') }) it('strips with no pathname, but with a hash string', () => { - console.log('\n\n\n\n\n\n\n\n') window.location.hash = '#/prefix#rest' const history = createHistory({ basename: '/prefix' }) - expect(history.location.pathname).toEqual('') - console.log('\n\n\n\n\n\n\n\n') + expect(history.location.pathname).toEqual('/') }) }) }) diff --git a/modules/__tests__/LocationUtils-test.js b/modules/__tests__/LocationUtils-test.js index 5a242912b..72fd373b4 100644 --- a/modules/__tests__/LocationUtils-test.js +++ b/modules/__tests__/LocationUtils-test.js @@ -50,7 +50,7 @@ describe('createLocation', () => { describe('given as a string', () => { it('has the correct properties', () => { expect(createLocation('?the=query#the-hash')).toMatch({ - pathname: '', + pathname: '/', search: '?the=query', hash: '#the-hash' }) @@ -60,7 +60,7 @@ describe('createLocation', () => { describe('given as an object', () => { it('has the correct properties', () => { expect(createLocation({ search: '?the=query', hash: '#the-hash' })).toMatch({ - pathname: '', + pathname: '/', search: '?the=query', hash: '#the-hash' }) diff --git a/modules/createBrowserHistory.js b/modules/createBrowserHistory.js index e71f436f0..6b750b3ff 100644 --- a/modules/createBrowserHistory.js +++ b/modules/createBrowserHistory.js @@ -6,7 +6,6 @@ import { stripTrailingSlash, hasBasename, stripBasename, - parsePath, createPath } from './PathUtils' import createTransitionManager from './createTransitionManager' @@ -69,11 +68,7 @@ const createBrowserHistory = (props = {}) => { if (basename) path = stripBasename(path, basename) - return { - ...parsePath(path), - state, - key - } + return createLocation(path, state, key) } const createKey = () => diff --git a/modules/createHashHistory.js b/modules/createHashHistory.js index 83b3f9f2e..995d6bcfd 100644 --- a/modules/createHashHistory.js +++ b/modules/createHashHistory.js @@ -7,7 +7,6 @@ import { stripTrailingSlash, hasBasename, stripBasename, - parsePath, createPath } from './PathUtils' import createTransitionManager from './createTransitionManager' @@ -84,7 +83,7 @@ const createHashHistory = (props = {}) => { if (basename) path = stripBasename(path, basename) - return parsePath(path) + return createLocation(path) } const transitionManager = createTransitionManager() From 3a6d31e17f479c753421f8dfdd1a6aa36004fe79 Mon Sep 17 00:00:00 2001 From: Paul Sherman Date: Thu, 20 Apr 2017 14:01:25 -0500 Subject: [PATCH 5/6] Only add key property if key is provided --- modules/LocationUtils.js | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/modules/LocationUtils.js b/modules/LocationUtils.js index 92de40a73..f8deb25e9 100644 --- a/modules/LocationUtils.js +++ b/modules/LocationUtils.js @@ -33,7 +33,8 @@ export const createLocation = (path, state, key, currentLocation) => { location.state = state } - location.key = key + if (key) + location.key = key if (currentLocation) { // Resolve incomplete/relative pathname relative to current location. From c56bcbd273a5202b7eed512f0a264dbdb3da5dd2 Mon Sep 17 00:00:00 2001 From: Paul Sherman Date: Thu, 20 Apr 2017 14:45:16 -0500 Subject: [PATCH 6/6] Test createLocation with no key --- modules/__tests__/LocationUtils-test.js | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/modules/__tests__/LocationUtils-test.js b/modules/__tests__/LocationUtils-test.js index 72fd373b4..da9222304 100644 --- a/modules/__tests__/LocationUtils-test.js +++ b/modules/__tests__/LocationUtils-test.js @@ -111,4 +111,16 @@ describe('createLocation', () => { }) }) }) + + describe('key', () => { + it('has a key property if a key is provided', () => { + const location = createLocation('/the/path', undefined, 'key') + expect(location).toIncludeKey('key') + }) + + it('has no key property if no key is provided', () => { + const location = createLocation('/the/path') + expect(location).toExcludeKey('key') + }) + }) })