Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix bug with changing fetcher key in a mounted component #11009

Merged
merged 2 commits into from
Nov 13, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/changing-fetcher-key.md
@@ -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
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
4 changes: 3 additions & 1 deletion packages/react-router-dom/index.tsx
Expand Up @@ -1547,7 +1547,9 @@ export function useFetcher<TData = any>({

// Fetcher key handling
let [fetcherKey, setFetcherKey] = React.useState<string>(key || "");
if (!fetcherKey) {
if (key && key !== fetcherKey) {
setFetcherKey(key);
} else if (!fetcherKey) {
setFetcherKey(getUniqueFetcherId());
}

Expand Down