Fix element <Route children>

This commit fixes <Route children> so that they do not render if the route does not match and they are elements. <Route children> functions still work as they did previously. Fixes #6362
remix-run · Sep 17, 2019 · 9665659 · 9665659
1 parent 0394c5e
commit 9665659
Show file tree

Hide file tree

Showing 2 changed files with 148 additions and 56 deletions.
diff --git a/packages/react-router/modules/Route.js b/packages/react-router/modules/Route.js
@@ -11,6 +11,19 @@ function isEmptyChildren(children) {
   return React.Children.count(children) === 0;
 }
 
+function evalChildrenDev(children, props, path) {
+  const value = children(props);
+
+  warning(
+    value !== undefined,
+    "You returned `undefined` from the `children` function of " +
+      `<Route${path ? ` path="${path}"` : ""}>, but you ` +
+      "should have returned a React element or `null`"
+  );
+
+  return value || null;
+}
+
 /**
  * The public API for matching a single path and rendering.
  */
@@ -25,8 +38,8 @@ class Route extends React.Component {
           const match = this.props.computedMatch
             ? this.props.computedMatch // <Switch> already computed the match for us
             : this.props.path
-              ? matchPath(location.pathname, this.props)
-              : context.match;
+            ? matchPath(location.pathname, this.props)
+            : context.match;
 
           const props = { ...context, location, match };
 
@@ -38,36 +51,25 @@ class Route extends React.Component {
             children = null;
           }
 
-          if (typeof children === "function") {
-            children = children(props);
-
-            if (children === undefined) {
-              if (__DEV__) {
-                const { path } = this.props;
-
-                warning(
-                  false,
-                  "You returned `undefined` from the `children` function of " +
-                    `<Route${path ? ` path="${path}"` : ""}>, but you ` +
-                    "should have returned a React element or `null`"
-                );
-              }
-
-              children = null;
-            }
-          }
-
           return (
             <RouterContext.Provider value={props}>
-              {children && !isEmptyChildren(children)
+              {props.match
                 ? children
-                : props.match
-                  ? component
-                    ? React.createElement(component, props)
-                    : render
-                      ? render(props)
-                      : null
-                  : null}
+                  ? typeof children === "function"
+                    ? __DEV__
+                      ? evalChildrenDev(children, props, this.props.path)
+                      : children(props)
+                    : children
+                  : component
+                  ? React.createElement(component, props)
+                  : render
+                  ? render(props)
+                  : null
+                : typeof children === "function"
+                ? __DEV__
+                  ? evalChildrenDev(children, props, this.props.path)
+                  : children(props)
+                : null}
             </RouterContext.Provider>
           );
         }}

diff --git a/packages/react-router/modules/__tests__/Route-test.js b/packages/react-router/modules/__tests__/Route-test.js
@@ -22,43 +22,133 @@ describe("A <Route>", () => {
     });
   });
 
-  it("renders when it matches", () => {
-    const text = "cupcakes";
+  describe("with a child element", () => {
+    it("renders when it matches", () => {
+      const text = "cupcakes";
 
-    renderStrict(
-      <MemoryRouter initialEntries={["/cupcakes"]}>
-        <Route path="/cupcakes" render={() => <h1>{text}</h1>} />
-      </MemoryRouter>,
-      node
-    );
+      renderStrict(
+        <MemoryRouter initialEntries={["/cupcakes"]}>
+          <Route path="/cupcakes">
+            <h1>{text}</h1>
+          </Route>
+        </MemoryRouter>,
+        node
+      );
+
+      expect(node.innerHTML).toContain(text);
+    });
+
+    it("renders when it matches at the root URL", () => {
+      const text = "cupcakes";
+
+      renderStrict(
+        <MemoryRouter initialEntries={["/"]}>
+          <Route path="/">
+            <h1>{text}</h1>
+          </Route>
+        </MemoryRouter>,
+        node
+      );
+
+      expect(node.innerHTML).toContain(text);
+    });
+
+    it("does not render when it does not match", () => {
+      const text = "bubblegum";
 
-    expect(node.innerHTML).toContain(text);
+      renderStrict(
+        <MemoryRouter initialEntries={["/bunnies"]}>
+          <Route path="/flowers">
+            <h1>{text}</h1>
+          </Route>
+        </MemoryRouter>,
+        node
+      );
+
+      expect(node.innerHTML).not.toContain(text);
+    });
   });
 
-  it("renders when it matches at the root URL", () => {
-    const text = "cupcakes";
+  describe("with a children function", () => {
+    it("renders when it matches", () => {
+      const text = "cupcakes";
 
-    renderStrict(
-      <MemoryRouter initialEntries={["/"]}>
-        <Route path="/" render={() => <h1>{text}</h1>} />
-      </MemoryRouter>,
-      node
-    );
+      renderStrict(
+        <MemoryRouter initialEntries={["/cupcakes"]}>
+          <Route path="/cupcakes" children={() => <h1>{text}</h1>} />
+        </MemoryRouter>,
+        node
+      );
+
+      expect(node.innerHTML).toContain(text);
+    });
 
-    expect(node.innerHTML).toContain(text);
+    it("renders when it matches at the root URL", () => {
+      const text = "cupcakes";
+
+      renderStrict(
+        <MemoryRouter initialEntries={["/"]}>
+          <Route path="/" children={() => <h1>{text}</h1>} />
+        </MemoryRouter>,
+        node
+      );
+
+      expect(node.innerHTML).toContain(text);
+    });
+
+    it("renders when it does not match", () => {
+      const text = "bubblegum";
+
+      renderStrict(
+        <MemoryRouter initialEntries={["/bunnies"]}>
+          <Route path="/flowers" children={() => <h1>{text}</h1>} />
+        </MemoryRouter>,
+        node
+      );
+
+      expect(node.innerHTML).toContain(text);
+    });
   });
 
-  it("does not render when it does not match", () => {
-    const text = "bubblegum";
+  describe("with a render prop", () => {
+    it("renders when it matches", () => {
+      const text = "cupcakes";
 
-    renderStrict(
-      <MemoryRouter initialEntries={["/bunnies"]}>
-        <Route path="/flowers" render={() => <h1>{text}</h1>} />
-      </MemoryRouter>,
-      node
-    );
+      renderStrict(
+        <MemoryRouter initialEntries={["/cupcakes"]}>
+          <Route path="/cupcakes" render={() => <h1>{text}</h1>} />
+        </MemoryRouter>,
+        node
+      );
 
-    expect(node.innerHTML).not.toContain(text);
+      expect(node.innerHTML).toContain(text);
+    });
+
+    it("renders when it matches at the root URL", () => {
+      const text = "cupcakes";
+
+      renderStrict(
+        <MemoryRouter initialEntries={["/"]}>
+          <Route path="/" render={() => <h1>{text}</h1>} />
+        </MemoryRouter>,
+        node
+      );
+
+      expect(node.innerHTML).toContain(text);
+    });
+
+    it("does not render when it does not match", () => {
+      const text = "bubblegum";
+
+      renderStrict(
+        <MemoryRouter initialEntries={["/bunnies"]}>
+          <Route path="/flowers" render={() => <h1>{text}</h1>} />
+        </MemoryRouter>,
+        node
+      );
+
+      expect(node.innerHTML).not.toContain(text);
+    });
   });
 
   it("matches using nextContext when updating", () => {