From 3b538fb5eec1a0bb601d96bd34b56fa457de12ce Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Fri, 3 Feb 2023 16:50:46 +0100 Subject: [PATCH 1/3] add `NoInfer` to hook options `TData` and `TVariables` for `useQuery`, `useLazyQuery` and `useMutation` to prevent accidental "widening" of inferred generics --- .changeset/hungry-eagles-kick.md | 5 ++++ .../hooks/__tests__/useLazyQuery.test.tsx | 16 +++++++++++ .../hooks/__tests__/useMutation.test.tsx | 13 +++++++++ src/react/hooks/__tests__/useQuery.test.tsx | 16 +++++++++++ src/react/hooks/useLazyQuery.ts | 3 ++- src/react/hooks/useMutation.ts | 3 ++- src/react/hooks/useQuery.ts | 3 ++- src/react/types/types.ts | 27 +++++++++++++++++++ 8 files changed, 83 insertions(+), 3 deletions(-) create mode 100644 .changeset/hungry-eagles-kick.md diff --git a/.changeset/hungry-eagles-kick.md b/.changeset/hungry-eagles-kick.md new file mode 100644 index 00000000000..b83f7969726 --- /dev/null +++ b/.changeset/hungry-eagles-kick.md @@ -0,0 +1,5 @@ +--- +'@apollo/client': minor +--- + +prevent accidental widening of inferred TData and TVariables generics for query hook option arguments diff --git a/src/react/hooks/__tests__/useLazyQuery.test.tsx b/src/react/hooks/__tests__/useLazyQuery.test.tsx index 9f5f780e090..13bcbc03c45 100644 --- a/src/react/hooks/__tests__/useLazyQuery.test.tsx +++ b/src/react/hooks/__tests__/useLazyQuery.test.tsx @@ -1257,3 +1257,19 @@ describe('useLazyQuery Hook', () => { }); }); }); + +describe.skip("Type Tests", () => { + test('NoInfer prevents adding arbitrary additional variables', () => { + const typedNode = {} as TypedDocumentNode<{ foo: string}, { bar: number }> + const [_, { variables }] = useLazyQuery(typedNode, { + variables: { + bar: 4, + // @ts-expect-error + nonExistingVariable: "string" + } + }); + variables?.bar + // @ts-expect-error + variables?.nonExistingVariable + }) +}) diff --git a/src/react/hooks/__tests__/useMutation.test.tsx b/src/react/hooks/__tests__/useMutation.test.tsx index 2f02d0c6869..8c74a8f0a46 100644 --- a/src/react/hooks/__tests__/useMutation.test.tsx +++ b/src/react/hooks/__tests__/useMutation.test.tsx @@ -2567,3 +2567,16 @@ describe('useMutation Hook', () => { }); }); }); + +describe.skip("Type Tests", () => { + test('NoInfer prevents adding arbitrary additional variables', () => { + const typedNode = {} as TypedDocumentNode<{ foo: string}, { bar: number }> + useMutation(typedNode, { + variables: { + bar: 4, + // @ts-expect-error + nonExistingVariable: "string" + } + }); + }) +}) diff --git a/src/react/hooks/__tests__/useQuery.test.tsx b/src/react/hooks/__tests__/useQuery.test.tsx index 9bff8e7120c..7dbb85ac8a7 100644 --- a/src/react/hooks/__tests__/useQuery.test.tsx +++ b/src/react/hooks/__tests__/useQuery.test.tsx @@ -7135,3 +7135,19 @@ describe('useQuery Hook', () => { }); }); }); + +describe.skip("Type Tests", () => { + test('NoInfer prevents adding arbitrary additional variables', () => { + const typedNode = {} as TypedDocumentNode<{ foo: string}, { bar: number }> + const { variables } = useQuery(typedNode, { + variables: { + bar: 4, + // @ts-expect-error + nonExistingVariable: "string" + } + }); + variables?.bar + // @ts-expect-error + variables?.nonExistingVariable + }) +}) diff --git a/src/react/hooks/useLazyQuery.ts b/src/react/hooks/useLazyQuery.ts index 1493255d798..0b239cac0f3 100644 --- a/src/react/hooks/useLazyQuery.ts +++ b/src/react/hooks/useLazyQuery.ts @@ -7,6 +7,7 @@ import { mergeOptions } from '../../utilities'; import { LazyQueryHookOptions, LazyQueryResultTuple, + NoInfer, QueryResult, } from '../types/types'; import { useInternalState } from './useQuery'; @@ -25,7 +26,7 @@ const EAGER_METHODS = [ export function useLazyQuery( query: DocumentNode | TypedDocumentNode, - options?: LazyQueryHookOptions + options?: LazyQueryHookOptions, NoInfer> ): LazyQueryResultTuple { const abortControllersRef = useRef(new Set()); const internalState = useInternalState( diff --git a/src/react/hooks/useMutation.ts b/src/react/hooks/useMutation.ts index 4937c08ba6f..382fef3c1a8 100644 --- a/src/react/hooks/useMutation.ts +++ b/src/react/hooks/useMutation.ts @@ -6,6 +6,7 @@ import { MutationHookOptions, MutationResult, MutationTuple, + NoInfer, } from '../types/types'; import { @@ -26,7 +27,7 @@ export function useMutation< TCache extends ApolloCache = ApolloCache, >( mutation: DocumentNode | TypedDocumentNode, - options?: MutationHookOptions, + options?: MutationHookOptions, NoInfer, TContext, TCache>, ): MutationTuple { const client = useApolloClient(options?.client); verifyDocumentType(mutation, DocumentType.Mutation); diff --git a/src/react/hooks/useQuery.ts b/src/react/hooks/useQuery.ts index 97e010ad4ff..953fe9ca497 100644 --- a/src/react/hooks/useQuery.ts +++ b/src/react/hooks/useQuery.ts @@ -26,6 +26,7 @@ import { QueryHookOptions, QueryResult, ObservableQueryFields, + NoInfer, } from '../types/types'; import { DocumentType, verifyDocumentType } from '../parser'; @@ -43,7 +44,7 @@ export function useQuery< TVariables extends OperationVariables = OperationVariables, >( query: DocumentNode | TypedDocumentNode, - options: QueryHookOptions = Object.create(null), + options: QueryHookOptions, NoInfer> = Object.create(null), ): QueryResult { return useInternalState( useApolloClient(options.client), diff --git a/src/react/types/types.ts b/src/react/types/types.ts index cdff55c382d..5eb41637c81 100644 --- a/src/react/types/types.ts +++ b/src/react/types/types.ts @@ -307,3 +307,30 @@ export interface SubscriptionCurrentObservable { query?: Observable; subscription?: ObservableSubscription; } + +/** +Helper type that allows using a type in a way that cannot be "widened" by inference on the value it is used on. + +This type was first suggested [in this Github discussion](https://github.com/microsoft/TypeScript/issues/14829#issuecomment-504042546). + +Example usage: +```ts +export function useQuery< + TData = any, + TVariables extends OperationVariables = OperationVariables, +>( + query: DocumentNode | TypedDocumentNode, + options: QueryHookOptions, NoInfer> = Object.create(null), +) +``` +In this case, `TData` and `TVariables` should be inferred from `query`, but never widened from something in `options`. + +So, in this code example: +```ts +declare const typedNode: TypedDocumentNode<{ foo: string}, { bar: number }> +const { variables } = useQuery(typedNode, { variables: { bar: 4, nonExistingVariable: "string" } }); +``` +Without the use of `NoInfer`, `variables` would now be of the type `{ bar: number, nonExistingVariable: "string" }`. +With `NoInfer`, it will instead give an error on `nonExistingVariable`. + */ +export type NoInfer = [T][T extends any ? 0 : never] From 9981a692fea49eba7b850bd8ab09847c4b0b1f0b Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Sun, 5 Feb 2023 20:27:13 +0100 Subject: [PATCH 2/3] also add `NoInfer` to `UseFragmentOptions` --- src/react/hooks/__tests__/useFragment.test.tsx | 15 +++++++++++++++ src/react/hooks/useFragment.ts | 4 +++- 2 files changed, 18 insertions(+), 1 deletion(-) diff --git a/src/react/hooks/__tests__/useFragment.test.tsx b/src/react/hooks/__tests__/useFragment.test.tsx index 213f66dc129..4c942593a42 100644 --- a/src/react/hooks/__tests__/useFragment.test.tsx +++ b/src/react/hooks/__tests__/useFragment.test.tsx @@ -915,3 +915,18 @@ describe("useFragment", () => { }); }); }); + +describe.skip("Type Tests", () => { + test('NoInfer prevents adding arbitrary additional variables', () => { + const typedNode = {} as TypedDocumentNode<{ foo: string}, { bar: number }> + useFragment({ + fragment: typedNode, + from: { __typename: "Query" }, + variables: { + bar: 4, + // @ts-expect-error + nonExistingVariable: "string" + } + }); + }) +}) diff --git a/src/react/hooks/useFragment.ts b/src/react/hooks/useFragment.ts index 187a83222aa..9246aaebbf6 100644 --- a/src/react/hooks/useFragment.ts +++ b/src/react/hooks/useFragment.ts @@ -12,10 +12,11 @@ import { import { useApolloClient } from "./useApolloClient"; import { useSyncExternalStore } from "./useSyncExternalStore"; import { OperationVariables } from "../../core"; +import { NoInfer } from "../types/types"; export interface UseFragmentOptions extends Omit< - Cache.DiffOptions, + Cache.DiffOptions, NoInfer>, | "id" | "query" | "optimistic" @@ -23,6 +24,7 @@ extends Omit< >, Omit< Cache.ReadFragmentOptions, | "id" + | "variables" > { from: StoreObject | Reference | string; // Override this field to make it optional (default: true). From e9c1d21b60d9412f5dfafe84570790ff3a9d5b7f Mon Sep 17 00:00:00 2001 From: Lenz Weber Date: Sun, 5 Feb 2023 20:38:18 +0100 Subject: [PATCH 3/3] add `NoInfer` to `useSubscription` --- .../hooks/__tests__/useSubscription.test.tsx | 18 +++++++++++++++++- src/react/hooks/useSubscription.ts | 5 +++-- 2 files changed, 20 insertions(+), 3 deletions(-) diff --git a/src/react/hooks/__tests__/useSubscription.test.tsx b/src/react/hooks/__tests__/useSubscription.test.tsx index 1c208814bef..0c920df3eb7 100644 --- a/src/react/hooks/__tests__/useSubscription.test.tsx +++ b/src/react/hooks/__tests__/useSubscription.test.tsx @@ -2,7 +2,7 @@ import React from 'react'; import { renderHook, waitFor } from '@testing-library/react'; import gql from 'graphql-tag'; -import { ApolloClient, ApolloError, ApolloLink, concat } from '../../../core'; +import { ApolloClient, ApolloError, ApolloLink, concat, TypedDocumentNode } from '../../../core'; import { InMemoryCache as Cache } from '../../../cache'; import { ApolloProvider, resetApolloContext } from '../../context'; import { MockSubscriptionLink } from '../../../testing'; @@ -933,3 +933,19 @@ describe('useSubscription Hook', () => { warningSpy.mockRestore(); }); }); + +describe.skip("Type Tests", () => { + test('NoInfer prevents adding arbitrary additional variables', () => { + const typedNode = {} as TypedDocumentNode<{ foo: string}, { bar: number }> + const { variables } = useSubscription(typedNode, { + variables: { + bar: 4, + // @ts-expect-error + nonExistingVariable: "string" + } + }); + variables?.bar + // @ts-expect-error + variables?.nonExistingVariable + }) +}) diff --git a/src/react/hooks/useSubscription.ts b/src/react/hooks/useSubscription.ts index bce2aa555b6..5f6e7d0c09c 100644 --- a/src/react/hooks/useSubscription.ts +++ b/src/react/hooks/useSubscription.ts @@ -7,6 +7,7 @@ import { equal } from '@wry/equality'; import { DocumentType, verifyDocumentType } from '../parser'; import { + NoInfer, SubscriptionHookOptions, SubscriptionResult } from '../types/types'; @@ -15,12 +16,12 @@ import { useApolloClient } from './useApolloClient'; export function useSubscription( subscription: DocumentNode | TypedDocumentNode, - options?: SubscriptionHookOptions, + options?: SubscriptionHookOptions, NoInfer>, ) { const hasIssuedDeprecationWarningRef = useRef(false); const client = useApolloClient(options?.client); verifyDocumentType(subscription, DocumentType.Subscription); - const [result, setResult] = useState>({ + const [result, setResult] = useState>({ loading: !options?.skip, error: void 0, data: void 0,