From 4081e0d00880fc272008d96148ea453b067e316a Mon Sep 17 00:00:00 2001
From: William Chargin <wchargin@gmail.com>
Date: Mon, 13 Aug 2018 21:25:20 -0700
Subject: [PATCH] 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
---
 src/app/routeData.js      |  4 ++--
 src/app/routeData.test.js | 44 +++++++++++++++++++++++++++++++++++++++
 2 files changed, 46 insertions(+), 2 deletions(-)
 create mode 100644 src/app/routeData.test.js

diff --git a/src/app/routeData.js b/src/app/routeData.js
index 9b31cfa..e40b4d3 100644
--- a/src/app/routeData.js
+++ b/src/app/routeData.js
@@ -28,7 +28,7 @@ const routeData /*: $ReadOnlyArray<RouteDatum> */ = [
     navTitle: "Home",
   },
   {
-    path: "/prototype",
+    path: "/prototype/",
     contents: {
       type: "PAGE",
       component: () => require("./credExplorer/App").default,
@@ -37,7 +37,7 @@ const routeData /*: $ReadOnlyArray<RouteDatum> */ = [
     navTitle: "Prototype",
   },
   {
-    path: "/discord-invite",
+    path: "/discord-invite/",
     contents: {
       type: "EXTERNAL_REDIRECT",
       redirectTo: "https://discord.gg/tsBTgc9",
diff --git a/src/app/routeData.test.js b/src/app/routeData.test.js
new file mode 100644
index 0000000..8e3628b
--- /dev/null
+++ b/src/app/routeData.test.js
@@ -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 + "/");
+      }
+    }
+  });
+});