Skip to content

Commit

Permalink
Fix bug with changing fetcher key in a mounted component
Browse files Browse the repository at this point in the history
  • Loading branch information
brophdawg11 committed Nov 9, 2023
1 parent fe066bd commit 2acc01b
Show file tree
Hide file tree
Showing 3 changed files with 82 additions and 0 deletions.
5 changes: 5 additions & 0 deletions .changeset/changing-fetcher-key.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"react-router-dom": patch
---

Fix issue where a changing fetcher `key` in a `useFetcher` that remains mounted wasn't getting picked up
71 changes: 71 additions & 0 deletions packages/react-router-dom/__tests__/data-browser-router-test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5309,6 +5309,77 @@ function testDomRouter(
);
});

it("updates the key if it changes while the fetcher remains mounted", async () => {
let router = createTestRouter(
[
{
path: "/",
Component() {
let fetchers = useFetchers();
let [fetcherKey, setFetcherKey] = React.useState("a");
return (
<>
<ReusedFetcher fetcherKey={fetcherKey} />
<button onClick={() => setFetcherKey("b")}>
Change Key
</button>
<p>Fetchers:</p>
<pre>{JSON.stringify(fetchers)}</pre>
</>
);
},
},
{
path: "/echo",
loader: ({ request }) => request.url,
},
],
{ window: getWindow("/") }
);

function ReusedFetcher({ fetcherKey }: { fetcherKey: string }) {
let fetcher = useFetcher({ key: fetcherKey });

return (
<>
<button
onClick={() => fetcher.load(`/echo?fetcherKey=${fetcherKey}`)}
>
Load Fetcher
</button>
<p>{`fetcherKey:${fetcherKey}`}</p>
<p>Fetcher:{JSON.stringify(fetcher)}</p>
</>
);
}

let { container } = render(<RouterProvider router={router} />);

// Start with idle fetcher 'a'
expect(getHtml(container)).toContain('{"Form":{},"state":"idle"}');
expect(getHtml(container)).toContain("fetcherKey:a");

fireEvent.click(screen.getByText("Load Fetcher"));
await waitFor(
() => screen.getAllByText(/\/echo\?fetcherKey=a/).length > 0
);

// Fetcher 'a' now has data
expect(getHtml(container)).toContain(
'{"Form":{},"state":"idle","data":"http://localhost/echo?fetcherKey=a"}'
);
expect(getHtml(container)).toContain(
'[{"state":"idle","data":"http://localhost/echo?fetcherKey=a","key":"a"}]'
);

fireEvent.click(screen.getByText("Change Key"));
await waitFor(() => screen.getByText("fetcherKey:b"));

// We should have a new uninitialized/idle fetcher 'b'
expect(getHtml(container)).toContain('{"Form":{},"state":"idle"');
expect(getHtml(container)).toContain("[]");
});

it("exposes fetcher keys via useFetchers", async () => {
let router = createTestRouter(
[
Expand Down
6 changes: 6 additions & 0 deletions packages/react-router-dom/index.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1551,6 +1551,12 @@ export function useFetcher<TData = any>({
setFetcherKey(getUniqueFetcherId());
}

React.useEffect(() => {
if (key && key !== "" && key != fetcherKey) {
setFetcherKey(key);
}
}, [key, fetcherKey]);

// Registration/cleanup
React.useEffect(() => {
router.getFetcher(fetcherKey);
Expand Down

0 comments on commit 2acc01b

Please sign in to comment.