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

Enable strict mode in tsconfig and fix type errors #11200

Merged
merged 33 commits into from
Nov 10, 2023
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
33 commits
Select commit Hold shift + click to select a range
aca904f
Enable strict mode in tsconfig.json
jerelmiller Sep 7, 2023
0da3d3d
Fix type issues in utilities/graphql/transform
jerelmiller Sep 7, 2023
78ccf61
Use spread to push incoming objects in offsetLimitPagination
jerelmiller Sep 7, 2023
69d528a
Fix type issues in writeToStore test
jerelmiller Sep 7, 2023
e503cbb
Fix type issues in Concast
jerelmiller Sep 7, 2023
0fe49a0
Remove unused filterInPlace utility
jerelmiller Sep 7, 2023
e34d084
Add this type to itAsync
jerelmiller Sep 7, 2023
962bd47
Add ! annotation to mockLink
jerelmiller Sep 7, 2023
36129f8
Fix assignment from unknown in error link
jerelmiller Sep 7, 2023
cea054e
Fix type issue in readFromStore
jerelmiller Sep 7, 2023
bed3439
Fix type issues in useQuery
jerelmiller Sep 7, 2023
b568689
some progress
phryneas Sep 8, 2023
6498326
more type fixes
phryneas Sep 13, 2023
b8c6eb6
change `addExportedVariables` signature
phryneas Sep 13, 2023
c3a761d
better typing
phryneas Sep 13, 2023
9eb5e3e
undo deprecation
phryneas Sep 13, 2023
0966cc9
Merge branch 'main' into enable-strict-mode
phryneas Sep 13, 2023
a101977
test types
phryneas Sep 13, 2023
55a6351
add api extractor
phryneas Sep 14, 2023
f909921
add api reports
phryneas Sep 14, 2023
e1e2fbb
add workflow
phryneas Sep 14, 2023
0426c84
formatting
phryneas Sep 14, 2023
be151b0
package lock
phryneas Sep 14, 2023
c83b042
remove `ensureResult` for this PR, add `TODO` type
phryneas Sep 14, 2023
4aa485b
Merge branch 'pr/api-extractor' into enable-strict-mode
phryneas Sep 14, 2023
1e7c5c6
API updates for this PR
phryneas Sep 14, 2023
c080309
Merge remote-tracking branch 'origin/main' into enable-strict-mode
phryneas Nov 10, 2023
0b2cf25
size-limit
phryneas Nov 10, 2023
9303630
introduce IDE-only `tsconfig.json`
phryneas Nov 10, 2023
5e3fb70
formatting
phryneas Nov 10, 2023
04dcc0d
fix path
phryneas Nov 10, 2023
2cb5c83
Merge pull request #11359 from apollographql/pr/enable-test-ts
jerelmiller Nov 10, 2023
94b07f9
Add changeset
jerelmiller Nov 10, 2023
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
Prev Previous commit
Next Next commit
more type fixes
  • Loading branch information
phryneas committed Sep 13, 2023
commit 64983263216c51c0b9e0aa66a104043e6e15cb87
6 changes: 5 additions & 1 deletion src/cache/inmemory/entityStore.ts
Original file line number Diff line number Diff line change
Expand Up @@ -755,7 +755,11 @@ class Layer extends EntityStore {
public getStorage(): StorageType {
let p: EntityStore = this.parent;
while ((p as Layer).parent) p = (p as Layer).parent;
return p.getStorage.apply(p, arguments);
return p.getStorage.apply(
p,
// @ts-expect-error
arguments
);
}
}

Expand Down
18 changes: 11 additions & 7 deletions src/core/ObservableQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -929,13 +929,7 @@ Did you mean to call refetch(variables) instead of refetch({ variables })?`,
newNetworkStatus?: NetworkStatus
) {
return this.reobserveAsConcast(newOptions, newNetworkStatus).promise.then(
(value) => {
invariant(
value,
"A Concast finished without a result. This in an Apollo Client bug, please file a bug report."
);
return value;
}
ensureResult
);
}

Expand Down Expand Up @@ -1102,3 +1096,13 @@ function skipCacheDataFor(
fetchPolicy === "standby"
);
}

export function ensureResult<TData = any>(
value: ApolloQueryResult<TData> | undefined
): ApolloQueryResult<TData> {
invariant(
value,
"A Concast finished without a result. This in an Apollo Client bug, please file a bug report."
);
return value;
}
7 changes: 4 additions & 3 deletions src/core/QueryInfo.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ function wrapDestructiveCacheMethod(
// that matters in any conceivable practical scenario.
(destructiveMethodCounts.get(cache)! + 1) % 1e15
);
// @ts-expect-error this is just too generic to be typed correctly
return original.apply(this, arguments);
};
}
Expand Down Expand Up @@ -113,7 +114,7 @@ export class QueryInfo {
// NetworkStatus.loading, but also possibly fetchMore, poll, refetch,
// or setVariables.
networkStatus?: NetworkStatus;
observableQuery?: ObservableQuery<any>;
observableQuery?: ObservableQuery<any, any>;
Copy link
Member

Choose a reason for hiding this comment

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

We are generally very inconsistent with the extends OperationVariables on TVariables, and I have to say I'm not too happy with OperationVariables, as it works with type-declared types, but not with interfaces (as those don't have an implicit index signature).
We might want to look into that - but not in this PR.

lastRequestId?: number;
}): this {
let networkStatus = query.networkStatus || NetworkStatus.loading;
Expand Down Expand Up @@ -214,10 +215,10 @@ export class QueryInfo {
}
}

public readonly observableQuery: ObservableQuery<any> | null = null;
public readonly observableQuery: ObservableQuery<any, any> | null = null;
private oqListener?: QueryListener;

setObservableQuery(oq: ObservableQuery<any> | null) {
setObservableQuery(oq: ObservableQuery<any, any> | null) {
if (oq === this.observableQuery) return;

if (this.oqListener) {
Expand Down
19 changes: 13 additions & 6 deletions src/core/QueryManager.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,11 @@ import type {
ErrorPolicy,
MutationFetchPolicy,
} from "./watchQueryOptions.js";
import { ObservableQuery, logMissingFieldErrors } from "./ObservableQuery.js";
import {
ObservableQuery,
ensureResult,
logMissingFieldErrors,
} from "./ObservableQuery.js";
import { NetworkStatus, isNetworkRequestInFlight } from "./networkStatus.js";
import type {
ApolloQueryResult,
Expand Down Expand Up @@ -479,7 +483,7 @@ export class QueryManager<TStore> {
const results: any[] = [];

this.refetchQueries({
updateCache: (cache: TCache) => {
updateCache: (cache) => {
if (!skipCache) {
cacheWrites.forEach((write) => cache.write(write));
}
Expand Down Expand Up @@ -526,7 +530,7 @@ export class QueryManager<TStore> {
// either a SingleExecutionResult or the final ExecutionPatchResult,
// call the update function.
if (isFinalResult) {
update(cache, result, {
update(cache as TCache, result, {
Copy link
Member

Choose a reason for hiding this comment

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

the TCache generic argument here is essentially an any-cast. I'm not super happy about that, but on the other hand, realistically all the generic arguments on markMutationResult are any-casts

context: mutation.context,
variables: mutation.variables,
});
Expand Down Expand Up @@ -616,8 +620,11 @@ export class QueryManager<TStore> {
options: WatchQueryOptions<TVars, TData>,
networkStatus?: NetworkStatus
): Promise<ApolloQueryResult<TData>> {
return this.fetchConcastWithInfo(queryId, options, networkStatus).concast
.promise;
return this.fetchConcastWithInfo(
queryId,
options,
networkStatus
).concast.promise.then(ensureResult);
}

public getQueryStore() {
Expand Down Expand Up @@ -1301,7 +1308,7 @@ export class QueryManager<TStore> {
normalized.variables,
normalized.context
)
.then(fromVariables)
.then(fromVariables as (Variables: any) => SourcesAndInfo<TData>)
.then((sourcesWithInfo) => sourcesWithInfo.sources)
);
// there is just no way we can synchronously get the *right* value here,
Expand Down
2 changes: 0 additions & 2 deletions src/link/http/HttpLink.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,8 @@
import type { RequestHandler } from "../core/index.js";
import { ApolloLink } from "../core/index.js";
import type { HttpOptions } from "./selectHttpOptionsAndBody.js";
import { createHttpLink } from "./createHttpLink.js";

export class HttpLink extends ApolloLink {
public requester: RequestHandler;
constructor(public options: HttpOptions = {}) {
Comment on lines 5 to 6
Copy link
Member

Choose a reason for hiding this comment

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

requester appears in the whole code base only here and it doesn't seem to be set or read anywhere - it's also not there at runtime. I guess this can be removed?

super(createHttpLink(options).request);
}
Expand Down
14 changes: 11 additions & 3 deletions src/link/http/iterators/reader.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@
import { canUseAsyncIteratorSymbol } from "../../../utilities/index.js";

interface ReaderIterator<T> {
next(): Promise<ReadableStreamReadResult<T>>;
next(): Promise<IteratorResult<T, T | undefined>>;
[Symbol.asyncIterator]?(): AsyncIterator<T>;
}

Expand All @@ -15,12 +15,20 @@ export default function readerIterator<T>(
): AsyncIterableIterator<T> {
const iterator: ReaderIterator<T> = {
next() {
return reader.read();
return reader.read() as Promise<
| ReadableStreamReadValueResult<T>
// DoneResult has `value` optional, which doesn't comply with an
// `IteratorResult`, so we assert it to `T | undefined` instead
| Required<ReadableStreamReadDoneResult<T | undefined>>
>;
},
};

if (canUseAsyncIteratorSymbol) {
iterator[Symbol.asyncIterator] = function (): AsyncIterator<T> {
iterator[Symbol.asyncIterator] = function (): AsyncIterator<
T,
T | undefined
> {
return this;
};
}
Expand Down
2 changes: 1 addition & 1 deletion src/link/http/serializeFetchParameter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ export const serializeFetchParameter = (p: any, label: string) => {
let serialized;
try {
serialized = JSON.stringify(p);
} catch (e) {
} catch (e: any) {
const parseError = newInvariantError(
`Network request failed. %s is not serializable: %s`,
label,
Expand Down
6 changes: 4 additions & 2 deletions src/react/cache/SuspenseCache.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,9 @@ export class SuspenseCache {
cacheKey: CacheKey,
createObservable: () => ObservableQuery<TData>
) {
const ref = this.queryRefs.lookupArray(cacheKey);
const ref = this.queryRefs.lookupArray(cacheKey) as {
current?: InternalQueryReference<TData>;
};

if (!ref.current) {
ref.current = new InternalQueryReference(createObservable(), {
Expand All @@ -44,6 +46,6 @@ export class SuspenseCache {
});
}

return ref.current as InternalQueryReference<TData>;
return ref.current;
}
}
1 change: 1 addition & 0 deletions src/react/hoc/mutation-hoc.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ export function withMutation<

return (
<Mutation ignoreResults {...opts} mutation={document}>
{/* @ts-expect-error */}
Copy link
Member

Choose a reason for hiding this comment

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

not gonna fix types in the old HOCs...

{(
mutate: MutationFunction<TData, TGraphQLVariables>,
{ data, ...r }: MutationResult<TData>
Expand Down
6 changes: 4 additions & 2 deletions src/react/hooks/useBackgroundQuery.ts
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import * as React from "react";
import type {
DocumentNode,
FetchMoreQueryOptions,
OperationVariables,
TypedDocumentNode,
WatchQueryOptions,
} from "../../core/index.js";
import { useApolloClient } from "./useApolloClient.js";
import { wrapQueryRef } from "../cache/QueryReference.js";
Expand Down Expand Up @@ -197,7 +199,7 @@ export function useBackgroundQuery<
];

const queryRef = suspenseCache.getQueryRef(cacheKey, () =>
client.watchQuery(watchQueryOptions)
client.watchQuery(watchQueryOptions as WatchQueryOptions<any, any>)
);

const [promiseCache, setPromiseCache] = React.useState(
Expand All @@ -213,7 +215,7 @@ export function useBackgroundQuery<

const fetchMore: FetchMoreFunction<TData, TVariables> = React.useCallback(
(options) => {
const promise = queryRef.fetchMore(options);
const promise = queryRef.fetchMore(options as FetchMoreQueryOptions<any>);

setPromiseCache((promiseCache) =>
new Map(promiseCache).set(queryRef.key, queryRef.promise)
Expand Down
1 change: 1 addition & 0 deletions src/react/hooks/useLazyQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ export function useLazyQuery<
// Only the first time populating execOptionsRef.current matters here.
internalState.forceUpdateState();
}
// @ts-expect-error this is just too generic to type
return method.apply(this, arguments);
};
}
Expand Down
20 changes: 15 additions & 5 deletions src/react/hooks/useMutation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import type {
import type {
ApolloCache,
DefaultContext,
MutationOptions,
OperationVariables,
} from "../../core/index.js";
import { mergeOptions } from "../../utilities/index.js";
Expand Down Expand Up @@ -87,10 +88,10 @@ export function useMutation<
}

const mutationId = ++ref.current.mutationId;
const clientOptions = mergeOptions(baseOptions, executeOptions as any);
const clientOptions = mergeOptions(baseOptions, executeOptions);

return client
.mutate(clientOptions)
.mutate(clientOptions as MutationOptions<TData, OperationVariables>)
.then((response) => {
const { data, errors } = response;
const error =
Expand All @@ -102,7 +103,10 @@ export function useMutation<
executeOptions.onError || ref.current.options?.onError;

if (error && onError) {
onError(error, clientOptions);
onError(
error,
clientOptions as MutationOptions<TData, OperationVariables>
);
Comment on lines +106 to +109
Copy link
Member

Choose a reason for hiding this comment

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

We might want to look closer into these, but for now I'm just casting them around..

}

if (
Expand All @@ -126,7 +130,10 @@ export function useMutation<
executeOptions.onCompleted || ref.current.options?.onCompleted;

if (!error) {
onCompleted?.(response.data!, clientOptions);
onCompleted?.(
response.data!,
clientOptions as MutationOptions<TData, OperationVariables>
);
}

return response;
Expand All @@ -150,7 +157,10 @@ export function useMutation<
executeOptions.onError || ref.current.options?.onError;

if (onError) {
onError(error, clientOptions);
onError(
error,
clientOptions as MutationOptions<TData, OperationVariables>
);

// TODO(brian): why are we returning this here???
return { data: void 0, errors: error };
Expand Down
41 changes: 24 additions & 17 deletions src/react/hooks/useSuspenseQuery.ts
Original file line number Diff line number Diff line change
Expand Up @@ -189,7 +189,7 @@ export function useSuspenseQuery<
];

const queryRef = suspenseCache.getQueryRef(cacheKey, () =>
client.watchQuery(watchQueryOptions)
client.watchQuery(watchQueryOptions as WatchQueryOptions<any, any>)
);

const [promiseCache, setPromiseCache] = React.useState(
Expand Down Expand Up @@ -236,18 +236,21 @@ export function useSuspenseQuery<

const result = fetchPolicy === "standby" ? skipResult : __use(promise);

const fetchMore: FetchMoreFunction<TData, TVariables> = React.useCallback(
(options) => {
const promise = queryRef.fetchMore(options);
const fetchMore: FetchMoreFunction<TData | undefined, TVariables> =
React.useCallback(
(options) => {
const promise = queryRef.fetchMore(
options as WatchQueryOptions<TVariables, TData | undefined>
);

setPromiseCache((previousPromiseCache) =>
new Map(previousPromiseCache).set(queryRef.key, queryRef.promise)
);
setPromiseCache((previousPromiseCache) =>
new Map(previousPromiseCache).set(queryRef.key, queryRef.promise)
);

return promise;
},
[queryRef]
);
return promise;
},
[queryRef]
);

const refetch: RefetchFunction<TData, TVariables> = React.useCallback(
(variables) => {
Expand All @@ -262,13 +265,17 @@ export function useSuspenseQuery<
[queryRef]
);

const subscribeToMore: SubscribeToMoreFunction<TData, TVariables> =
React.useCallback(
(options) => queryRef.observable.subscribeToMore(options),
[queryRef]
);
const subscribeToMore: SubscribeToMoreFunction<
TData | undefined,
TVariables
> = React.useCallback(
(options) => queryRef.observable.subscribeToMore(options),
[queryRef]
);

return React.useMemo(() => {
return React.useMemo<
UseSuspenseQueryResult<TData | undefined, TVariables>
>(() => {
return {
client,
data: result.data,
Expand Down
2 changes: 1 addition & 1 deletion src/testing/core/mocking/mockSubscriptionLink.ts
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ export interface MockedSubscriptionResult {
export class MockSubscriptionLink extends ApolloLink {
public unsubscribers: any[] = [];
public setups: any[] = [];
public operation: Operation;
public operation?: Operation;

private observers: any[] = [];

Expand Down
3 changes: 1 addition & 2 deletions src/testing/core/withConsoleSpy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,7 @@ function wrapTestFunction(
fn: (...args: any[]) => any,
consoleMethodName: "log" | "warn" | "error"
) {
return function () {
const args = arguments;
return function (this: any, ...args: any[]) {
const spy = jest.spyOn(console, consoleMethodName);
spy.mockImplementation(() => {});
return new Promise((resolve) => {
Expand Down
2 changes: 1 addition & 1 deletion src/testing/internal/profile/profile.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ export function profile<
return render;
},
async takeRender(options: NextRenderOptions = {}) {
let error: { message?: string } | undefined = undefined;
let error: unknown = undefined;
try {
return await Profiled.peekRender({
[_stackTrace]: captureStackTrace(Profiled.takeRender),
Expand Down
Loading