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

prevent accidental widening of inferred TData and TVariables generics for query hook option arguments #10506

Merged
merged 4 commits into from
Feb 15, 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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
5 changes: 5 additions & 0 deletions .changeset/hungry-eagles-kick.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@apollo/client': minor
---

prevent accidental widening of inferred TData and TVariables generics for query hook option arguments
15 changes: 15 additions & 0 deletions src/react/hooks/__tests__/useFragment.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
});
})
})
16 changes: 16 additions & 0 deletions src/react/hooks/__tests__/useLazyQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -1328,3 +1328,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
})
})
13 changes: 13 additions & 0 deletions src/react/hooks/__tests__/useMutation.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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"
}
});
})
})
16 changes: 16 additions & 0 deletions src/react/hooks/__tests__/useQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
})
18 changes: 17 additions & 1 deletion src/react/hooks/__tests__/useSubscription.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down Expand Up @@ -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
})
})
4 changes: 3 additions & 1 deletion src/react/hooks/useFragment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,19 @@ import {
import { useApolloClient } from "./useApolloClient";
import { useSyncExternalStore } from "./useSyncExternalStore";
import { OperationVariables } from "../../core";
import { NoInfer } from "../types/types";

export interface UseFragmentOptions<TData, TVars>
extends Omit<
Cache.DiffOptions<TData, TVars>,
Cache.DiffOptions<NoInfer<TData>, NoInfer<TVars>>,
| "id"
| "query"
| "optimistic"
| "previousResult"
>, Omit<
Cache.ReadFragmentOptions<TData, TVars>,
| "id"
| "variables"
> {
from: StoreObject | Reference | string;
// Override this field to make it optional (default: true).
Expand Down
3 changes: 2 additions & 1 deletion src/react/hooks/useLazyQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { mergeOptions } from '../../utilities';
import {
LazyQueryHookOptions,
LazyQueryResultTuple,
NoInfer,
QueryResult,
} from '../types/types';
import { useInternalState } from './useQuery';
Expand All @@ -25,7 +26,7 @@ const EAGER_METHODS = [

export function useLazyQuery<TData = any, TVariables extends OperationVariables = OperationVariables>(
query: DocumentNode | TypedDocumentNode<TData, TVariables>,
options?: LazyQueryHookOptions<TData, TVariables>
options?: LazyQueryHookOptions<NoInfer<TData>, NoInfer<TVariables>>
): LazyQueryResultTuple<TData, TVariables> {
const abortControllersRef = useRef(new Set<AbortController>());

Expand Down
3 changes: 2 additions & 1 deletion src/react/hooks/useMutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import {
MutationHookOptions,
MutationResult,
MutationTuple,
NoInfer,
} from '../types/types';

import {
Expand All @@ -26,7 +27,7 @@ export function useMutation<
TCache extends ApolloCache<any> = ApolloCache<any>,
>(
mutation: DocumentNode | TypedDocumentNode<TData, TVariables>,
options?: MutationHookOptions<TData, TVariables, TContext, TCache>,
options?: MutationHookOptions<NoInfer<TData>, NoInfer<TVariables>, TContext, TCache>,
): MutationTuple<TData, TVariables, TContext, TCache> {
const client = useApolloClient(options?.client);
verifyDocumentType(mutation, DocumentType.Mutation);
Expand Down
3 changes: 2 additions & 1 deletion src/react/hooks/useQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,7 @@ import {
QueryHookOptions,
QueryResult,
ObservableQueryFields,
NoInfer,
} from '../types/types';

import { DocumentType, verifyDocumentType } from '../parser';
Expand All @@ -43,7 +44,7 @@ export function useQuery<
TVariables extends OperationVariables = OperationVariables,
>(
query: DocumentNode | TypedDocumentNode<TData, TVariables>,
options: QueryHookOptions<TData, TVariables> = Object.create(null),
options: QueryHookOptions<NoInfer<TData>, NoInfer<TVariables>> = Object.create(null),
): QueryResult<TData, TVariables> {
return useInternalState(
useApolloClient(options.client),
Expand Down
5 changes: 3 additions & 2 deletions src/react/hooks/useSubscription.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import { equal } from '@wry/equality';

import { DocumentType, verifyDocumentType } from '../parser';
import {
NoInfer,
SubscriptionHookOptions,
SubscriptionResult
} from '../types/types';
Expand All @@ -14,12 +15,12 @@ import { useApolloClient } from './useApolloClient';

export function useSubscription<TData = any, TVariables extends OperationVariables = OperationVariables>(
subscription: DocumentNode | TypedDocumentNode<TData, TVariables>,
options?: SubscriptionHookOptions<TData, TVariables>,
options?: SubscriptionHookOptions<NoInfer<TData>, NoInfer<TVariables>>,
) {
const hasIssuedDeprecationWarningRef = useRef(false);
const client = useApolloClient(options?.client);
verifyDocumentType(subscription, DocumentType.Subscription);
const [result, setResult] = useState<SubscriptionResult<TData>>({
const [result, setResult] = useState<SubscriptionResult<TData, TVariables>>({
loading: !options?.skip,
error: void 0,
data: void 0,
Expand Down
27 changes: 27 additions & 0 deletions src/react/types/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -307,3 +307,30 @@ export interface SubscriptionCurrentObservable {
query?: Observable<any>;
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<TData, TVariables>,
options: QueryHookOptions<NoInfer<TData>, NoInfer<TVariables>> = 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][T extends any ? 0 : never]