From e07508777843bc9a0e45e32e69e1d3330ef5973e Mon Sep 17 00:00:00 2001 From: Paul Sherman Date: Thu, 20 Apr 2017 17:16:52 -0500 Subject: [PATCH] Smarter stripPrefix (#459) * 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. * Test public API, strip Basename * Use hasBasename * Have getDOMLocation call createLocation * Only add key property if key is provided * Test createLocation with no key --- modules/LocationUtils.js | 8 ++++- modules/PathUtils.js | 9 ++++-- modules/__tests__/BrowserHistory-test.js | 39 +++++++++++++++++++++++ modules/__tests__/HashHistory-test.js | 40 ++++++++++++++++++++++++ modules/__tests__/LocationUtils-test.js | 16 ++++++++-- modules/createBrowserHistory.js | 14 +++------ modules/createHashHistory.js | 10 +++--- 7 files changed, 116 insertions(+), 20 deletions(-) diff --git a/modules/LocationUtils.js b/modules/LocationUtils.js index a9dd903b7..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. @@ -42,6 +43,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 4382e6981..e398b9e3c 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) => - path.indexOf(prefix) === 0 ? 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 @@ -26,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 0e088c4dc..1fe4e9ed5 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..4acfb18b4 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,43 @@ 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', () => { + window.location.hash = '#/prefix#rest' + const history = createHistory({ basename: '/prefix' }) + expect(history.location.pathname).toEqual('/') + }) + }) }) diff --git a/modules/__tests__/LocationUtils-test.js b/modules/__tests__/LocationUtils-test.js index 5a242912b..da9222304 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' }) @@ -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') + }) + }) }) diff --git a/modules/createBrowserHistory.js b/modules/createBrowserHistory.js index bf0f06fb9..6b750b3ff 100644 --- a/modules/createBrowserHistory.js +++ b/modules/createBrowserHistory.js @@ -4,8 +4,8 @@ import { createLocation } from './LocationUtils' import { addLeadingSlash, stripTrailingSlash, - stripPrefix, - parsePath, + hasBasename, + stripBasename, createPath } from './PathUtils' import createTransitionManager from './createTransitionManager' @@ -60,19 +60,15 @@ 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 + '".' ) if (basename) - path = stripPrefix(path, 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 b8a841564..995d6bcfd 100644 --- a/modules/createHashHistory.js +++ b/modules/createHashHistory.js @@ -5,8 +5,8 @@ import { addLeadingSlash, stripLeadingSlash, stripTrailingSlash, - stripPrefix, - parsePath, + hasBasename, + stripBasename, createPath } from './PathUtils' import createTransitionManager from './createTransitionManager' @@ -75,15 +75,15 @@ 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 + '".' ) if (basename) - path = stripPrefix(path, basename) + path = stripBasename(path, basename) - return parsePath(path) + return createLocation(path) } const transitionManager = createTransitionManager()