Use trailing slashes for all routes (#660)

Summary:
See justification in the added unit test.

Test Plan:
Added unit test, with justification.

Also, `yarn sharness-full` passes, and `yarn start` still works.

wchargin-branch: route-trailing-slashes
This commit is contained in:
William Chargin 2018-08-13 21:25:20 -07:00 committed by GitHub
parent 080edb380d
commit 4081e0d008
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
2 changed files with 46 additions and 2 deletions

View File

@ -28,7 +28,7 @@ const routeData /*: $ReadOnlyArray<RouteDatum> */ = [
navTitle: "Home", navTitle: "Home",
}, },
{ {
path: "/prototype", path: "/prototype/",
contents: { contents: {
type: "PAGE", type: "PAGE",
component: () => require("./credExplorer/App").default, component: () => require("./credExplorer/App").default,
@ -37,7 +37,7 @@ const routeData /*: $ReadOnlyArray<RouteDatum> */ = [
navTitle: "Prototype", navTitle: "Prototype",
}, },
{ {
path: "/discord-invite", path: "/discord-invite/",
contents: { contents: {
type: "EXTERNAL_REDIRECT", type: "EXTERNAL_REDIRECT",
redirectTo: "https://discord.gg/tsBTgc9", redirectTo: "https://discord.gg/tsBTgc9",

44
src/app/routeData.test.js Normal file
View File

@ -0,0 +1,44 @@
// @flow
import {routeData} from "./routeData";
describe("app/routeData", () => {
/*
* React Router doesn't support relative paths. I'm not sure exactly
* what a path without a leading slash would do; it's asking for
* trouble. If we need them, we can reconsider this test.
*/
it("every path has a leading slash", () => {
for (const route of routeData) {
if (!route.path.startsWith("/")) {
expect(route.path).toEqual("/" + route.path);
}
}
});
/*
* A route representing a page should have a trailing slash so that
* relative links work in the expected way. For instance, a route
* "/about/team/" may reference "/about/logo.png" via "../logo.png".
* But for the route "/about/team", "../logo.png" refers instead to
* "/logo.png", which is not the intended semantics. Therefore, we
* should consistently either include or omit trailing slashes to
* avoid confusion.
*
* The choice is made for us by the fact that many web servers
* (prominently, GitHub Pages and Python's SimpleHTTPServer) redirect
* "/foo" to "/foo/" when serving "/foo/index.html".
*
* In theory, we might have some file routes like "/about/data.csv"
* that we actually want to appear without a trailing slash. But those
* are outside the scope of our React application, and should be
* handled by a different pipeline (e.g., `copy-webpack-plugin`).
*/
it("every path has a trailing slash", () => {
for (const route of routeData) {
if (!route.path.endsWith("/")) {
expect(route.path).toEqual(route.path + "/");
}
}
});
});