From d6b1290e99616fba041f796af7799dbcee04fade Mon Sep 17 00:00:00 2001 From: Paul Sherman Date: Thu, 19 Jan 2017 11:28:51 -0600 Subject: [PATCH 1/3] resolve relative paths The function resolveLocation can be used for resolving location descriptors and strings. Resolving is done with a modified version of RFC 3986's resolution algorithm in which the trailing URL segment is not removed. This allows a to be resolved relative to the that is is located in. resolveLocation supports pathnames that include double dot notation (../foo). The function simpleResolve can be used for resolving and pathnames. This simply joins the pathname (iff it is not absolute) to the base pathname using a forward slash. --- packages/react-router-dom/modules/Link.js | 35 +++- .../modules/__tests__/Link-test.js | 91 +++++++++++ .../modules/components/ExampleRouter.js | 41 ++++- packages/react-router/modules/Redirect.js | 11 +- packages/react-router/modules/Route.js | 12 +- packages/react-router/modules/Switch.js | 13 +- .../modules/__tests__/Redirect-test.js | 34 ++++ .../modules/__tests__/Route-test.js | 56 +++++++ .../modules/__tests__/Switch-test.js | 14 ++ .../modules/__tests__/resolve-test.js | 152 ++++++++++++++++++ packages/react-router/modules/resolve.js | 60 +++++++ packages/react-router/package.json | 2 + 12 files changed, 503 insertions(+), 18 deletions(-) create mode 100644 packages/react-router/modules/__tests__/Redirect-test.js create mode 100644 packages/react-router/modules/__tests__/resolve-test.js create mode 100644 packages/react-router/modules/resolve.js diff --git a/packages/react-router-dom/modules/Link.js b/packages/react-router-dom/modules/Link.js index 1d16c50b99..3bfd828d49 100644 --- a/packages/react-router-dom/modules/Link.js +++ b/packages/react-router-dom/modules/Link.js @@ -1,4 +1,5 @@ import React, { PropTypes } from 'react' +import { resolveLocation } from 'react-router/resolve' const isModifiedEvent = (event) => !!(event.metaKey || event.altKey || event.ctrlKey || event.shiftKey) @@ -7,11 +8,15 @@ const isModifiedEvent = (event) => * The public API for rendering a router-aware . */ class Link extends React.Component { + static contextTypes = { router: PropTypes.shape({ push: PropTypes.func.isRequired, replace: PropTypes.func.isRequired, - createHref: PropTypes.func.isRequired + createHref: PropTypes.func.isRequired, + match: PropTypes.shape({ + url: PropTypes.string + }) }).isRequired } @@ -42,8 +47,8 @@ class Link extends React.Component { event.preventDefault() const { router } = this.context - const { replace, to } = this.props - + const { replace } = this.props + const to = this.absolutePathname() if (replace) { router.replace(to) } else { @@ -52,11 +57,29 @@ class Link extends React.Component { } } + absolutePathname() { + const { router } = this.context + const { to } = this.props + const { match } = router + const base = (match && match.url) ? match.url : '' + return resolveLocation(to, base) + } + + componentWillMount() { + const { router } = this.context + this.unlisten = router.listen(() => this.forceUpdate()) + } + + componentWillUnmount() { + this.unlisten() + } + render() { + const { router } = this.context const { replace, to, ...props } = this.props // eslint-disable-line no-unused-vars - - const href = this.context.router.createHref( - typeof to === 'string' ? { pathname: to } : to + const absoluteTo = this.absolutePathname() + const href = router.createHref( + typeof absoluteTo === 'string' ? { pathname: absoluteTo } : absoluteTo ) return diff --git a/packages/react-router-dom/modules/__tests__/Link-test.js b/packages/react-router-dom/modules/__tests__/Link-test.js index 78ce53f5d1..d990e30ada 100644 --- a/packages/react-router-dom/modules/__tests__/Link-test.js +++ b/packages/react-router-dom/modules/__tests__/Link-test.js @@ -1,7 +1,10 @@ import expect from 'expect' import React from 'react' import ReactDOM from 'react-dom' +import { Simulate } from 'react-addons-test-utils' import MemoryRouter from 'react-router/MemoryRouter' +import Route from 'react-router/Route' +import Switch from 'react-router/Switch' import HashRouter from '../HashRouter' import Link from '../Link' @@ -88,4 +91,92 @@ describe('A underneath a ', () => { expect(linkNode.getAttribute('href')).toEqual('#foo') }) }) + + describe('relative to', () => { + const node = document.createElement('div') + + afterEach(() => { + ReactDOM.unmountComponentAtNode(node) + }) + + it('resolves using the parent match', () => { + const initialEntries = ['/', '/recipes'] + ReactDOM.render(( + + ( + Chess + )} /> + + ), node) + const a = node.getElementsByTagName('a')[0] + expect(a.pathname).toBe('/recipes/tacos') + }) + + it('works when not in a route', () => { + const initialEntries = ['/'] + ReactDOM.render(( + + Recipes + + ), node) + const a = node.getElementsByTagName('a')[0] + expect(a.pathname).toBe('/recipes') + }) + + it('navigates correctly', () => { + const initialEntries = ['/', '/recipes'] + const RESTAURANTS = 'RESTAURANTS' + ReactDOM.render(( + + + ( + Order Takeout + )} /> + ( +
{RESTAURANTS}
+ )} /> + + + ), node) + expect(node.textContent).toNotContain(RESTAURANTS) + const a = node.getElementsByTagName('a')[0] + Simulate.click(a, { + defaultPrevented: false, + preventDefault() { this.defaultPrevented = true }, + metaKey: null, + altKey: null, + ctrlKey: null, + shiftKey: null, + button: 0 + }) + expect(node.textContent).toContain(RESTAURANTS) + }) + + it('updates regardless of sCU blocks', () => { + let goForward + class UpdateBlocker extends React.Component { + shouldComponentUpdate() { + return false + } + render() { + goForward = this.props.goForward + return + } + } + + ReactDOM.render(( + + + + ), node) + + let href = node.querySelector('a').getAttribute('href') + expect(href).toEqual('/bubblegum/store') + + goForward() + + href = node.querySelector('a').getAttribute('href') + expect(href).toEqual('/shoelaces/store') + }) + }) }) diff --git a/packages/react-router-website/modules/components/ExampleRouter.js b/packages/react-router-website/modules/components/ExampleRouter.js index a23b496e3d..7ed6c8603b 100644 --- a/packages/react-router-website/modules/components/ExampleRouter.js +++ b/packages/react-router-website/modules/components/ExampleRouter.js @@ -1,7 +1,40 @@ -import React from 'react' +import React, { PropTypes } from 'react' -const ExampleRouter = ({ children }) => ( - children ? React.Children.only(children) : null -) +class ExampleRouter extends React.Component { + + static contextTypes = { + router: PropTypes.object.isRequired + } + + static childContextTypes = { + router: PropTypes.object.isRequired + } + + getChildContext() { + return { + router: this.router + } + } + + componentWillMount() { + this.router = { + ...this.context.router, + match: null + } + + this.unlisten = this.router.listen(() => { + this.router.location = this.context.router.location + }) + } + + componentWillUnmount() { + this.unlisten() + } + + render() { + const { children } = this.props + return children ? React.Children.only(children) : null + } +} export default ExampleRouter diff --git a/packages/react-router/modules/Redirect.js b/packages/react-router/modules/Redirect.js index 00accac0cf..2544a10284 100644 --- a/packages/react-router/modules/Redirect.js +++ b/packages/react-router/modules/Redirect.js @@ -1,4 +1,5 @@ import React, { PropTypes } from 'react' +import { resolveLocation } from './resolve' /** * The public API for updating the location programatically @@ -10,6 +11,9 @@ class Redirect extends React.Component { push: PropTypes.func.isRequired, replace: PropTypes.func.isRequired, staticContext: PropTypes.object + match: PropTypes.shape({ + url: PropTypes.string + }) }).isRequired } @@ -38,11 +42,12 @@ class Redirect extends React.Component { perform() { const { router } = this.context const { push, to } = this.props - + const { match } = router + const loc = resolveLocation(to, match && match.url ? match.url : '') if (push) { - router.push(to) + router.push(loc) } else { - router.replace(to) + router.replace(loc) } } diff --git a/packages/react-router/modules/Route.js b/packages/react-router/modules/Route.js index 451427cfb9..df1ec623f6 100644 --- a/packages/react-router/modules/Route.js +++ b/packages/react-router/modules/Route.js @@ -1,9 +1,14 @@ import warning from 'warning' import React, { PropTypes } from 'react' import matchPath from './matchPath' +import { simpleResolve } from './resolve' const computeMatch = (router, { computedMatch, path, exact, strict }) => - computedMatch || matchPath(router.location.pathname, path, { exact, strict }) + computedMatch || matchPath( + router.location.pathname, + simpleResolve(path, router.match && router.match.path ? router.match.path : ''), + { exact, strict } + ) /** * The public API for matching a single path and rendering. @@ -51,7 +56,8 @@ class Route extends React.Component { static contextTypes = { router: PropTypes.shape({ - listen: PropTypes.func.isRequired + listen: PropTypes.func.isRequired, + match: PropTypes.object }).isRequired } @@ -98,7 +104,7 @@ class Route extends React.Component { componentWillReceiveProps(nextProps) { Object.assign(this.router, { - match: computeMatch(this.router, nextProps) + match: computeMatch(this.context.router, nextProps) }) } diff --git a/packages/react-router/modules/Switch.js b/packages/react-router/modules/Switch.js index c7627685ff..1564b047ff 100644 --- a/packages/react-router/modules/Switch.js +++ b/packages/react-router/modules/Switch.js @@ -1,5 +1,6 @@ import React, { PropTypes } from 'react' import matchPath from './matchPath' +import { simpleResolve } from './resolve' /** * The public API for rendering the first that matches. @@ -7,7 +8,8 @@ import matchPath from './matchPath' class Switch extends React.Component { static contextTypes = { router: PropTypes.shape({ - listen: PropTypes.func.isRequired + listen: PropTypes.func.isRequired, + match: PropTypes.object }).isRequired } @@ -41,12 +43,19 @@ class Switch extends React.Component { render() { const { children } = this.props const { location } = this.state + const { match:parentMatch } = this.context.router + const routes = React.Children.toArray(children) + const parentPath = parentMatch && parentMatch.path ? parentMatch.path : '' let route, match for (let i = 0, length = routes.length; match == null && i < length; ++i) { route = routes[i] - match = matchPath(location.pathname, route.props.path, route.props) + match = matchPath( + location.pathname, + simpleResolve(route.props.path, parentPath), + route.props + ) } return match ? React.cloneElement(route, { computedMatch: match }) : null diff --git a/packages/react-router/modules/__tests__/Redirect-test.js b/packages/react-router/modules/__tests__/Redirect-test.js new file mode 100644 index 0000000000..a609e0c0bc --- /dev/null +++ b/packages/react-router/modules/__tests__/Redirect-test.js @@ -0,0 +1,34 @@ +import expect from 'expect' +import React from 'react' +import ReactDOM from 'react-dom' +import createMemoryHistory from 'history/createMemoryHistory' +import Router from '../Router' +import Redirect from '../Redirect' + +describe('Redirect', () => { + const createHistoryMock = () => { + const pushes = [] + const replaces = [] + const history = createMemoryHistory() + history.push = (loc) => pushes.push(loc) + history.replace = (loc) => replaces.push(loc) + history.getResults = () => ({ replaces, pushes }) + return history + } + + it('works with relative paths', () => { + const div = document.createElement('div') + const REDIRECTED = 'REDIRECTED' + const history = createHistoryMock() + ReactDOM.render(( + + + + ), div, () => { + const { pushes, replaces } = history.getResults() + expect(pushes.length).toEqual(1) + expect(pushes[0]).toEqual('/' + REDIRECTED) + expect(replaces.length).toEqual(0) + }) + }) +}) \ No newline at end of file diff --git a/packages/react-router/modules/__tests__/Route-test.js b/packages/react-router/modules/__tests__/Route-test.js index 7fac62affd..dd48b2ca95 100644 --- a/packages/react-router/modules/__tests__/Route-test.js +++ b/packages/react-router/modules/__tests__/Route-test.js @@ -115,4 +115,60 @@ describe('A ', () => { expect(node.innerHTML).toNotContain(TEXT) }) + + describe('relative paths', () => { + it('resolves using the parent match', () => { + const initialEntries = ['/', '/recipes', '/recipes/pizza'] + const TEXT = 'TEXT' + const node = document.createElement('div') + ReactDOM.render(( + + ( +
{TEXT}
} /> + )} /> + + ), node) + expect(node.textContent).toContain(TEXT) + }) + + it('works when match is null', () => { + const initialEntries = ['/', '/recipes'] + const TEXT = 'TEXT' + const node = document.createElement('div') + ReactDOM.render(( + +
{TEXT}
} /> + + ), node) + expect(node.textContent).toContain(TEXT) + }) + + it('works when path is empty string', () => { + const initialEntries = ['/hello'] + const TEXT = 'TEXT' + const node = document.createElement('div') + ReactDOM.render(( + + ( +
{TEXT}
} /> + )} /> + + ), node) + expect(node.textContent).toContain(TEXT) + }) + + it('works with pathless routes', () => { + const initialEntries = ['/', '/pizza'] + const TEXT = 'TEXT' + const node = document.createElement('div') + ReactDOM.render(( + + ( +
{TEXT}
} /> + )} /> + + ), node) + expect(node.textContent).toContain(TEXT) + }) + }) }) diff --git a/packages/react-router/modules/__tests__/Switch-test.js b/packages/react-router/modules/__tests__/Switch-test.js index 0cd60effe1..37c3c36737 100644 --- a/packages/react-router/modules/__tests__/Switch-test.js +++ b/packages/react-router/modules/__tests__/Switch-test.js @@ -59,4 +59,18 @@ describe('A ', () => { expect(node.innerHTML).toNotContain('one') expect(node.innerHTML).toContain('two') }) + + it('works with relative Routes', () => { + const initialEntries = ['/', '/recipes'] + const TEXT = 'TEXT' + const node = document.createElement('div') + ReactDOM.render(( + + +
{TEXT}
} /> + + + ), node) + expect(node.textContent).toContain(TEXT) + }) }) diff --git a/packages/react-router/modules/__tests__/resolve-test.js b/packages/react-router/modules/__tests__/resolve-test.js new file mode 100644 index 0000000000..0d90eee34c --- /dev/null +++ b/packages/react-router/modules/__tests__/resolve-test.js @@ -0,0 +1,152 @@ +import expect from 'expect' +import { resolveLocation, simpleResolve } from '../resolve' + +describe('resolve', () => { + + describe('resolveLocation', () => { + const BASE = '/a/b' + + describe('empty base', () => { + const cases = [ + ['', '/'], + ['recipes', '/recipes'], + ['/restaurants', '/restaurants'], + ['.', '/'], + ['..', '/'], + ['../', '/'] + ] + + it('works when base is empty string', () => { + const BASE = '' + cases.forEach(([input, output]) => { + expect(resolveLocation(input, BASE)).toEqual(output) + }) + }) + + it('works when base is null', () => { + const BASE = null + cases.forEach(([input, output]) => { + expect(resolveLocation(input, BASE)).toEqual(output) + }) + }) + + it('works when base is undefined', () => { + const BASE = undefined + cases.forEach(([input, output]) => { + expect(resolveLocation(input, BASE)).toEqual(output) + }) + }) + }) + + describe('string location', () => { + it('returns a string', () => { + const path = 'recipes' + expect(resolveLocation(path, BASE)).toBeA('string') + }) + + it('works for absolute pathnames', () => { + const path = '/recipes' + expect(resolveLocation(path, BASE)).toBe(path) + }) + + it('resolves to base when there is no pathname', () => { + expect(resolveLocation(undefined, BASE)).toEqual(BASE) + }) + }) + + describe('object location', () => { + it('returns an object with a pathname', () => { + const location = { pathname: 'recipes' } + const retLocation = resolveLocation(location) + expect(retLocation).toBeA('object') + expect(retLocation).toIncludeKey('pathname') + }) + + it('works for absolute pathname', () => { + const descriptor = { pathname: '/recipes' } + expect(resolveLocation(descriptor).pathname).toBe(descriptor.pathname) + }) + + it('resolves to base when there is no pathname', () => { + const descriptors = [ + { query: {a: 'b'} }, + { query: {} }, + { search: '?a=b' }, + { hash: '#recipes' } + ] + descriptors.forEach(d => { + expect(resolveLocation(d, BASE).pathname).toBe(BASE) + }) + }) + }) + + describe('slash strategy', () => { + it('preserves pathnames slash strategy', () => { + const cases = [ + ['recipes/', '/a/b/recipes/'], + ['recipes', '/a/b/recipes'], + ['../', '/a/'] + ] + cases.forEach(([input, output]) => { + expect(resolveLocation(input, BASE)).toEqual(output) + }) + }) + + it('removes trailing slash when pathname is removed by dot notation', () => { + const cases = [ + ['restaurants/..', '/a/b'], + ['restaurants/.', '/a/b/restaurants'] + ] + cases.forEach(([input, output]) => { + expect(resolveLocation(input, BASE)).toEqual(output) + }) + }) + }) + }) + + describe('simpleResolve', () => { + it('returns pathname if it is absolute', () => { + const pathname = '/recipes' + expect(simpleResolve(pathname, '/base')).toEqual(pathname) + }) + + it('joins pathname with the base', () => { + const pathname = 'recipes' + expect(simpleResolve(pathname, '/base')).toEqual('/base/recipes') + }) + + it('correctly joins when base has a trailing slash', () => { + const pathname = 'recipes' + expect(simpleResolve(pathname, '/base/')).toEqual('/base/recipes') + }) + + describe('empty base', () => { + const cases = [ + ['', '/'], + ['recipes', '/recipes'], + ['/restaurants', '/restaurants'] + ] + + it('works when base is empty string', () => { + const BASE = '' + cases.forEach(([input, output]) => { + expect(simpleResolve(input, BASE)).toEqual(output) + }) + }) + + it('works when base is null', () => { + const BASE = null + cases.forEach(([input, output]) => { + expect(simpleResolve(input, BASE)).toEqual(output) + }) + }) + + it('works when base is undefined', () => { + const BASE = undefined + cases.forEach(([input, output]) => { + expect(simpleResolve(input, BASE)).toEqual(output) + }) + }) + }) + }) +}) diff --git a/packages/react-router/modules/resolve.js b/packages/react-router/modules/resolve.js new file mode 100644 index 0000000000..e3b9c69226 --- /dev/null +++ b/packages/react-router/modules/resolve.js @@ -0,0 +1,60 @@ +import resolvePathname from 'resolve-pathname' + +const hasTrailingSlash = pathname => { + if (pathname == null) { + return false + } + return pathname.charAt(pathname.length-1) === '/' +} + +const withTrailingSlash = pathname => { + if (pathname == null) { + return '/' + } + return hasTrailingSlash(pathname) ? pathname : pathname + '/' +} + +const stripTrailingSlash = pathname => { + if (pathname == null) { + return '/' + } + return pathname.length > 1 && hasTrailingSlash(pathname) ? + pathname.slice(0, -1) : pathname +} + +const isAbsolute = pathname => !!(pathname && pathname.charAt(0) === '/') + +const sameSlashStrategy = (pathname, toSlash, fromSlash) => + !toSlash && !fromSlash ? stripTrailingSlash(pathname) : pathname + +export const resolveLocation = (location, base) => { + if (location === undefined) { + return base + } else if (typeof location === 'string') { + return resolve(location, base) + } else { + location.pathname = resolve(location.pathname, base) + return location + } +} + +const resolve = (to, from = '') => { + const toSlash = hasTrailingSlash(to) + const fromSlash = hasTrailingSlash(from) + const pathname = resolvePathname(to, withTrailingSlash(from)) + return sameSlashStrategy(pathname, toSlash, fromSlash) +} + +export const simpleResolve = (pathname, base) => { + if (base == null || base === '') { + base = '/' + } + + if (pathname === undefined || isAbsolute(pathname)) { + return pathname + } else if (pathname === '') { + return base + } else { + return `${withTrailingSlash(base)}${pathname}` + } +} diff --git a/packages/react-router/package.json b/packages/react-router/package.json index 14dd9ed79d..462bbd9913 100644 --- a/packages/react-router/package.json +++ b/packages/react-router/package.json @@ -19,6 +19,7 @@ "index.js", "matchPath.js", "withRouter.js", + "resolve.js", "README.md", "es", "umd" @@ -45,6 +46,7 @@ "invariant": "^2.2.2", "loose-envify": "^1.3.1", "path-to-regexp": "^1.5.3", + "resolve-pathname": "^2.0.2", "warning": "^3.0.0" }, "devDependencies": { From 04fd26a4b0e709557fbb6fd2807dbae1c36a8367 Mon Sep 17 00:00:00 2001 From: Paul Sherman Date: Fri, 3 Feb 2017 14:00:13 -0600 Subject: [PATCH 2/3] resolve Route/Switch in matchPath using match.url The parent match is passed to matchPath. match.url is used for resolving instead of math.path to simplify matching (just match a string instead of having to parse params). matchPath will combine its parent's params with its params (favoring its params when there are conflicts) --- packages/react-router/modules/Redirect.js | 2 +- packages/react-router/modules/Route.js | 6 +- packages/react-router/modules/Switch.js | 8 +- .../modules/__tests__/Route-test.js | 125 ++++++++++-------- .../modules/__tests__/Switch-test.js | 23 ++-- packages/react-router/modules/matchPath.js | 20 ++- 6 files changed, 104 insertions(+), 80 deletions(-) diff --git a/packages/react-router/modules/Redirect.js b/packages/react-router/modules/Redirect.js index 2544a10284..1280d23862 100644 --- a/packages/react-router/modules/Redirect.js +++ b/packages/react-router/modules/Redirect.js @@ -10,7 +10,7 @@ class Redirect extends React.Component { router: PropTypes.shape({ push: PropTypes.func.isRequired, replace: PropTypes.func.isRequired, - staticContext: PropTypes.object + staticContext: PropTypes.object, match: PropTypes.shape({ url: PropTypes.string }) diff --git a/packages/react-router/modules/Route.js b/packages/react-router/modules/Route.js index df1ec623f6..4ac54264f6 100644 --- a/packages/react-router/modules/Route.js +++ b/packages/react-router/modules/Route.js @@ -1,13 +1,13 @@ import warning from 'warning' import React, { PropTypes } from 'react' import matchPath from './matchPath' -import { simpleResolve } from './resolve' const computeMatch = (router, { computedMatch, path, exact, strict }) => computedMatch || matchPath( router.location.pathname, - simpleResolve(path, router.match && router.match.path ? router.match.path : ''), - { exact, strict } + path, + { exact, strict }, + router.match ) /** diff --git a/packages/react-router/modules/Switch.js b/packages/react-router/modules/Switch.js index 1564b047ff..643341357e 100644 --- a/packages/react-router/modules/Switch.js +++ b/packages/react-router/modules/Switch.js @@ -1,6 +1,5 @@ import React, { PropTypes } from 'react' import matchPath from './matchPath' -import { simpleResolve } from './resolve' /** * The public API for rendering the first that matches. @@ -46,15 +45,14 @@ class Switch extends React.Component { const { match:parentMatch } = this.context.router const routes = React.Children.toArray(children) - - const parentPath = parentMatch && parentMatch.path ? parentMatch.path : '' let route, match for (let i = 0, length = routes.length; match == null && i < length; ++i) { route = routes[i] match = matchPath( location.pathname, - simpleResolve(route.props.path, parentPath), - route.props + route.props.path, + route.props, + parentMatch ) } diff --git a/packages/react-router/modules/__tests__/Route-test.js b/packages/react-router/modules/__tests__/Route-test.js index dd48b2ca95..afb70e345b 100644 --- a/packages/react-router/modules/__tests__/Route-test.js +++ b/packages/react-router/modules/__tests__/Route-test.js @@ -116,59 +116,76 @@ describe('A ', () => { expect(node.innerHTML).toNotContain(TEXT) }) - describe('relative paths', () => { - it('resolves using the parent match', () => { - const initialEntries = ['/', '/recipes', '/recipes/pizza'] - const TEXT = 'TEXT' - const node = document.createElement('div') - ReactDOM.render(( - - ( -
{TEXT}
} /> - )} /> - - ), node) - expect(node.textContent).toContain(TEXT) - }) - - it('works when match is null', () => { - const initialEntries = ['/', '/recipes'] - const TEXT = 'TEXT' - const node = document.createElement('div') - ReactDOM.render(( - -
{TEXT}
} /> - - ), node) - expect(node.textContent).toContain(TEXT) - }) - - it('works when path is empty string', () => { - const initialEntries = ['/hello'] - const TEXT = 'TEXT' - const node = document.createElement('div') - ReactDOM.render(( - - ( -
{TEXT}
} /> - )} /> - - ), node) - expect(node.textContent).toContain(TEXT) - }) - - it('works with pathless routes', () => { - const initialEntries = ['/', '/pizza'] - const TEXT = 'TEXT' - const node = document.createElement('div') - ReactDOM.render(( - - ( -
{TEXT}
} /> - )} /> - - ), node) - expect(node.textContent).toContain(TEXT) - }) +}) + +describe('A with a relative path', () => { + it('resolves using the parent match', () => { + const initialEntries = ['/', '/recipes', '/recipes/pizza'] + const TEXT = 'TEXT' + const node = document.createElement('div') + ReactDOM.render(( + + ( +
{TEXT}
} /> + )} /> + + ), node) + expect(node.textContent).toContain(TEXT) + }) + + it('works when match is null', () => { + const initialEntries = ['/', '/recipes'] + const TEXT = 'TEXT' + const node = document.createElement('div') + ReactDOM.render(( + +
{TEXT}
} /> + + ), node) + expect(node.textContent).toContain(TEXT) + }) + + it('works when path is empty string', () => { + const initialEntries = ['/hello'] + const TEXT = 'TEXT' + const node = document.createElement('div') + ReactDOM.render(( + + ( +
{TEXT}
} /> + )} /> + + ), node) + expect(node.textContent).toContain(TEXT) + }) + + it('works with pathless routes', () => { + const initialEntries = ['/', '/pizza'] + const TEXT = 'TEXT' + const node = document.createElement('div') + ReactDOM.render(( + + ( +
{TEXT}
} /> + )} /> + + ), node) + expect(node.textContent).toContain(TEXT) + }) + + it('inherits params from its parent match', () => { + const FOOD = 'pizza' + const TOPPING = 'pineapple' + const initialEntries = [`/${FOOD}/${TOPPING}`] + const TEXT = 'TEXT' + const node = document.createElement('div') + ReactDOM.render(( + + ( +
{FOOD} {TOPPING}
} /> + )} /> + + ), node) + expect(node.textContent).toContain(`${FOOD} ${TOPPING}`) }) }) diff --git a/packages/react-router/modules/__tests__/Switch-test.js b/packages/react-router/modules/__tests__/Switch-test.js index 37c3c36737..f05f16b77d 100644 --- a/packages/react-router/modules/__tests__/Switch-test.js +++ b/packages/react-router/modules/__tests__/Switch-test.js @@ -61,16 +61,17 @@ describe('A ', () => { }) it('works with relative Routes', () => { - const initialEntries = ['/', '/recipes'] - const TEXT = 'TEXT' - const node = document.createElement('div') - ReactDOM.render(( - - -
{TEXT}
} /> - - - ), node) - expect(node.textContent).toContain(TEXT) + const node = document.createElement('div') + + const initialEntries = ['/recipes'] + const TEXT = 'TEXT' + ReactDOM.render(( + + +
{TEXT}
} /> + + + ), node) + expect(node.textContent).toContain(TEXT) }) }) diff --git a/packages/react-router/modules/matchPath.js b/packages/react-router/modules/matchPath.js index ccc1f6824a..db86cc1fbd 100644 --- a/packages/react-router/modules/matchPath.js +++ b/packages/react-router/modules/matchPath.js @@ -1,4 +1,5 @@ import pathToRegexp from 'path-to-regexp' +import { simpleResolve } from './resolve' const patternCache = {} const cacheLimit = 10000 @@ -26,11 +27,13 @@ const compilePath = (pattern, options) => { /** * Public API for matching a URL pathname to a path pattern. */ -const matchPath = (pathname, path, options = {}) => { +const matchPath = (pathname, path, options = {}, parent = null) => { const { exact = false, strict = false } = options if (!path) - return { url: pathname, isExact: true, params: {} } + return { url: parent ? parent.url : '/', isExact: true, params: {} } + + path = simpleResolve(path, parent && parent.url ? parent.url : '') const { re, keys } = compilePath(path, { end: exact, strict }) const match = re.exec(pathname) @@ -44,14 +47,19 @@ const matchPath = (pathname, path, options = {}) => { if (exact && !isExact) return null + const params = Object.assign({}, + parent && parent.params, + keys.reduce((memo, key, index) => { + memo[key.name] = values[index] + return memo + }, {}) + ) + return { path, // the path pattern used to match url: path === '/' && url === '' ? '/' : url, // the matched portion of the URL isExact, // whether or not we matched exactly - params: keys.reduce((memo, key, index) => { - memo[key.name] = values[index] - return memo - }, {}) + params } } From 95b8aeae85d69a89b02b4c991e1eddd4b38373ab Mon Sep 17 00:00:00 2001 From: Paul Sherman Date: Fri, 3 Feb 2017 17:47:46 -0600 Subject: [PATCH 3/3] can't use pathless component in test --- packages/react-router-dom/modules/Link.js | 3 +- .../modules/__tests__/Link-test.js | 151 +++++++++--------- 2 files changed, 76 insertions(+), 78 deletions(-) diff --git a/packages/react-router-dom/modules/Link.js b/packages/react-router-dom/modules/Link.js index 3bfd828d49..c0790bef3c 100644 --- a/packages/react-router-dom/modules/Link.js +++ b/packages/react-router-dom/modules/Link.js @@ -58,9 +58,8 @@ class Link extends React.Component { } absolutePathname() { - const { router } = this.context const { to } = this.props - const { match } = router + const { match } = this.context.router const base = (match && match.url) ? match.url : '' return resolveLocation(to, base) } diff --git a/packages/react-router-dom/modules/__tests__/Link-test.js b/packages/react-router-dom/modules/__tests__/Link-test.js index d990e30ada..d2dcf61a81 100644 --- a/packages/react-router-dom/modules/__tests__/Link-test.js +++ b/packages/react-router-dom/modules/__tests__/Link-test.js @@ -91,92 +91,91 @@ describe('A underneath a ', () => { expect(linkNode.getAttribute('href')).toEqual('#foo') }) }) +}) - describe('relative to', () => { - const node = document.createElement('div') +describe('A with a relative to', () => { + const node = document.createElement('div') - afterEach(() => { - ReactDOM.unmountComponentAtNode(node) - }) + afterEach(() => { + ReactDOM.unmountComponentAtNode(node) + }) - it('resolves using the parent match', () => { - const initialEntries = ['/', '/recipes'] - ReactDOM.render(( - - ( - Chess - )} /> - - ), node) - const a = node.getElementsByTagName('a')[0] - expect(a.pathname).toBe('/recipes/tacos') - }) + it('resolves using the parent match', () => { + const initialEntries = ['/', '/recipes'] + ReactDOM.render(( + + ( + Chess + )} /> + + ), node) + const a = node.getElementsByTagName('a')[0] + expect(a.pathname).toBe('/recipes/tacos') + }) - it('works when not in a route', () => { - const initialEntries = ['/'] - ReactDOM.render(( - - Recipes - - ), node) - const a = node.getElementsByTagName('a')[0] - expect(a.pathname).toBe('/recipes') - }) + it('works when not in a route', () => { + const initialEntries = ['/'] + ReactDOM.render(( + + Recipes + + ), node) + const a = node.getElementsByTagName('a')[0] + expect(a.pathname).toBe('/recipes') + }) - it('navigates correctly', () => { - const initialEntries = ['/', '/recipes'] - const RESTAURANTS = 'RESTAURANTS' - ReactDOM.render(( - - - ( - Order Takeout - )} /> - ( -
{RESTAURANTS}
- )} /> - - - ), node) - expect(node.textContent).toNotContain(RESTAURANTS) - const a = node.getElementsByTagName('a')[0] - Simulate.click(a, { - defaultPrevented: false, - preventDefault() { this.defaultPrevented = true }, - metaKey: null, - altKey: null, - ctrlKey: null, - shiftKey: null, - button: 0 - }) - expect(node.textContent).toContain(RESTAURANTS) + it('navigates correctly', () => { + const initialEntries = ['/', '/recipes'] + const RESTAURANTS = 'RESTAURANTS' + ReactDOM.render(( + + + ( + Order Takeout + )} /> + ( +
{RESTAURANTS}
+ )} /> + + + ), node) + expect(node.textContent).toNotContain(RESTAURANTS) + const a = node.getElementsByTagName('a')[0] + Simulate.click(a, { + defaultPrevented: false, + preventDefault() { this.defaultPrevented = true }, + metaKey: null, + altKey: null, + ctrlKey: null, + shiftKey: null, + button: 0 }) + expect(node.textContent).toContain(RESTAURANTS) + }) - it('updates regardless of sCU blocks', () => { - let goForward - class UpdateBlocker extends React.Component { - shouldComponentUpdate() { - return false - } - render() { - goForward = this.props.goForward - return - } + it('updates regardless of sCU blocks', () => { + let goForward + class UpdateBlocker extends React.Component { + shouldComponentUpdate() { + return false } + render() { + goForward = this.props.goForward + return + } + } - ReactDOM.render(( - - - - ), node) - - let href = node.querySelector('a').getAttribute('href') - expect(href).toEqual('/bubblegum/store') + ReactDOM.render(( + + + + ), node) - goForward() + let href = node.querySelector('a').getAttribute('href') + expect(href).toEqual('/bubblegum/store') + goForward() - href = node.querySelector('a').getAttribute('href') - expect(href).toEqual('/shoelaces/store') - }) + href = node.querySelector('a').getAttribute('href') + expect(href).toEqual('/shoelaces/store') }) })