From ac382020a2d1bb300aac0680a10a13d2b7b736b9 Mon Sep 17 00:00:00 2001 From: Michael Ferris Date: Wed, 15 Jan 2025 11:59:16 -0500 Subject: [PATCH 1/2] Changed return type of useMutation when ignoreResult is explicitly set to true to hide unset result --- .changeset/tasty-steaks-think.md | 5 ++ .../hooks/__tests__/useMutation.test.tsx | 60 ++++++++++++++++++- src/react/hooks/useMutation.ts | 33 ++++++++++ src/react/types/types.ts | 8 +-- 4 files changed, 100 insertions(+), 6 deletions(-) create mode 100644 .changeset/tasty-steaks-think.md diff --git a/.changeset/tasty-steaks-think.md b/.changeset/tasty-steaks-think.md new file mode 100644 index 00000000000..d7c1072e8f7 --- /dev/null +++ b/.changeset/tasty-steaks-think.md @@ -0,0 +1,5 @@ +--- +"@apollo/client": patch +--- + +Changed return type of useMutation when ignoreResult is explicitly set to true to avoid using unset values of the result diff --git a/src/react/hooks/__tests__/useMutation.test.tsx b/src/react/hooks/__tests__/useMutation.test.tsx index 2ac84fb45b8..109f40fb412 100644 --- a/src/react/hooks/__tests__/useMutation.test.tsx +++ b/src/react/hooks/__tests__/useMutation.test.tsx @@ -38,7 +38,7 @@ import { createRenderStream, renderHookToSnapshotStream, } from "@testing-library/react-render-stream"; -import { MutationTuple, QueryResult } from "../../types/types"; +import { MutationResult, MutationTuple, QueryResult } from "../../types/types"; import { invariant } from "../../../utilities/globals"; describe("useMutation Hook", () => { @@ -3389,4 +3389,62 @@ describe.skip("Type Tests", () => { expectTypeOf(data).toMatchTypeOf(); expectTypeOf(mutate()).toMatchTypeOf>>(); }); + + test("should not be able to access result when using ignoreResults", async () => { + const mutation = gql` + mutation { + updateUser { + id + } + } + `; + + type Mutation = { + updateUser: { + __typename: "User"; + id: string; + }; + }; + + // Explicit `true` + { + const [mutate, result] = useMutation(mutation, { + ignoreResults: true, + }); + expectTypeOf(result).toMatchTypeOf<{ reset: () => void }>(); + expectTypeOf(result).not.toMatchTypeOf>(); + expectTypeOf(mutate()).toMatchTypeOf>>(); + const { + reset, + // @ts-expect-error + data, + // @ts-expect-error + loading, + // @ts-expect-error + error, + } = result; + reset; + data; + loading; + error; + } + + // Explicit `false` + { + const [mutate, result] = useMutation(mutation, { + ignoreResults: false, + }); + expectTypeOf(result).toMatchTypeOf>(); + expectTypeOf(mutate()).toMatchTypeOf>>(); + } + + // Unknown boolean + { + const [mutate, result] = useMutation(mutation, { + ignoreResults: Math.random() > 0.5, + }); + expectTypeOf(result).toMatchTypeOf>(); + expectTypeOf(mutate()).toMatchTypeOf>>(); + } + }); }); diff --git a/src/react/hooks/useMutation.ts b/src/react/hooks/useMutation.ts index ea0f47ddc6c..687c87ca2b8 100644 --- a/src/react/hooks/useMutation.ts +++ b/src/react/hooks/useMutation.ts @@ -2,6 +2,7 @@ import * as React from "rehackt"; import type { DocumentNode } from "graphql"; import type { TypedDocumentNode } from "@graphql-typed-document-node/core"; import type { + MutationFunction, MutationFunctionOptions, MutationHookOptions, MutationResult, @@ -69,6 +70,38 @@ import { useIsomorphicLayoutEffect } from "./internal/useIsomorphicLayoutEffect. * @param options - Options to control how the mutation is executed. * @returns A tuple in the form of `[mutate, result]` */ +export function useMutation< + TData = any, + TVariables = OperationVariables, + TContext = DefaultContext, + TCache extends ApolloCache = ApolloCache, +>( + mutation: DocumentNode | TypedDocumentNode, + options: { ignoreResults: true } & MutationHookOptions< + NoInfer, + NoInfer, + TContext, + TCache + > +): [ + MutationFunction, + // result is not reliable when ignoreResults is true + Pick, "reset">, +]; +export function useMutation< + TData = any, + TVariables = OperationVariables, + TContext = DefaultContext, + TCache extends ApolloCache = ApolloCache, +>( + mutation: DocumentNode | TypedDocumentNode, + options?: MutationHookOptions< + NoInfer, + NoInfer, + TContext, + TCache + > +): MutationTuple; export function useMutation< TData = any, TVariables = OperationVariables, diff --git a/src/react/types/types.ts b/src/react/types/types.ts index 7812fb34bc2..065bd769c62 100644 --- a/src/react/types/types.ts +++ b/src/react/types/types.ts @@ -394,6 +394,8 @@ export declare type MutationFunction< TCache extends ApolloCache = ApolloCache, > = ( options?: MutationFunctionOptions + // TODO This FetchResult seems strange here, as opposed to an + // ApolloQueryResult ) => Promise>>; export interface MutationHookOptions< @@ -418,11 +420,7 @@ export type MutationTuple< TContext = DefaultContext, TCache extends ApolloCache = ApolloCache, > = [ - mutate: ( - options?: MutationFunctionOptions - // TODO This FetchResult seems strange here, as opposed to an - // ApolloQueryResult - ) => Promise>>, + mutate: MutationFunction, result: MutationResult, ]; From 185c8890a57cae86d0dbf1e350c3814f9a2eb929 Mon Sep 17 00:00:00 2001 From: Michael Ferris Date: Mon, 20 Jan 2025 09:37:34 -0500 Subject: [PATCH 2/2] More thorough test --- .../hooks/__tests__/useMutation.test.tsx | 26 +++++++++---------- src/react/hooks/useMutation.ts | 1 + 2 files changed, 14 insertions(+), 13 deletions(-) diff --git a/src/react/hooks/__tests__/useMutation.test.tsx b/src/react/hooks/__tests__/useMutation.test.tsx index 109f40fb412..6c164e08bb2 100644 --- a/src/react/hooks/__tests__/useMutation.test.tsx +++ b/src/react/hooks/__tests__/useMutation.test.tsx @@ -3414,19 +3414,19 @@ describe.skip("Type Tests", () => { expectTypeOf(result).toMatchTypeOf<{ reset: () => void }>(); expectTypeOf(result).not.toMatchTypeOf>(); expectTypeOf(mutate()).toMatchTypeOf>>(); - const { - reset, - // @ts-expect-error - data, - // @ts-expect-error - loading, - // @ts-expect-error - error, - } = result; - reset; - data; - loading; - error; + // Available and reliable in the result + result.reset; + + // @ts-expect-error + result.called; + // @ts-expect-error + result.client; + // @ts-expect-error + result.data; + // @ts-expect-error + result.error; + // @ts-expect-error + result.loading; } // Explicit `false` diff --git a/src/react/hooks/useMutation.ts b/src/react/hooks/useMutation.ts index 687c87ca2b8..1ab39b81816 100644 --- a/src/react/hooks/useMutation.ts +++ b/src/react/hooks/useMutation.ts @@ -218,6 +218,7 @@ export function useMutation< return response; }) .catch((error) => { + // TODO: Should this be checking for ignoreResults? if (mutationId === ref.current.mutationId && ref.current.isMounted) { const result = { loading: false,