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

Improve useBackgroundQuery type interface and add type tests #10951

Merged
merged 7 commits into from
Jun 9, 2023
Merged
Show file tree
Hide file tree
Changes from 4 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/friendly-mugs-repeat.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
'@apollo/client': patch
---

Improve `useBackgroundQuery` type interface
225 changes: 180 additions & 45 deletions src/react/hooks/__tests__/useBackgroundQuery.test.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
} from '@testing-library/react';
import userEvent from '@testing-library/user-event';
import { ErrorBoundary, ErrorBoundaryProps } from 'react-error-boundary';
import { expectTypeOf } from 'expect-type';
import { GraphQLError } from 'graphql';
import {
gql,
Expand Down Expand Up @@ -38,8 +39,11 @@ import { useBackgroundQuery, useReadQuery } from '../useBackgroundQuery';
import { ApolloProvider } from '../../context';
import { SuspenseCache } from '../../cache';
import { InMemoryCache } from '../../../cache';
import { FetchMoreFunction } from '../../../react';
import { QueryReference } from '../../cache/QueryReference';
import {
FetchMoreFunction,
RefetchFunction,
QueryReference,
} from '../../../react';

function renderIntegrationTest({
client,
Expand Down Expand Up @@ -131,6 +135,37 @@ function renderIntegrationTest({
return { ...rest, query, client: _client, renders };
}

interface VariablesCaseData {
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lifted VariablesCaseData/VariablesCaseVariables up to the file scope to make type tests easier to write, a nice pattern borrowed from the useSuspenseQuery tests

character: {
id: string;
name: string;
};
}

interface VariablesCaseVariables {
id: string;
}

function useVariablesIntegrationTestCase() {
const query: TypedDocumentNode<
VariablesCaseData,
VariablesCaseVariables
> = gql`
query CharacterQuery($id: ID!) {
character(id: $id) {
id
name
}
}
`;
const CHARACTERS = ['Spider-Man', 'Black Widow', 'Iron Man', 'Hulk'];
let mocks = [...CHARACTERS].map((name, index) => ({
request: { query, variables: { id: String(index + 1) } },
result: { data: { character: { id: String(index + 1), name } } },
}));
return { mocks, query };
}

function renderVariablesIntegrationTest({
variables,
mocks,
Expand All @@ -150,32 +185,8 @@ function renderVariablesIntegrationTest({
variables: { id: string };
errorPolicy?: ErrorPolicy;
}) {
const CHARACTERS = ['Spider-Man', 'Black Widow', 'Iron Man', 'Hulk'];
let { mocks: _mocks, query } = useVariablesIntegrationTestCase();

interface QueryData {
character: {
id: string;
name: string;
};
}

interface QueryVariables {
id: string;
}

const query: TypedDocumentNode<QueryData, QueryVariables> = gql`
query CharacterQuery($id: ID!) {
character(id: $id) {
id
name
}
}
`;

let _mocks = [...CHARACTERS].map((name, index) => ({
request: { query, variables: { id: String(index + 1) } },
result: { data: { character: { id: String(index + 1), name } } },
}));
// duplicate mocks with (updated) in the name for refetches
_mocks = [..._mocks, ..._mocks, ..._mocks].map(
({ request, result }, index) => {
Expand Down Expand Up @@ -208,7 +219,7 @@ function renderVariablesIntegrationTest({
suspenseCount: number;
count: number;
frames: {
data: QueryData;
data: VariablesCaseData;
networkStatus: NetworkStatus;
error: ApolloError | undefined;
}[];
Expand Down Expand Up @@ -239,11 +250,11 @@ function renderVariablesIntegrationTest({
variables: _variables,
queryRef,
}: {
variables: QueryVariables;
variables: VariablesCaseVariables;
refetch: (
variables?: Partial<OperationVariables> | undefined
) => Promise<ApolloQueryResult<QueryData>>;
queryRef: QueryReference<QueryData>;
) => Promise<ApolloQueryResult<VariablesCaseData>>;
queryRef: QueryReference<VariablesCaseData>;
}) {
const { data, error, networkStatus } = useReadQuery(queryRef);
const [variables, setVariables] = React.useState(_variables);
Expand Down Expand Up @@ -276,7 +287,7 @@ function renderVariablesIntegrationTest({
variables,
errorPolicy = 'none',
}: {
variables: QueryVariables;
variables: VariablesCaseVariables;
errorPolicy?: ErrorPolicy;
}) {
const [queryRef, { refetch }] = useBackgroundQuery(query, {
Expand All @@ -294,7 +305,7 @@ function renderVariablesIntegrationTest({
variables,
errorPolicy,
}: {
variables: QueryVariables;
variables: VariablesCaseVariables;
errorPolicy?: ErrorPolicy;
}) {
return (
Expand All @@ -314,7 +325,7 @@ function renderVariablesIntegrationTest({
const { ...rest } = render(
<App errorPolicy={errorPolicy} variables={variables} />
);
const rerender = ({ variables }: { variables: QueryVariables }) => {
const rerender = ({ variables }: { variables: VariablesCaseVariables }) => {
return rest.rerender(<App variables={variables} />);
};
return { ...rest, query, rerender, client, renders };
Expand Down Expand Up @@ -1243,7 +1254,15 @@ describe('useBackgroundQuery', () => {
});

it('does not suspend deferred queries with data in the cache and using a "cache-and-network" fetch policy', async () => {
const query = gql`
interface Data {
greeting: {
__typename: string;
message: string;
recipient: { name: string; __typename: string };
};
}

const query: TypedDocumentNode<Data> = gql`
query {
greeting {
message
Expand All @@ -1256,13 +1275,6 @@ describe('useBackgroundQuery', () => {
}
`;

interface Data {
greeting: {
message: string;
recipient: { name: string };
};
}

const link = new MockSubscriptionLink();
const cache = new InMemoryCache();
cache.writeQuery({
Expand Down Expand Up @@ -1875,9 +1887,7 @@ describe('useBackgroundQuery', () => {
queryRef,
refetch,
}: {
refetch: (
variables?: Partial<OperationVariables> | undefined
) => Promise<ApolloQueryResult<Data>>;
refetch: RefetchFunction<Data, OperationVariables>;
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

refetch type is much cleaner now ✨

queryRef: QueryReference<Data>;
onChange: (id: string) => void;
}) {
Expand Down Expand Up @@ -2199,11 +2209,136 @@ describe('useBackgroundQuery', () => {
// @ts-expect-error should not allow returnPartialData in options
useBackgroundQuery(query, { returnPartialData: true });
});

it('disallows refetchWritePolicy in BackgroundQueryHookOptions', () => {
const { query } = renderIntegrationTest();

// @ts-expect-error should not allow refetchWritePolicy in options
useBackgroundQuery(query, { refetchWritePolicy: 'overwrite' });
});

it('returns unknown when TData cannot be inferred', () => {
const query = gql`
query {
hello
}
`;

const [queryRef] = useBackgroundQuery(query);
const { data } = useReadQuery(queryRef);

expectTypeOf(data).toEqualTypeOf<unknown>();
});

it('disallows wider variables type than specified', () => {
const { query } = useVariablesIntegrationTestCase();

// @ts-expect-error should not allow wider TVariables type
useBackgroundQuery(query, { variables: { id: '1', foo: 'bar' } });
});

it('returns TData in default case', () => {
const { query } = useVariablesIntegrationTestCase();

const [inferredQueryRef] = useBackgroundQuery(query);
const { data: inferred } = useReadQuery(inferredQueryRef);

expectTypeOf(inferred).toEqualTypeOf<VariablesCaseData>();
expectTypeOf(inferred).not.toEqualTypeOf<VariablesCaseData | undefined>();

const [explicitQueryRef] = useBackgroundQuery<
VariablesCaseData,
VariablesCaseVariables
>(query);

const { data: explicit } = useReadQuery(explicitQueryRef);

expectTypeOf(explicit).toEqualTypeOf<VariablesCaseData>();
expectTypeOf(explicit).not.toEqualTypeOf<VariablesCaseData | undefined>();
});

it('returns TData | undefined with errorPolicy: "ignore"', () => {
const { query } = useVariablesIntegrationTestCase();

const [inferredQueryRef] = useBackgroundQuery(query, {
errorPolicy: 'ignore',
});
const { data: inferred } = useReadQuery(inferredQueryRef);

expectTypeOf(inferred).toEqualTypeOf<VariablesCaseData | undefined>();
expectTypeOf(inferred).not.toEqualTypeOf<VariablesCaseData>();

const [explicitQueryRef] = useBackgroundQuery<
VariablesCaseData,
VariablesCaseVariables
>(query, {
errorPolicy: 'ignore',
});

const { data: explicit } = useReadQuery(explicitQueryRef);

expectTypeOf(explicit).toEqualTypeOf<VariablesCaseData | undefined>();
expectTypeOf(explicit).not.toEqualTypeOf<VariablesCaseData>();
});

it('returns TData | undefined with errorPolicy: "all"', () => {
const { query } = useVariablesIntegrationTestCase();

const [inferredQueryRef] = useBackgroundQuery(query, {
errorPolicy: 'all',
});
const { data: inferred } = useReadQuery(inferredQueryRef);

expectTypeOf(inferred).toEqualTypeOf<VariablesCaseData | undefined>();
expectTypeOf(inferred).not.toEqualTypeOf<VariablesCaseData>();

const [explicitQueryRef] = useBackgroundQuery(query, {
errorPolicy: 'all',
});
const { data: explicit } = useReadQuery(explicitQueryRef);

expectTypeOf(explicit).toEqualTypeOf<VariablesCaseData | undefined>();
expectTypeOf(explicit).not.toEqualTypeOf<VariablesCaseData>();
});

it('returns TData with errorPolicy: "none"', () => {
const { query } = useVariablesIntegrationTestCase();

const [inferredQueryRef] = useBackgroundQuery(query, {
errorPolicy: 'none',
});
const { data: inferred } = useReadQuery(inferredQueryRef);

expectTypeOf(inferred).toEqualTypeOf<VariablesCaseData>();
expectTypeOf(inferred).not.toEqualTypeOf<VariablesCaseData | undefined>();

const [explicitQueryRef] = useBackgroundQuery(query, {
errorPolicy: 'none',
});
const { data: explicit } = useReadQuery(explicitQueryRef);

expectTypeOf(explicit).toEqualTypeOf<VariablesCaseData>();
expectTypeOf(explicit).not.toEqualTypeOf<VariablesCaseData | undefined>();
});

// TODO: https://github.com/apollographql/apollo-client/issues/10893
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The remaining five type tests from useSuspenseQuery have references to returnPartialData and will be completed as part of the work in #10893

// it('returns DeepPartial<TData> with returnPartialData: true', () => {
// });

// TODO: https://github.com/apollographql/apollo-client/issues/10893
// it('returns TData with returnPartialData: false', () => {
// });

// TODO: https://github.com/apollographql/apollo-client/issues/10893
// it('returns TData when passing an option that does not affect TData', () => {
// });

// TODO: https://github.com/apollographql/apollo-client/issues/10893
// it('handles combinations of options', () => {
// });

// TODO: https://github.com/apollographql/apollo-client/issues/10893
// it('returns correct TData type when combined options that do not affect TData', () => {
// });
});
});
Loading