Skip to content

Commit

Permalink
feat(catch-all): handled mapping the catch-all route to a 404 status
Browse files Browse the repository at this point in the history
  • Loading branch information
travi committed Dec 19, 2016
1 parent c48d04b commit 50d4d38
Show file tree
Hide file tree
Showing 12 changed files with 70 additions and 22 deletions.
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ export default {
<IndexRoute component={Index}/>
<Route path="/foo" component={Foo}/>
<Route path="/bar" component={Bar}/>
<Route path="*" component={NotFound}/>
</Route>
),
Root: ({store, children}) => (
Expand All @@ -69,6 +70,10 @@ are currently not provided, so these dependencies are required.
additional steps before the response
* `routes`: the definition of your react-router routes that this plugin should match the request url
against
* If you use a [catch-all route](https://github.com/ReactTraining/react-router/blob/c3cd9675bd8a31368f87da74ac588981cbd6eae7/upgrade-guides/v1.0.0.md#notfound-route)
to display an appropriate message when the route does not match, it should have a `displayName` of `NotFound`. This
will enable the status code to be passed to `respond` as `404`. Please note that the automatic mapping of the `name`
property should not be relied on because it can be mangled during minification and, therefore, not match in production.
* `Root`: a react component that will wrap the mounted components that result from the matched route
* `store`: a data store that will be passed as a prop to the `<Root />` component so that your
component can inject it into the context through a provider component.
Expand Down
12 changes: 12 additions & 0 deletions example/components/not-found.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
import React from 'react';

export default function NotFound() {
return (
<div>
<h1>404</h1>
<p>Page Not Found</p>
</div>
);
}

NotFound.displayName = 'NotFound';
2 changes: 1 addition & 1 deletion example/manifest.js
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ export default {
{
plugin: {
register: '../src/route',
options: {respond, routes, Root, store: createStore(() => undefined)}
options: {respond, routes, Root, configureStore: () => createStore(() => undefined)}
}
}
]
Expand Down
4 changes: 2 additions & 2 deletions example/respond.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
export default function respond(reply, {renderedContent}) {
export default function respond(reply, {renderedContent, status}) {
reply.view('layout', {
renderedContent,
title: '<title>Example Title</title>'
});
}).code(status);
}
6 changes: 3 additions & 3 deletions example/routes.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,13 @@ import Wrap from './components/wrap';
import Index from './components/index';
import Foo from './components/foo';
import Bar from './components/bar';
import NotFound from './components/not-found';

const routes = (
export default (
<Route path="/" component={Wrap}>
<IndexRoute component={Index} />
<Route path="/foo" component={Foo} />
<Route path="/bar" component={Bar} />
<Route path="*" component={NotFound} />
</Route>
);

export default routes;
3 changes: 3 additions & 0 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -94,5 +94,8 @@
"commitizen": {
"path": "./node_modules/cz-conventional-changelog"
}
},
"dependencies": {
"http-status-codes": "1.0.6"
}
}
4 changes: 2 additions & 2 deletions src/data-fetcher.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import {trigger} from 'redial';

export default function ({renderProps, store}) {
export default function ({renderProps, store, status}) {
return trigger('fetch', renderProps.components, {
params: renderProps.params,
dispatch: store.dispatch,
state: store.getState()
}).then(() => Promise.resolve(({renderProps}))).catch(e => Promise.reject(e));
}).then(() => Promise.resolve(({renderProps, status}))).catch(e => Promise.reject(e));
}
9 changes: 8 additions & 1 deletion src/route-matcher.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,12 @@
import {OK, NOT_FOUND} from 'http-status-codes';
import {match, createMemoryHistory} from 'react-router';

function determineStatusFrom(components) {
if (components.map(component => component.displayName).includes('NotFound')) return NOT_FOUND;

return OK;
}

export default function matchRoute(url, routes) {
return new Promise((resolve, reject) => {
const history = createMemoryHistory();
Expand All @@ -9,7 +16,7 @@ export default function matchRoute(url, routes) {
reject(err);
}

resolve({redirectLocation, renderProps});
resolve({redirectLocation, renderProps, status: determineStatusFrom(renderProps.components)});
});
});
}
5 changes: 3 additions & 2 deletions src/router-wrapper.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,10 @@ import fetchData from './data-fetcher';

export default function renderThroughReactRouter(request, reply, {routes, respond, Root, store}) {
return matchRoute(request.raw.req.url, routes)
.then(({renderProps}) => fetchData({renderProps, store}))
.then(({renderProps}) => respond(reply, {
.then(({renderProps, status}) => fetchData({renderProps, store, status}))
.then(({renderProps, status}) => respond(reply, {
store,
status,
renderedContent: renderToString(
<Root request={request} store={store}>
<RouterContext {...renderProps} />
Expand Down
3 changes: 2 additions & 1 deletion test/unit/data-fetcher-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -22,10 +22,11 @@ suite('data fetcher', () => {
const state = any.simpleObject();
const getState = sinon.stub().returns(state);
const renderProps = {...any.simpleObject(), components, params};
const status = any.integer();
const store = {...any.simpleObject(), dispatch, getState};
redial.trigger.withArgs('fetch', components, {params, dispatch, state}).resolves();

return assert.isFulfilled(fetchData({renderProps, store}), {renderProps});
return assert.isFulfilled(fetchData({renderProps, store, status}), {renderProps, status});
});

test('that a redial rejection bubbles', () => {
Expand Down
32 changes: 25 additions & 7 deletions test/unit/route-matcher-test.js
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
import {OK, NOT_FOUND} from 'http-status-codes';
import * as reactRouter from 'react-router';
import sinon from 'sinon';
import {assert} from 'chai';
Expand All @@ -7,6 +8,10 @@ import matchRoute from '../../src/route-matcher';
suite('route matcher', () => {
let sandbox;
const createLocation = sinon.stub();
const routes = any.simpleObject();
const redirectLocation = any.string();
const renderProps = any.simpleObject();
const url = any.string();

setup(() => {
sandbox = sinon.sandbox.create();
Expand All @@ -21,15 +26,16 @@ suite('route matcher', () => {
});

test('that renderProps and redirectLocation are returned when matching resolves', () => {
const url = any.string();
const location = any.string();
const routes = any.simpleObject();
const renderProps = any.simpleObject();
const redirectLocation = any.string();
createLocation.withArgs(url).returns(location);
reactRouter.match.withArgs({location, routes}).yields(null, redirectLocation, renderProps);

return assert.becomes(matchRoute(url, routes), {redirectLocation, renderProps});
const renderPropWithComponents = {...renderProps, components: any.listOf(() => ({displayName: any.word()}))};
reactRouter.match.withArgs({location, routes}).yields(null, redirectLocation, renderPropWithComponents);

return assert.becomes(matchRoute(url, routes), {
redirectLocation,
renderProps: renderPropWithComponents,
status: OK
});
});

test('that a matching error results in a rejection', () => {
Expand All @@ -38,4 +44,16 @@ suite('route matcher', () => {

return assert.isRejected(matchRoute(), error);
});

test('that the status code is returned as 404 when the catch-all route matches', () => {
const components = [{displayName: any.string()}, {displayName: 'NotFound'}, {displayName: any.string()}];
const renderPropsWithComponents = {components};
reactRouter.match.yields(null, redirectLocation, renderPropsWithComponents);

return assert.becomes(matchRoute(url, routes), {
redirectLocation,
renderProps: renderPropsWithComponents,
status: NOT_FOUND
});
});
});
7 changes: 4 additions & 3 deletions test/unit/router-wrapper-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -31,20 +31,21 @@ suite('router-wrapper', () => {
const request = {raw: {req: {url}}};
const reply = sinon.spy();
const renderProps = any.simpleObject();
const status = any.integer();
const context = any.simpleObject();
const Root = any.simpleObject();
const store = any.simpleObject();
const rootComponent = any.simpleObject();
const renderedContent = any.string();
routeMatcher.default.withArgs(url, routes).resolves({renderProps});
dataFetcher.default.withArgs({renderProps, store}).resolves({renderProps});
routeMatcher.default.withArgs(url, routes).resolves({renderProps, status});
dataFetcher.default.withArgs({renderProps, store, status}).resolves({renderProps, status});
React.createElement.withArgs(RouterContext, sinon.match(renderProps)).returns(context);
React.createElement.withArgs(Root, {request, store}).returns(rootComponent);
domServer.renderToString.withArgs(rootComponent).returns(renderedContent);

return renderThroughReactRouter(request, reply, {routes, respond, Root, store}).then(() => {
assert.notCalled(reply);
assert.calledWith(respond, reply, {renderedContent, store});
assert.calledWith(respond, reply, {renderedContent, store, status});
});
});

Expand Down

0 comments on commit 50d4d38

Please sign in to comment.