From ff9d603ffe59d667e1666f57906e4667ead85012 Mon Sep 17 00:00:00 2001
From: Michael Jackson <michael@jackson.us>
Date: Wed, 1 Sep 2021 15:46:17 -0700
Subject: [PATCH] Allow nested absolute paths

- Nested routes with absolute paths that do not match their parent
  path are now an error
- Also adds <Route index> prop for index routes

Fixes #7335
---
 packages/react-router-dom/index.tsx           |  1 -
 .../__tests__/absolute-path-matching-test.tsx |  4 +
 .../__tests__/index-routes-test.tsx           |  3 +
 .../__tests__/path-matching-test.tsx          | 10 +-
 .../route-depth-order-matching-test.tsx       | 51 ++++++++++
 .../__tests__/route-matching-test.tsx         |  9 +-
 .../react-router/__tests__/useRoutes-test.tsx |  4 +-
 packages/react-router/index.tsx               | 96 +++++++++++++------
 8 files changed, 137 insertions(+), 41 deletions(-)
 create mode 100644 packages/react-router/__tests__/absolute-path-matching-test.tsx
 create mode 100644 packages/react-router/__tests__/index-routes-test.tsx

diff --git a/packages/react-router-dom/index.tsx b/packages/react-router-dom/index.tsx
index bc05752fe4..8261c90cfb 100644
--- a/packages/react-router-dom/index.tsx
+++ b/packages/react-router-dom/index.tsx
@@ -81,7 +81,6 @@ export type {
   Navigator,
   OutletProps,
   Params,
-  PartialRouteObject,
   PathMatch,
   RouteMatch,
   RouteObject,
diff --git a/packages/react-router/__tests__/absolute-path-matching-test.tsx b/packages/react-router/__tests__/absolute-path-matching-test.tsx
new file mode 100644
index 0000000000..a3c4a21e70
--- /dev/null
+++ b/packages/react-router/__tests__/absolute-path-matching-test.tsx
@@ -0,0 +1,4 @@
+describe("absolute paths", () => {
+  it.todo("matches when the path matches");
+  it.todo("throws when the nested path does not begin with its parent path");
+});
diff --git a/packages/react-router/__tests__/index-routes-test.tsx b/packages/react-router/__tests__/index-routes-test.tsx
new file mode 100644
index 0000000000..256a6780e0
--- /dev/null
+++ b/packages/react-router/__tests__/index-routes-test.tsx
@@ -0,0 +1,3 @@
+describe("index routes", () => {
+  it.todo("throws when the index route has children");
+});
diff --git a/packages/react-router/__tests__/path-matching-test.tsx b/packages/react-router/__tests__/path-matching-test.tsx
index 97a04515fd..7e11df17a0 100644
--- a/packages/react-router/__tests__/path-matching-test.tsx
+++ b/packages/react-router/__tests__/path-matching-test.tsx
@@ -1,5 +1,5 @@
-import { matchRoutes } from "react-router";
 import type { PartialRouteObject } from "react-router";
+import { matchRoutes } from "react-router";
 
 describe("path matching", () => {
   function pickPaths(routes: PartialRouteObject[], pathname: string) {
@@ -71,7 +71,7 @@ describe("path matching", () => {
             children: [{ path: "subjects" }]
           },
           { path: "new" },
-          { path: "/" },
+          { index: true },
           { path: "*" }
         ]
       },
@@ -87,7 +87,7 @@ describe("path matching", () => {
       { path: "*" }
     ];
 
-    expect(pickPaths(routes, "/courses")).toEqual(["courses", "/"]);
+    expect(pickPaths(routes, "/courses")).toEqual(["courses", ""]);
     expect(pickPaths(routes, "/courses/routing")).toEqual(["courses", ":id"]);
     expect(pickPaths(routes, "/courses/routing/subjects")).toEqual([
       "courses",
@@ -115,7 +115,7 @@ describe("path matching", () => {
     let routes = [
       {
         path: ":page",
-        children: [{ path: "/" }]
+        children: [{ index: true }]
       },
       { path: "page" }
     ];
@@ -125,7 +125,7 @@ describe("path matching", () => {
 });
 
 describe("path matching with a basename", () => {
-  let routes = [
+  let routes: PartialRouteObject[] = [
     {
       path: "/users/:userId",
       children: [
diff --git a/packages/react-router/__tests__/route-depth-order-matching-test.tsx b/packages/react-router/__tests__/route-depth-order-matching-test.tsx
index 4b8f13cc17..7bfc6f60ef 100644
--- a/packages/react-router/__tests__/route-depth-order-matching-test.tsx
+++ b/packages/react-router/__tests__/route-depth-order-matching-test.tsx
@@ -3,6 +3,57 @@ import { act, create as createTestRenderer } from "react-test-renderer";
 import { MemoryRouter as Router, Outlet, Routes, Route } from "react-router";
 import type { ReactTestRenderer } from "react-test-renderer";
 
+describe("nested routes with no path", () => {
+  it("matches them depth-first", () => {
+    let renderer!: ReactTestRenderer;
+    act(() => {
+      renderer = createTestRenderer(
+        <Router initialEntries={["/"]}>
+          <Routes>
+            <Route element={<First />}>
+              <Route element={<Second />}>
+                <Route element={<Third />} />
+              </Route>
+            </Route>
+          </Routes>
+        </Router>
+      );
+    });
+
+    expect(renderer.toJSON()).toMatchInlineSnapshot(`
+      <div>
+        First 
+        <div>
+          Second 
+          <div>
+            Third
+          </div>
+        </div>
+      </div>
+    `);
+  });
+
+  function First() {
+    return (
+      <div>
+        First <Outlet />
+      </div>
+    );
+  }
+
+  function Second() {
+    return (
+      <div>
+        Second <Outlet />
+      </div>
+    );
+  }
+
+  function Third() {
+    return <div>Third</div>;
+  }
+});
+
 describe("nested /", () => {
   it("matches them depth-first", () => {
     let renderer!: ReactTestRenderer;
diff --git a/packages/react-router/__tests__/route-matching-test.tsx b/packages/react-router/__tests__/route-matching-test.tsx
index 3f5851435b..cfae042b92 100644
--- a/packages/react-router/__tests__/route-matching-test.tsx
+++ b/packages/react-router/__tests__/route-matching-test.tsx
@@ -1,5 +1,6 @@
 import * as React from "react";
 import { create as createTestRenderer } from "react-test-renderer";
+import type { PartialRouteObject } from "react-router";
 import {
   MemoryRouter as Router,
   Outlet,
@@ -8,7 +9,6 @@ import {
   useParams,
   useRoutes
 } from "react-router";
-import type { PartialRouteObject } from "react-router";
 import type { InitialEntry } from "history";
 
 describe("route matching", () => {
@@ -28,7 +28,7 @@ describe("route matching", () => {
             children: [{ path: "grades", element: <CourseGrades /> }]
           },
           { path: "new", element: <NewCourse /> },
-          { path: "/", element: <CoursesIndex /> },
+          { index: true, element: <CoursesIndex /> },
           { path: "*", element: <CoursesNotFound /> }
         ]
       },
@@ -56,7 +56,7 @@ describe("route matching", () => {
             <Route path="grades" element={<CourseGrades />} />
           </Route>
           <Route path="new" element={<NewCourse />} />
-          <Route path="/" element={<CoursesIndex />} />
+          <Route index element={<CoursesIndex />} />
           <Route path="*" element={<CoursesNotFound />} />
         </Route>
         <Route path="courses" element={<Landing />}>
@@ -80,7 +80,7 @@ describe("route matching", () => {
             <CourseGrades path="grades" />
           </Course>
           <NewCourse path="new" />
-          <CoursesIndex path="/" />
+          <CoursesIndex index />
           <CoursesNotFound path="*" />
         </Courses>
         <Landing path="courses">
@@ -193,4 +193,5 @@ describe("route matching", () => {
 interface Props {
   children?: React.ReactNode;
   path?: string;
+  index?: boolean;
 }
diff --git a/packages/react-router/__tests__/useRoutes-test.tsx b/packages/react-router/__tests__/useRoutes-test.tsx
index d1fe6f5c2f..fe38b69294 100644
--- a/packages/react-router/__tests__/useRoutes-test.tsx
+++ b/packages/react-router/__tests__/useRoutes-test.tsx
@@ -1,9 +1,9 @@
 import * as React from "react";
 import * as ReactDOM from "react-dom";
 import { create as createTestRenderer } from "react-test-renderer";
+import type { RouteObject } from "react-router";
 import { MemoryRouter as Router, useRoutes } from "react-router";
 import { act } from "react-dom/test-utils";
-import type { PartialRouteObject } from "react-router";
 
 describe("useRoutes", () => {
   it("returns the matching element from a route config", () => {
@@ -59,7 +59,7 @@ function RoutesRenderer({
   basename,
   location
 }: {
-  routes: PartialRouteObject[];
+  routes: Partial<RouteObject>[];
   basename?: string;
   location?: Partial<Location> & { pathname: string };
 }) {
diff --git a/packages/react-router/index.tsx b/packages/react-router/index.tsx
index 10fba67385..098b26e4e8 100644
--- a/packages/react-router/index.tsx
+++ b/packages/react-router/index.tsx
@@ -195,6 +195,7 @@ export interface RouteProps {
   caseSensitive?: boolean;
   children?: React.ReactNode;
   element?: React.ReactElement | null;
+  index?: boolean;
   path?: string;
 }
 
@@ -528,13 +529,34 @@ function useRoutes_(
     // You won't get a warning about 2 different <Routes> under a <Route>
     // without a trailing *, but this is a best-effort warning anyway since we
     // cannot even give the warning unless they land at the parent route.
+    //
+    // Example:
+    //
+    // <Routes>
+    //   {/* This route path MUST end with /* because otherwise
+    //       it will never match /blog/post/123 */}
+    //   <Route path="blog" element={<Blog />} />
+    //   <Route path="blog/feed" element={<BlogFeed />} />
+    // </Routes>
+    //
+    // function Blog() {
+    //   return (
+    //     <>
+    //       <Link to="post/123">123</Link>
+    //       <Routes>
+    //         <Route path="post/:id" element={<Post />} />
+    //       </Routes>
+    //     </>
+    //   )
+    // }
+    // function Post() { ... }
     let parentPath = parentRoute && parentRoute.path;
     warningOnce(
       parentPathname,
       !parentRoute || parentRoute.path.endsWith("*"),
       `You rendered descendant <Routes> (or called \`useRoutes\`) at ` +
         `"${parentPathname}" (under <Route path="${parentPath}">) but the ` +
-        `parent route path has no trailing "*". This means if you navigate ` +
+        `parent route path has no trailing "/*". This means if you navigate ` +
         `deeper, the parent won't match anymore and therefore the child ` +
         `routes will never render.\n\n` +
         `Please change the parent <Route path="${parentPath}"> to <Route ` +
@@ -550,12 +572,14 @@ function useRoutes_(
   let location = locationOverride ?? contextLocation;
 
   let matches = React.useMemo(
-    () => matchRoutes(routes, location, basenameForMatching),
+    () => matchRoutes_(routes, location, basenameForMatching),
     [location, routes, basenameForMatching]
   );
 
   if (!matches) {
-    // TODO: Warn about nothing matching, suggest using a catch-all route.
+    if (__DEV__) {
+      // TODO: Warn about nothing matching, suggest using a catch-all route.
+    }
     return null;
   }
 
@@ -595,8 +619,10 @@ export function createRoutesFromArray(
 ): RouteObject[] {
   return array.map(partialRoute => {
     let route: RouteObject = {
-      path: partialRoute.path || "/",
+      path: partialRoute.path || "",
       caseSensitive: partialRoute.caseSensitive === true,
+      index: partialRoute.index === true,
+      // This is the same as default value of <Route element>
       element: partialRoute.element || <Outlet />
     };
 
@@ -637,8 +663,9 @@ export function createRoutesFromChildren(
     }
 
     let route: RouteObject = {
-      path: element.props.path || "/",
+      path: element.props.path || "",
       caseSensitive: element.props.caseSensitive === true,
+      index: element.props.index === true,
       // Default behavior is to just render the element that was given. This
       // permits people to use any element they prefer, not just <Route> (though
       // all our official examples and docs use <Route> for clarity).
@@ -670,6 +697,7 @@ export type Params = Record<string, string>;
 export interface RouteObject {
   caseSensitive: boolean;
   children?: RouteObject[];
+  index: boolean;
   element: React.ReactNode;
   path: string;
 }
@@ -679,11 +707,9 @@ export interface RouteObject {
  * certain properties of a real route object such as `path` and `element`,
  * which have reasonable defaults.
  */
-export interface PartialRouteObject {
-  caseSensitive?: boolean;
+export interface PartialRouteObject
+  extends Omit<Partial<RouteObject>, "children"> {
   children?: PartialRouteObject[];
-  element?: React.ReactNode;
-  path?: string;
 }
 
 /**
@@ -716,6 +742,14 @@ export function matchRoutes(
     location = parsePath(location);
   }
 
+  return matchRoutes_(createRoutesFromArray(routes), location, basename);
+}
+
+function matchRoutes_(
+  routes: RouteObject[],
+  location: Partial<Location>,
+  basename = ""
+): RouteMatch[] | null {
   let pathname = location.pathname || "/";
   if (basename) {
     let base = basename.replace(/^\/*/, "/").replace(/\/+$/, "");
@@ -746,28 +780,41 @@ export interface RouteMatch {
 }
 
 function flattenRoutes(
-  routes: PartialRouteObject[],
+  routes: RouteObject[],
   branches: RouteBranch[] = [],
   parentPath = "",
   parentRoutes: RouteObject[] = [],
   parentIndexes: number[] = []
 ): RouteBranch[] {
-  (routes as RouteObject[]).forEach((route, index) => {
-    route = {
-      ...route,
-      path: route.path || "/",
-      caseSensitive: !!route.caseSensitive,
-      element: route.element
-    };
+  routes.forEach((route, index) => {
+    let path: string;
+    if (route.path.startsWith("/")) {
+      invariant(
+        parentPath && !route.path.startsWith(parentPath),
+        `Absolute <Route path="${route.path}"> must begin ` +
+          `with its parent path "${parentPath}", otherwise it ` +
+          `will be unreachable.`
+      );
+
+      path = route.path;
+    } else {
+      path = joinPaths([parentPath, route.path]);
+    }
 
-    let path = joinPaths([parentPath, route.path]);
     let routes = parentRoutes.concat(route);
     let indexes = parentIndexes.concat(index);
 
     // Add the children before adding this route to the array so we traverse the
     // route tree depth-first and child routes appear before their parents in
     // the "flattened" version.
-    if (route.children) {
+    if (route.children && route.children.length > 0) {
+      invariant(
+        !route.index,
+        `Index route for <Route path="${parentPath}"> must ` +
+          `not have children. If you want the route to be a layout ` +
+          `for its child routes, remove the \`index\` prop.`
+      );
+
       flattenRoutes(route.children, branches, path, routes, indexes);
     }
 
@@ -785,9 +832,7 @@ function rankRouteBranches(branches: RouteBranch[]): void {
     return memo;
   }, {});
 
-  // Sorting is stable in modern browsers, but we still support IE 11, so we
-  // need this little helper.
-  stableSort(branches, (a, b) => {
+  branches.sort((a, b) => {
     let [aPath, , aIndexes] = a;
     let aScore = pathScores[aPath];
 
@@ -843,13 +888,6 @@ function compareIndexes(a: number[], b: number[]): number {
       0;
 }
 
-function stableSort(array: any[], compareItems: (a: any, b: any) => number) {
-  // This copy lets us get the original index of an item so we can preserve the
-  // original ordering in the case that they sort equally.
-  let copy = array.slice(0);
-  array.sort((a, b) => compareItems(a, b) || copy.indexOf(a) - copy.indexOf(b));
-}
-
 function matchRouteBranch(
   branch: RouteBranch,
   pathname: string