Don't override path in NavLink component. Fixes #6613 (#6623)

* Don't override path in NavLink component. Fixes #6613 * Add tests for withRouter() inside NavLink * NavLink: Use RouterContext directly. Same reasons as for withRouter: Less nesting, clearer error message. * NavLink: Remove obsolete import. Updated size snapshot
remix-run · May 16, 2019 · a38ef04 · a38ef04
1 parent 56c829b
commit a38ef04
Show file tree

Hide file tree

Showing 4 changed files with 99 additions and 29 deletions.
diff --git a/packages/react-router-dom/.size-snapshot.json b/packages/react-router-dom/.size-snapshot.json
@@ -1,8 +1,8 @@
 {
   "esm/react-router-dom.js": {
-    "bundled": 8159,
-    "minified": 4903,
-    "gzipped": 1641,
+    "bundled": 8362,
+    "minified": 5058,
+    "gzipped": 1651,
     "treeshaked": {
       "rollup": {
         "code": 453,
@@ -14,13 +14,13 @@
     }
   },
   "umd/react-router-dom.js": {
-    "bundled": 159187,
-    "minified": 56687,
-    "gzipped": 16378
+    "bundled": 159395,
+    "minified": 56787,
+    "gzipped": 16387
   },
   "umd/react-router-dom.min.js": {
-    "bundled": 96042,
-    "minified": 33707,
-    "gzipped": 9946
+    "bundled": 96151,
+    "minified": 33747,
+    "gzipped": 9951
   }
 }
diff --git a/packages/react-router-dom/modules/NavLink.js b/packages/react-router-dom/modules/NavLink.js
@@ -1,8 +1,8 @@
 import React from "react";
-import { Route } from "react-router";
+import { __RouterContext as RouterContext, matchPath } from "react-router";
 import PropTypes from "prop-types";
-
 import Link from "./Link";
+import invariant from "tiny-invariant";
 
 function joinClassnames(...classnames) {
   return classnames.filter(i => i).join(" ");
@@ -18,7 +18,7 @@ function NavLink({
   className: classNameProp,
   exact,
   isActive: isActiveProp,
-  location,
+  location: locationProp,
   strict,
   style: styleProp,
   to,
@@ -30,14 +30,18 @@ function NavLink({
   const escapedPath = path && path.replace(/([.+*?=^!:${}()[\]|/\\])/g, "\\$1");
 
   return (
-    <Route
-      path={escapedPath}
-      exact={exact}
-      strict={strict}
-      location={location}
-      children={({ location, match }) => {
+    <RouterContext.Consumer>
+      {context => {
+        invariant(context, "You should not use <NavLink> outside a <Router>");
+
+        const pathToMatch = locationProp
+          ? locationProp.pathname
+          : context.location.pathname;
+        const match = escapedPath
+          ? matchPath(pathToMatch, { path: escapedPath, exact, strict })
+          : null;
         const isActive = !!(isActiveProp
-          ? isActiveProp(match, location)
+          ? isActiveProp(match, context.location)
           : match);
 
         const className = isActive
@@ -55,7 +59,7 @@ function NavLink({
           />
         );
       }}
-    />
+    </RouterContext.Consumer>
   );
 }
 
@@ -75,10 +79,10 @@ if (__DEV__) {
     activeClassName: PropTypes.string,
     activeStyle: PropTypes.object,
     className: PropTypes.string,
-    exact: Route.propTypes.exact,
+    exact: PropTypes.bool,
     isActive: PropTypes.func,
     location: PropTypes.object,
-    strict: Route.propTypes.strict,
+    strict: PropTypes.bool,
     style: PropTypes.object
   };
 }

diff --git a/packages/react-router-dom/modules/__tests__/NavLink-test.js b/packages/react-router-dom/modules/__tests__/NavLink-test.js
@@ -1,6 +1,6 @@
 import React from "react";
 import ReactDOM from "react-dom";
-import { MemoryRouter, NavLink, withRouter } from "react-router-dom";
+import { MemoryRouter, NavLink, withRouter, Route } from "react-router-dom";
 
 import renderStrict from "./utils/renderStrict";
 
@@ -487,4 +487,70 @@ describe("A <NavLink>", () => {
       expect(a.className).not.toContain("active");
     });
   });
+
+  describe("doesn't interfere with withRouter", () => {
+    let props;
+
+    const PropsChecker = withRouter(p => {
+      props = p;
+      return null;
+    });
+
+    beforeEach(() => {
+      props = null;
+    });
+
+    it("allows withRouter to access the correct match without route", () => {
+      renderStrict(
+        <MemoryRouter initialEntries={["/pizza/"]}>
+          <NavLink to="/pizza/">
+            <PropsChecker />
+          </NavLink>
+        </MemoryRouter>,
+        node
+      );
+
+      expect(props.match).not.toBeNull();
+      expect(props.match.url).toBe("/");
+    });
+
+    it("allows withRouter to access the correct match inside a route", () => {
+      renderStrict(
+        <MemoryRouter initialEntries={["/pizza/"]}>
+          <Route
+            path="/pizza"
+            component={() => (
+              <NavLink to="/pizza/">
+                <PropsChecker />
+              </NavLink>
+            )}
+          />
+        </MemoryRouter>,
+        node
+      );
+
+      expect(props.match).not.toBeNull();
+      expect(props.match.url).toBe("/pizza/");
+    });
+
+    it("allows withRouter to access the correct match with params inside a route", () => {
+      renderStrict(
+        <MemoryRouter initialEntries={["/pizza/cheese"]}>
+          <Route
+            path="/pizza/:topping"
+            component={() => (
+              <NavLink to="/pizza/">
+                <PropsChecker />
+              </NavLink>
+            )}
+          />
+        </MemoryRouter>,
+        node
+      );
+
+      expect(props.match).not.toBeNull();
+      expect(props.match.url).toBe("/pizza/cheese");
+      expect(props.match.params).toEqual({ topping: "cheese" });
+    });
+  });
 });
diff --git a/packages/react-router/.size-snapshot.json b/packages/react-router/.size-snapshot.json
@@ -1,8 +1,8 @@
 {
   "esm/react-router.js": {
-    "bundled": 23463,
-    "minified": 13259,
-    "gzipped": 3684,
+    "bundled": 23522,
+    "minified": 13316,
+    "gzipped": 3688,
     "treeshaked": {
       "rollup": {
         "code": 2356,
@@ -14,9 +14,9 @@
     }
   },
   "umd/react-router.js": {
-    "bundled": 99114,
-    "minified": 35078,
-    "gzipped": 11239
+    "bundled": 99173,
+    "minified": 35111,
+    "gzipped": 11242
   },
   "umd/react-router.min.js": {
     "bundled": 61717,