Skip to content
This repository has been archived by the owner on Nov 11, 2023. It is now read-only.

Commit

Permalink
Avoid requests side effects once component unmounts
Browse files Browse the repository at this point in the history
  • Loading branch information
carlesba authored and fabien0102 committed Nov 1, 2018
1 parent b4683d4 commit 237c08a
Show file tree
Hide file tree
Showing 3 changed files with 47 additions and 3 deletions.
28 changes: 28 additions & 0 deletions src/Get.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -102,6 +102,34 @@ describe("Get", () => {
await wait(() => expect(children.mock.calls.length).toBe(2));
expect(children.mock.calls[1][0]).toEqual({ hello: "world" });
});

it("shouldn't resolve after component unmounts", async () => {
let requestResolves;
const pendingRequestFinishes = new Promise(resolvePromise => {
requestResolves = resolvePromise;
});
nock("https://my-awesome-api.fake")
.get("/")
.reply(200, async () => {
await pendingRequestFinishes;
});

const children = jest.fn();
children.mockReturnValue(<div />);
const resolve = jest.fn((a: any) => a);

const { unmount } = render(
<RestfulProvider base="https://my-awesome-api.fake">
<Get path="" resolve={resolve}>
{children}
</Get>
</RestfulProvider>,
);

unmount();
requestResolves();
await wait(() => expect(resolve).not.toHaveBeenCalled());
});
});

describe("with error", () => {
Expand Down
17 changes: 16 additions & 1 deletion src/Get.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,12 @@ class ContextlessGet<TData, TError> extends React.Component<
}
}

/**
* Abort controller to cancel the current fetch query
*/
private abortController = new AbortController();
private signal = this.abortController.signal;

public readonly state: Readonly<GetState<TData, TError>> = {
data: null, // Means we don't _yet_ have data.
response: null,
Expand Down Expand Up @@ -184,6 +190,10 @@ class ContextlessGet<TData, TError> extends React.Component<
}
}

public componentWillUnmount() {
this.abortController.abort();
}

public getRequestOptions = (
extraOptions?: Partial<RequestInit>,
extraHeaders?: boolean | { [key: string]: string },
Expand Down Expand Up @@ -228,9 +238,14 @@ class ContextlessGet<TData, TError> extends React.Component<
};

const request = new Request(makeRequestPath(), this.getRequestOptions(thisRequestOptions));
const response = await fetch(request);
const response = await fetch(request, { signal: this.signal });
const { data, responseError } = await processResponse(response);

// avoid state updates when component has been unmounted
if (this.signal.aborted) {
return;
}

if (!response.ok || responseError) {
const error = {
message: `Failed to fetch: ${response.status} ${response.statusText}${responseError ? " - " + data : ""}`,
Expand Down
5 changes: 3 additions & 2 deletions src/Poll.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -226,8 +226,9 @@ class ContextlessPoll<TData, TError> extends React.Component<
const response = await fetch(request, { signal: this.signal });
const { data, responseError } = await processResponse(response);

if (!this.keepPolling) {
// Early return if we have stopped polling to avoid memory leaks
if (!this.keepPolling || this.signal.aborted) {
// Early return if we have stopped polling or component was unmounted
// to avoid memory leaks
return;
}

Expand Down

0 comments on commit 237c08a

Please sign in to comment.