diff --git a/docs/page-api.md b/docs/page-api.md index e84f4b3a7..e86ec3fc9 100644 --- a/docs/page-api.md +++ b/docs/page-api.md @@ -6,11 +6,13 @@ A binding of URL paths to a Page class that can render the results for those URL * A string or regex that will be tested against the path of the URL to determine if this route applies. -`method, optional: String | [String]` +`method, optional: undefined | String | [String]` -* The HTTP method(s) that are acceptable. +* The HTTP method(s) that are acceptable. Use an array of strings to specify multiple methods. Do not + set this value to `null` or `[]`. To match all HTTP methods, either leave out the parameter altogether or explicitly + set the value to `undefined`. -* Default: “get” +* Default: undefined `page: Page` * The page to render diff --git a/packages/react-server-cli/src/compileClient.js b/packages/react-server-cli/src/compileClient.js index e9c72fd55..a915015a9 100755 --- a/packages/react-server-cli/src/compileClient.js +++ b/packages/react-server-cli/src/compileClient.js @@ -291,14 +291,20 @@ module.exports = { for (let routeName of Object.keys(routes.routes)) { let route = routes.routes[routeName]; + // On the line below specifying 'method', if the route doesn't have a method, we'll set it to `undefined` so that + // routr passes through and matches any method + // https://github.com/yahoo/routr/blob/v2.1.0/lib/router.js#L49-L57 + let method = route.method; + + // Safely check for an empty object, array, or string and specifically set it to 'undefined' + if (method === undefined || method === null || method === {} || method.length === 0) { + method = undefined; // 'undefined' is the value that routr needs to accept any method + } + routesOutput.push(` - ${routeName}: {`); - routesOutput.push(` - path: ${JSON.stringify(route.path)},`); - // if the route doesn't have a method, we'll assume it's "get". routr doesn't - // have a default (method is required), so we set it here. - routesOutput.push(` - method: "${route.method || "get"}",`); + ${routeName}: { + path: ${JSON.stringify(route.path)}, + method: ${JSON.stringify(method)},`); let formats = normalizeRoutesPage(route.page); routesOutput.push(` diff --git a/packages/react-server/core/__tests__/context/navigator/DumbPage.js b/packages/react-server/core/__tests__/context/navigator/DumbPage.js new file mode 100644 index 000000000..a3180b19b --- /dev/null +++ b/packages/react-server/core/__tests__/context/navigator/DumbPage.js @@ -0,0 +1,6 @@ +export default class DumbPage { + getElements() { + const routeName = this.getRequest().getRouteName(); + return
{routeName}
; + } +} diff --git a/packages/react-server/core/__tests__/context/navigator/NavigatorRoutes.js b/packages/react-server/core/__tests__/context/navigator/NavigatorRoutes.js new file mode 100644 index 000000000..75a6effe7 --- /dev/null +++ b/packages/react-server/core/__tests__/context/navigator/NavigatorRoutes.js @@ -0,0 +1,66 @@ +import DumbPage from "./DumbPage"; + +// This wrapper is taken from the compileClient function that writes the routes_(client|server).js file +const pageWrapper = { + default: () => { + return { + done: (cb) => { cb(DumbPage); }, + }; + }, +}; + +const Routes = { + "GetPage": { + "path": ["/getPage"], + "page": pageWrapper, + "method": "get", + }, + "HeadPage": { + "path": ["/headPage"], + "page": pageWrapper, + "method": "head", + }, + "PostPage": { + "path": ["/postPage"], + "page": pageWrapper, + "method": "post", + }, + "PatchPage": { + "path": ["/patchPage"], + "page": pageWrapper, + "method": "patch", + }, + "PutPage": { + "path": ["/putPage"], + "page": pageWrapper, + "method": "put", + }, + "GetPageCaps": { + "path": ["/getPageCaps"], + "page": pageWrapper, + "method": "GET", // this should be all caps because the req.getMethod() will return 'get'. + }, + "GetAndPostPage": { + "path": ["/getAndPostPage"], + "page": pageWrapper, + "method": ["get", "post"], + }, + "AllMethodsPage": { + "path": ["/allMethodsPage"], + "page": pageWrapper, + "method": undefined, + }, + "NullMethodsPage": { + "path": ["/nullMethodsPage"], + "page": pageWrapper, + "method": null, + }, + "EmptyArrayMethodsPage": { + "path": ["/emptyArrayMethodsPage"], + "page": pageWrapper, + "method": [], + }, + +}; + +export default Routes; diff --git a/packages/react-server/core/__tests__/context/navigator/NavigatorSpec.js b/packages/react-server/core/__tests__/context/navigator/NavigatorSpec.js new file mode 100644 index 000000000..9670a9e86 --- /dev/null +++ b/packages/react-server/core/__tests__/context/navigator/NavigatorSpec.js @@ -0,0 +1,202 @@ +import Navigator from "../../../context/Navigator"; +import History from "../../../components/History"; +import ExpressServerRequest from "../../../ExpressServerRequest"; +import NavigatorRoutes from "./NavigatorRoutes"; + +class RequestContextStub { + constructor(options) { + this.navigator = new Navigator(this, options); + } + navigate (request, type) { + this.navigator.navigate(request, type); + } + framebackControllerWillHandle() { return false; } + getMobileDetect() { return null; } +} + + +describe("Navigator", () => { + let requestContext; + const options = { + routes: NavigatorRoutes, + }; + + beforeEach(() => { + requestContext = new RequestContextStub(options); + }); + afterEach(() => { + requestContext = null; + }); + + it("routes to a page using a method GET (caps)", (done) => { + const req = { + method: "get", + protocol: "http", + secure: false, + hostname: "localhost", + url: "/getPageCaps", + }; + const expressRequest = new ExpressServerRequest(req); + + const navigatedToPage = () => { + expect(expressRequest.getRouteName()).toBe('GetPageCaps'); + done(); + }; + requestContext.navigator.on('navigateDone', navigatedToPage); + requestContext.navigator.on('page', navigatedToPage); + + requestContext.navigate(expressRequest, History.events.PAGELOAD); + }); + + it("routes to a page that can handle GET and POST using a method GET", (done) => { + const req = { + method: "get", + protocol: "http", + secure: false, + hostname: "localhost", + url: "/getAndPostPage", + }; + const expressRequest = new ExpressServerRequest(req); + + const navigatedToPage = () => { + expect(expressRequest.getRouteName()).toBe('GetAndPostPage'); + done(); + }; + requestContext.navigator.on('navigateDone', navigatedToPage); + requestContext.navigator.on('page', navigatedToPage); + + requestContext.navigate(expressRequest, History.events.PAGELOAD); + }); + + it("routes to a page that can handle GET and POST using a method POST", (done) => { + const req = { + method: "post", + protocol: "http", + secure: false, + hostname: "localhost", + url: "/getAndPostPage", + }; + const expressRequest = new ExpressServerRequest(req); + + const navigatedToPage = () => { + expect(expressRequest.getRouteName()).toBe('GetAndPostPage'); + done(); + }; + requestContext.navigator.on('navigateDone', navigatedToPage); + requestContext.navigator.on('page', navigatedToPage); + + requestContext.navigate(expressRequest, History.events.PAGELOAD); + }); + + + const methods = [ + 'get', + 'head', + 'put', + 'patch', + 'post', + ]; + + methods.forEach((testingMethod) => { + methods.forEach((otherMethod) => { + const req = { + method: otherMethod, + protocol: "http", + secure: false, + hostname: "localhost", + url: `/${testingMethod}Page`, + }; + const expressRequest = new ExpressServerRequest(req); + + if (testingMethod === otherMethod) { + it(`does route to a page expecting ${otherMethod} using a method ${testingMethod}`, (done) => { + const navigatedToPage = (err) => { + const httpStatus = err.status || 200; + expect(httpStatus).toBe(200, `A route with method ${testingMethod} was not found when one should have been found.`); + done(); + }; + requestContext.navigator.on('navigateDone', navigatedToPage); + requestContext.navigator.on('page', navigatedToPage); + requestContext.navigate(expressRequest, History.events.PAGELOAD); + }); + + it("routes to a page that accepts all methods", (done) => { + const allMethodReq = { + method: otherMethod, + protocol: "http", + secure: false, + hostname: "localhost", + url: "/allMethodsPage", + }; + const allMethodExpressRequest = new ExpressServerRequest(allMethodReq); + + const navigatedToPage = (err) => { + const httpStatus = err.status || 200; + expect(httpStatus).toBe(200, `A route with method ${testingMethod} was not found for the AllMethodsPage.`); + expect(allMethodExpressRequest.getRouteName()).toBe('AllMethodsPage'); + done(); + }; + requestContext.navigator.on('navigateDone', navigatedToPage); + requestContext.navigator.on('page', navigatedToPage); + + requestContext.navigate(allMethodExpressRequest, History.events.PAGELOAD); + }); + + it("doesn't route to a page because the methods were set improperly with a 'null'", (done) => { + const noMethodReq = { + method: otherMethod, + protocol: "http", + secure: false, + hostname: "localhost", + url: "/nullMethodsPage", + }; + const noMethodExpressRequest = new ExpressServerRequest(noMethodReq); + + const navigatedToPage = (err) => { + const httpStatus = err.status || 200; + expect(httpStatus).toBe(404, `A route with method ${testingMethod} was found for the NullMethodsPage.`); + done(); + }; + requestContext.navigator.on('navigateDone', navigatedToPage); + requestContext.navigator.on('page', navigatedToPage); + + requestContext.navigate(noMethodExpressRequest, History.events.PAGELOAD); + }); + + it("doesn't route to a page because the methods were set improperly with a '[]'", (done) => { + const noMethodReq = { + method: otherMethod, + protocol: "http", + secure: false, + hostname: "localhost", + url: "/emptyArrayMethodsPage", + }; + const noMethodExpressRequest = new ExpressServerRequest(noMethodReq); + + const navigatedToPage = (err) => { + const httpStatus = err.status || 200; + expect(httpStatus).toBe(404, `A route with method ${testingMethod} was found for the EmptyArrayMethodsPage.`); + done(); + }; + requestContext.navigator.on('navigateDone', navigatedToPage); + requestContext.navigator.on('page', navigatedToPage); + + requestContext.navigate(noMethodExpressRequest, History.events.PAGELOAD); + }); + } else { + it(`doesn't route to a page expecting ${otherMethod} using a method ${testingMethod}`, (done) => { + const navigatedToPage = (err) => { + const httpStatus = err.status || 200; + expect(httpStatus).toBe(404, `A route with method ${testingMethod} was found when one should not have been found.`); + done(); + }; + requestContext.navigator.on('navigateDone', navigatedToPage); + requestContext.navigator.on('page', navigatedToPage); + requestContext.navigate(expressRequest, History.events.PAGELOAD); + }); + } + }); + + }); + +}); diff --git a/packages/react-server/core/context/Navigator.js b/packages/react-server/core/context/Navigator.js index c32b42fdc..f215b2583 100644 --- a/packages/react-server/core/context/Navigator.js +++ b/packages/react-server/core/context/Navigator.js @@ -50,7 +50,7 @@ class Navigator extends EventEmitter { this._haveInitialized = true; - var route = this.router.getRoute(request.getUrl(), {navigate: {path:request.getUrl(), type:type}}); + var route = this.router.getRoute(request.getUrl(), { method: request.getMethod() }); if (route) { logger.debug(`Mapped ${request.getUrl()} to route ${route.name}`);