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

Commit

Permalink
stabilize across multiple calls fix even more
Browse files Browse the repository at this point in the history
Revisiting #335 in light of the issue noted in #337

First of all, stabilize the useMutate function even more by using a deep
compare effect. This better handles cases in which the argument to
`useMutate` is an inline object, which is the typical use case.
  • Loading branch information
amacleay-cohere authored and fabien0102 committed Apr 22, 2021
1 parent d34e7e7 commit 5d2ad1e
Show file tree
Hide file tree
Showing 2 changed files with 78 additions and 26 deletions.
48 changes: 47 additions & 1 deletion src/useMutate.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,24 @@ describe("useMutate", () => {
});
});

it("should cancel on unmount", async () => {
nock("https://my-awesome-api.fake")
.delete("/plop")
.reply(200, { oops: true });
const wrapper: React.FC = ({ children }) => (
<RestfulProvider base="https://my-awesome-api.fake">{children}</RestfulProvider>
);

const resolve = jest.fn(() => "/plop");
const { result, unmount } = renderHook(() => useMutate("DELETE", resolve), { wrapper });

const resultPromise = result.current.mutate({});
unmount();
const res = await resultPromise;

expect(res).toEqual(undefined);
});

it("should call the correct url with a specific id", async () => {
nock("https://my-awesome-api.fake")
.delete("/plop")
Expand Down Expand Up @@ -419,7 +437,7 @@ describe("useMutate", () => {
});

describe("Mutate identity", () => {
it("should remain the same across calls", async () => {
it("should remain the same across calls with static props", async () => {
nock("https://my-awesome-api.fake")
.post("/plop/one")
.reply(200, { id: 1 })
Expand Down Expand Up @@ -447,6 +465,34 @@ describe("useMutate", () => {
expect(mutate0).toEqual(mutate1);
expect(mutate0).toEqual(mutate2);
});

it("should remain the same across calls with deeply equal props", async () => {
nock("https://my-awesome-api.fake")
.post("/plop/one")
.reply(200, { id: 1 })
.persist();

const wrapper: React.FC = ({ children }) => (
<RestfulProvider base="https://my-awesome-api.fake">{children}</RestfulProvider>
);
const getPath = ({ id }: { id: string }) => `plop/${id}`;
const { result } = renderHook(
() =>
useMutate<{ id: number }, unknown, {}, {}, { id: string }>("POST", getPath, {
pathParams: { id: "one" },
}),
{
wrapper,
},
);
const mutate0 = result.current.mutate;
const mutate1 = result.current.mutate;
await result.current.mutate({});
const mutate2 = result.current.mutate;

expect(mutate0).toEqual(mutate1);
expect(mutate0).toEqual(mutate2);
});
});

describe("Path Params", () => {
Expand Down
56 changes: 31 additions & 25 deletions src/useMutate.tsx
Original file line number Diff line number Diff line change
@@ -1,11 +1,12 @@
import merge from "lodash/merge";
import { useCallback, useContext, useEffect, useState } from "react";
import { useContext, useEffect, useState } from "react";
import { Context } from "./Context";
import { MutateMethod, MutateState, MutateRequestOptions } from "./Mutate";
import { Omit, UseGetProps } from "./useGet";
import { constructUrl } from "./util/constructUrl";
import { processResponse } from "./util/processResponse";
import { useAbort } from "./useAbort";
import { useDeepCompareCallback, useDeepCompareEffect } from "./util/useDeepCompareEffect";

export interface UseMutateProps<TData, TError, TQueryParams, TRequestBody, TPathParams>
extends Omit<UseGetProps<TData, TError, TQueryParams, TPathParams>, "lazy" | "debounce" | "mock"> {
Expand Down Expand Up @@ -94,13 +95,32 @@ export function useMutate<
useEffect(() => () => abort(), [abort]);

const { pathInlineBodyEncode, queryParamStringifyOptions, requestOptions, localErrorOnly, onMutate } = props;
const mutate = useCallback<MutateMethod<TData, TRequestBody, TQueryParams, TPathParams>>(

const effectDependencies = [
path,
pathParams,
queryParams,
verb,
isDelete,
base,
context,
queryParamStringifyOptions,
requestOptions,
onMutate,
abort,
pathInlineBodyEncode,
localErrorOnly,
resolve,
];
const mutate = useDeepCompareCallback<MutateMethod<TData, TRequestBody, TQueryParams, TPathParams>>(
async (body: TRequestBody, mutateRequestOptions?: MutateRequestOptions<TQueryParams, TPathParams>) => {
const signal = getAbortSignal();

setState(prevState => {
if (prevState.loading) {
abort();
if (prevState.error || !prevState.loading) {
return { ...prevState, loading: true, error: null };
}
return { ...prevState, loading: true, error: null };
return prevState;
});

const pathStr =
Expand Down Expand Up @@ -129,8 +149,6 @@ export function useMutate<
options.body = (body as unknown) as string;
}

const signal = getAbortSignal();

const url = constructUrl(
base,
pathParts.join("/"),
Expand Down Expand Up @@ -233,25 +251,13 @@ export function useMutate<

return data;
},
// eslint-disable-next-line react-hooks/exhaustive-deps
[
/* getAbortSignal - changes too much! */
path,
pathParams,
queryParams,
verb,
isDelete,
base,
context,
queryParamStringifyOptions,
requestOptions,
onMutate,
abort,
pathInlineBodyEncode,
localErrorOnly,
resolve,
],
effectDependencies,
);
useDeepCompareEffect(() => {
if (state.loading) {
abort();
}
}, effectDependencies);

return {
...state,
Expand Down

0 comments on commit 5d2ad1e

Please sign in to comment.