Skip to content

Commit

Permalink
Allow useNavigate to be called from child component effects (#10394)
Browse files Browse the repository at this point in the history 
Co-authored-by: 42shadow42 <42shadow42@gmail.com>
  • Loading branch information
@brophdawg11 @42shadow42
brophdawg11 and 42shadow42 committed Apr 25, 2023
1 parent af76d50 commit 5d45497
Show file tree
Hide file tree
Showing 4 changed files with 244 additions and 8 deletions.
5 changes: 5 additions & 0 deletions .changeset/navigate-in-effect.md
View file
@@ -0,0 +1,5 @@
---
"react-router": patch
---

Fix detection of `useNavigate` in the render cycle by setting the `activeRef` in a layout effect, allowing the `navigate` function to be passed to child components and called in a `useEffect` there.
4 changes: 2 additions & 2 deletions package.json
View file
Expand Up @@ -108,10 +108,10 @@
"none": "45.8 kB"
},
"packages/react-router/dist/react-router.production.min.js": {
"none": "13 kB"
"none": "13.1 kB"
},
"packages/react-router/dist/umd/react-router.production.min.js": {
"none": "15.3 kB"
"none": "15.4 kB"
},
"packages/react-router-dom/dist/react-router-dom.production.min.js": {
"none": "12 kB"
Expand Down
205 changes: 205 additions & 0 deletions packages/react-router/__tests__/useNavigate-test.tsx
View file
Expand Up @@ -301,6 +301,211 @@ describe("useNavigate", () => {
);
});

describe("navigating in effects versus render", () => {
let warnSpy: jest.SpyInstance;

beforeEach(() => {
warnSpy = jest.spyOn(console, "warn").mockImplementation(() => {});
});

afterEach(() => {
warnSpy.mockRestore();
});

describe("MemoryRouter", () => {
it("does not allow navigation from the render cycle", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<MemoryRouter>
<Routes>
<Route index element={<Home />} />
<Route path="about" element={<h1>About</h1>} />
</Routes>
</MemoryRouter>
);
});

function Home() {
let navigate = useNavigate();
navigate("/about");
return <h1>Home</h1>;
}

// @ts-expect-error
expect(renderer.toJSON()).toMatchInlineSnapshot(`
<h1>
Home
</h1>
`);
expect(warnSpy).toHaveBeenCalledWith(
"You should call navigate() in a React.useEffect(), not when your component is first rendered."
);
});

it("allows navigation from effects", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<MemoryRouter>
<Routes>
<Route index element={<Home />} />
<Route path="about" element={<h1>About</h1>} />
</Routes>
</MemoryRouter>
);
});

function Home() {
let navigate = useNavigate();
React.useEffect(() => navigate("/about"), [navigate]);
return <h1>Home</h1>;
}

// @ts-expect-error
expect(renderer.toJSON()).toMatchInlineSnapshot(`
<h1>
About
</h1>
`);
expect(warnSpy).not.toHaveBeenCalled();
});

it("allows navigation in child useEffects", () => {
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(
<MemoryRouter initialEntries={["/home"]}>
<Routes>
<Route path="home" element={<Parent />} />
<Route path="about" element={<h1>About</h1>} />
</Routes>
</MemoryRouter>
);
});

function Parent() {
let navigate = useNavigate();
let onChildRendered = React.useCallback(
() => navigate("/about"),
[navigate]
);
return <Child onChildRendered={onChildRendered} />;
}

function Child({ onChildRendered }) {
React.useEffect(() => onChildRendered());
return null;
}

// @ts-expect-error
expect(renderer.toJSON()).toMatchInlineSnapshot(`
<h1>
About
</h1>
`);
});
});

describe("RouterProvider", () => {
it("does not allow navigation from the render cycle", async () => {
let router = createMemoryRouter([
{
index: true,
Component() {
let navigate = useNavigate();
navigate("/about");
return <h1>Home</h1>;
},
},
{
path: "about",
element: <h1>About</h1>,
},
]);
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(<RouterProvider router={router} />);
});

// @ts-expect-error
expect(renderer.toJSON()).toMatchInlineSnapshot(`
<h1>
Home
</h1>
`);
expect(warnSpy).toHaveBeenCalledWith(
"You should call navigate() in a React.useEffect(), not when your component is first rendered."
);
});

it("allows navigation from effects", () => {
let router = createMemoryRouter([
{
index: true,
Component() {
let navigate = useNavigate();
React.useEffect(() => navigate("/about"), [navigate]);
return <h1>Home</h1>;
},
},
{
path: "about",
element: <h1>About</h1>,
},
]);
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(<RouterProvider router={router} />);
});

// @ts-expect-error
expect(renderer.toJSON()).toMatchInlineSnapshot(`
<h1>
About
</h1>
`);
expect(warnSpy).not.toHaveBeenCalled();
});

it("allows navigation in child useEffects", () => {
let router = createMemoryRouter([
{
index: true,
Component() {
let navigate = useNavigate();
let onChildRendered = React.useCallback(
() => navigate("/about"),
[navigate]
);
return <Child onChildRendered={onChildRendered} />;
},
},
{
path: "about",
element: <h1>About</h1>,
},
]);
let renderer: TestRenderer.ReactTestRenderer;
TestRenderer.act(() => {
renderer = TestRenderer.create(<RouterProvider router={router} />);
});

function Child({ onChildRendered }) {
React.useEffect(() => onChildRendered());
return null;
}

// @ts-expect-error
expect(renderer.toJSON()).toMatchInlineSnapshot(`
<h1>
About
</h1>
`);
});
});
});

describe("with state", () => {
it("adds the state to location.state", () => {
function Home() {
Expand Down
38 changes: 32 additions & 6 deletions packages/react-router/lib/hooks.tsx
View file
Expand Up @@ -150,6 +150,23 @@ export interface NavigateFunction {
(delta: number): void;
}

const navigateEffectWarning =
`You should call navigate() in a React.useEffect(), not when ` +
`your component is first rendered.`;

// Mute warnings for calls to useNavigate in SSR environments
function useIsomorphicLayoutEffect(
cb: Parameters<typeof React.useLayoutEffect>[0]
) {
let isStatic = React.useContext(NavigationContext).static;
if (!isStatic) {
// We should be able to get rid of this once react 18.3 is released
// See: https://github.com/facebook/react/pull/26395
// eslint-disable-next-line react-hooks/rules-of-hooks
React.useLayoutEffect(cb);
}
}

/**
* Returns an imperative method for changing the location. Used by <Link>s, but
* may also be used by other elements to change the location.
Expand Down Expand Up @@ -180,18 +197,16 @@ function useNavigateUnstable(): NavigateFunction {
);

let activeRef = React.useRef(false);
React.useEffect(() => {
useIsomorphicLayoutEffect(() => {
activeRef.current = true;
});

let navigate: NavigateFunction = React.useCallback(
(to: To | number, options: NavigateOptions = {}) => {
warning(
activeRef.current,
`You should call navigate() in a React.useEffect(), not when ` +
`your component is first rendered.`
);
warning(activeRef.current, navigateEffectWarning);

// Short circuit here since if this happens on first render the navigate
// is useless because we haven't wired up our history listener yet
if (!activeRef.current) return;

if (typeof to === "number") {
Expand Down Expand Up @@ -931,8 +946,19 @@ function useNavigateStable(): NavigateFunction {
let { router } = useDataRouterContext(DataRouterHook.UseNavigateStable);
let id = useCurrentRouteId(DataRouterStateHook.UseNavigateStable);

let activeRef = React.useRef(false);
useIsomorphicLayoutEffect(() => {
activeRef.current = true;
});

let navigate: NavigateFunction = React.useCallback(
(to: To | number, options: NavigateOptions = {}) => {
warning(activeRef.current, navigateEffectWarning);

// Short circuit here since if this happens on first render the navigate
// is useless because we haven't wired up our router subscriber yet
if (!activeRef.current) return;

if (typeof to === "number") {
router.navigate(to);
} else {
Expand Down

0 comments on commit 5d45497

Please sign in to comment.