From 5dcc570a0de62ff293b49cfdf3652dbf2fb47061 Mon Sep 17 00:00:00 2001 From: John Pangalos Date: Mon, 25 Jul 2022 12:02:57 +0200 Subject: [PATCH 1/4] Wrap route with location arg in location context In order for the `useLocation` hook to work with the use of the modal pattern, routes with the locationArg prop are wrapped in a LocationContext. This is done conditionally in order to ensure performance in the default case doesn't change. --- .changeset/hungry-vans-ring.md | 8 ++++++ package.json | 2 +- .../react-router/__tests__/useRoutes-test.tsx | 15 ++++++++--- packages/react-router/lib/hooks.tsx | 26 ++++++++++++++++++- 4 files changed, 46 insertions(+), 5 deletions(-) create mode 100644 .changeset/hungry-vans-ring.md diff --git a/.changeset/hungry-vans-ring.md b/.changeset/hungry-vans-ring.md new file mode 100644 index 0000000000..4167a30ea0 --- /dev/null +++ b/.changeset/hungry-vans-ring.md @@ -0,0 +1,8 @@ +--- +"react-router": patch +"react-router-dom": patch +"react-router-dom-v5-compat": patch +"react-router-native": patch +--- + +Fix for `useLocation` returning the wrong `Location` when passing a `locationArg` into a `Routes` component. diff --git a/package.json b/package.json index cb0360635b..1a2216c015 100644 --- a/package.json +++ b/package.json @@ -107,7 +107,7 @@ "none": "12 kB" }, "packages/react-router/dist/umd/react-router.production.min.js": { - "none": "13.5 kB" + "none": "14 kB" }, "packages/react-router-dom/dist/react-router-dom.production.min.js": { "none": "10 kB" diff --git a/packages/react-router/__tests__/useRoutes-test.tsx b/packages/react-router/__tests__/useRoutes-test.tsx index 89cf99a0da..bfbff06875 100644 --- a/packages/react-router/__tests__/useRoutes-test.tsx +++ b/packages/react-router/__tests__/useRoutes-test.tsx @@ -1,7 +1,7 @@ import * as React from "react"; import * as TestRenderer from "react-test-renderer"; import type { RouteObject } from "react-router"; -import { MemoryRouter, useRoutes } from "react-router"; +import { MemoryRouter, useRoutes, useLocation } from "react-router"; describe("useRoutes", () => { it("returns the matching element from a route config", () => { @@ -54,9 +54,16 @@ describe("useRoutes", () => { }); it("Uses the `location` prop instead of context location`", () => { + let TwoComponent = () => { + const location = useLocation(); + return

two path:{location.pathname}

; + }; let routes = [ { path: "one", element:

one

}, - { path: "two", element:

two

}, + { + path: "two", + element: , + }, ]; let renderer: TestRenderer.ReactTestRenderer; @@ -68,9 +75,11 @@ describe("useRoutes", () => { ); }); + // console.log(renderer.toTree()); expect(renderer.toJSON()).toMatchInlineSnapshot(`

- two + two path: + /two

`); }); diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index 932d802738..9b49825400 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -408,7 +408,7 @@ export function useRoutes( ); } - return _renderMatches( + let renderedMatches = _renderMatches( matches && matches.map((match) => Object.assign({}, match, { @@ -423,6 +423,30 @@ export function useRoutes( parentMatches, dataRouterStateContext || undefined ); + + // When a user passes in a `locationArg` prop, the associated routes need to + // be wrapped in a `LocationContext.Provider` in order for `useLocation` to + // use the `locationArg` location instead of the global location. + if (locationArg) { + return ( + + {renderedMatches} + + ); + } + return renderedMatches; } function DefaultErrorElement() { From 6f0012b46ced641dbed75c9b5da17fc35158677d Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 12 Sep 2022 12:56:05 -0400 Subject: [PATCH 2/4] Update useLocation unit tests --- .changeset/hungry-vans-ring.md | 2 +- .../__tests__/useLocation-test.tsx | 49 +++++++++++++++++-- .../react-router/__tests__/useRoutes-test.tsx | 12 ++--- packages/react-router/lib/hooks.tsx | 4 +- 4 files changed, 50 insertions(+), 17 deletions(-) diff --git a/.changeset/hungry-vans-ring.md b/.changeset/hungry-vans-ring.md index 4167a30ea0..cbfe75bedc 100644 --- a/.changeset/hungry-vans-ring.md +++ b/.changeset/hungry-vans-ring.md @@ -5,4 +5,4 @@ "react-router-native": patch --- -Fix for `useLocation` returning the wrong `Location` when passing a `locationArg` into a `Routes` component. +fix: update `useLocation` to return the scoped `Location` when inside a `` component diff --git a/packages/react-router/__tests__/useLocation-test.tsx b/packages/react-router/__tests__/useLocation-test.tsx index 7da12f1a12..c802e1cf99 100644 --- a/packages/react-router/__tests__/useLocation-test.tsx +++ b/packages/react-router/__tests__/useLocation-test.tsx @@ -2,9 +2,9 @@ import * as React from "react"; import * as TestRenderer from "react-test-renderer"; import { MemoryRouter, Routes, Route, useLocation } from "react-router"; -function ShowPath() { - let { pathname, search, hash } = useLocation(); - return 
{JSON.stringify({ pathname, search, hash })}
; +function ShowLocation() { + let location = useLocation(); + return 
{JSON.stringify(location)}
; } describe("useLocation", () => { @@ -14,7 +14,7 @@ describe("useLocation", () => { renderer = TestRenderer.create( - } /> + } /> ); @@ -22,8 +22,47 @@ describe("useLocation", () => { expect(renderer.toJSON()).toMatchInlineSnapshot(` 
-        {"pathname":"/home","search":"?the=search","hash":"#the-hash"}
+        {"pathname":"/home","search":"?the=search","hash":"#the-hash","state":null,"key":"default"}
`); }); + + it("returns the scoped location object when nested in ", () => { + let renderer: TestRenderer.ReactTestRenderer; + TestRenderer.act(() => { + renderer = TestRenderer.create( + + + + ); + }); + + function App() { + return ( +
+

App

+ + } /> + + + } /> + +
+ ); + } + + expect(renderer.toJSON()).toMatchInlineSnapshot(` +
+

+ App +

+ 
+          {"pathname":"/home","search":"?the=search","hash":"#the-hash","state":null,"key":"default"}
+
+ 
+          {"pathname":"/scoped","search":"?scoped=search","hash":"#scoped-hash","state":null,"key":"default"}
+
+
+ `); + }); }); diff --git a/packages/react-router/__tests__/useRoutes-test.tsx b/packages/react-router/__tests__/useRoutes-test.tsx index bfbff06875..eef5e550a7 100644 --- a/packages/react-router/__tests__/useRoutes-test.tsx +++ b/packages/react-router/__tests__/useRoutes-test.tsx @@ -1,7 +1,7 @@ import * as React from "react"; import * as TestRenderer from "react-test-renderer"; import type { RouteObject } from "react-router"; -import { MemoryRouter, useRoutes, useLocation } from "react-router"; +import { MemoryRouter, useRoutes } from "react-router"; describe("useRoutes", () => { it("returns the matching element from a route config", () => { @@ -54,15 +54,11 @@ describe("useRoutes", () => { }); it("Uses the `location` prop instead of context location`", () => { - let TwoComponent = () => { - const location = useLocation(); - return

two path:{location.pathname}

; - }; let routes = [ { path: "one", element:

one

}, { path: "two", - element: , + element:

two

, }, ]; @@ -75,11 +71,9 @@ describe("useRoutes", () => { ); }); - // console.log(renderer.toTree()); expect(renderer.toJSON()).toMatchInlineSnapshot(`

- two path: - /two + two

`); }); diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index b435182768..fa0aa82543 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -449,11 +449,11 @@ export function useRoutes( Date: Mon, 12 Sep 2022 12:57:45 -0400 Subject: [PATCH 3/4] Remove static markup from test for brevity --- packages/react-router/__tests__/useLocation-test.tsx | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/react-router/__tests__/useLocation-test.tsx b/packages/react-router/__tests__/useLocation-test.tsx index c802e1cf99..2c7cfa6c69 100644 --- a/packages/react-router/__tests__/useLocation-test.tsx +++ b/packages/react-router/__tests__/useLocation-test.tsx @@ -40,7 +40,6 @@ describe("useLocation", () => { function App() { return (
-

App

} /> @@ -53,9 +52,6 @@ describe("useLocation", () => { expect(renderer.toJSON()).toMatchInlineSnapshot(`
-

- App - 

           {"pathname":"/home","search":"?the=search","hash":"#the-hash","state":null,"key":"default"}
From 29f273b142cf50f664f46a7971e4c3fe0eef6e48 Mon Sep 17 00:00:00 2001 From: Matt Brophy Date: Mon, 12 Sep 2022 13:00:15 -0400 Subject: [PATCH 4/4] Back out formatting change to test --- packages/react-router/__tests__/useRoutes-test.tsx | 5 +---- packages/react-router/lib/hooks.tsx | 7 ++++--- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/packages/react-router/__tests__/useRoutes-test.tsx b/packages/react-router/__tests__/useRoutes-test.tsx index eef5e550a7..89cf99a0da 100644 --- a/packages/react-router/__tests__/useRoutes-test.tsx +++ b/packages/react-router/__tests__/useRoutes-test.tsx @@ -56,10 +56,7 @@ describe("useRoutes", () => { it("Uses the `location` prop instead of context location`", () => { let routes = [ { path: "one", element:

one

}, - { - path: "two", - element:

two

, - }, + { path: "two", element:

two

}, ]; let renderer: TestRenderer.ReactTestRenderer; diff --git a/packages/react-router/lib/hooks.tsx b/packages/react-router/lib/hooks.tsx index fa0aa82543..020e0c63be 100644 --- a/packages/react-router/lib/hooks.tsx +++ b/packages/react-router/lib/hooks.tsx @@ -441,9 +441,9 @@ export function useRoutes( dataRouterStateContext || undefined ); - // When a user passes in a `locationArg` prop, the associated routes need to - // be wrapped in a `LocationContext.Provider` in order for `useLocation` to - // use the `locationArg` location instead of the global location. + // When a user passes in a `locationArg`, the associated routes need to + // be wrapped in a new `LocationContext.Provider` in order for `useLocation` + // to use the scoped location instead of the global location. if (locationArg) { return ( ); } + return renderedMatches; }