Compatibility with "classic" and "modern" signatures

[PowerKiKi]

@jerelmiller (cc @phryneas), I would like to make apollo-angular compatible with the latest changes of @apollo/client 4.2 that introduced "classic" and "modern" signatures. However, I am not quite sure how to proceed, and since you contributed very heavily to the new-ish typing structure of apollo-angular during the migration to Apollo v4, I hoped you might have a few advice on the matter ?

More specifically, I don't think we can use the types you introduced such as Apollo.QueryResult (vs ApolloClient.QueryResult) as-is anymore. And instead we should somewhat rely on the the ApolloClient.query.Signature, but I am not sure what would the best way to do that.

I am willing to break everything and make a major release if needed. I am even considering dropping most features, so that apollo-angular becomes an extremely thin wrapper that monkey-patch @apollo/client, in order to rely 100% on @apollo/client types instead of introducing an "extra layer" of types... but hopefully you have better comprehension of @apollo/client and you can guide me toward a saner solution 😅

[phryneas]

So if you really wanted to use our signatures, I believe you're in for a really big rewrite.

I'd not go that far, tbh.
If you're willing to be breaking and doing a major, to me the best option would probably to just go "all in" and not support "old signatures/new signatures", but just the new signatures.

At that point, you'd be looking at changes like this:

diff --git a/packages/apollo-angular/src/query.ts b/packages/apollo-angular/src/query.ts
index 4b165a6f..1a899245 100644
--- a/packages/apollo-angular/src/query.ts
+++ b/packages/apollo-angular/src/query.ts
@@ -1,10 +1,10 @@
 import type { DocumentNode } from 'graphql';
-import type { Observable } from 'rxjs';
 import { Injectable } from '@angular/core';
-import type { OperationVariables, TypedDocumentNode } from '@apollo/client';
+import type { ApolloClient, OperationVariables, TypedDocumentNode } from '@apollo/client';
 import { Apollo } from './apollo';
 import { QueryRef } from './query-ref';
 import { EmptyObject } from './types';
+import { VariablesOption } from '@apollo/client/utilities/internal/internal.cjs';
 
 export declare namespace Query {
   export type WatchOptions<
@@ -36,14 +36,28 @@ export abstract class Query<TData = unknown, TVariables extends OperationVariabl
     } as Apollo.WatchQueryOptions<TData, TVariables>);
   }
 
-  public fetch(
-    ...[options]: {} extends TVariables
-      ? [options?: Query.FetchOptions<TData, TVariables>]
-      : [options: Query.FetchOptions<TData, TVariables>]
-  ): Observable<Apollo.QueryResult<TData>> {
-    return this.apollo.use(this.client).query<TData, TVariables>({
+  public fetch<
+          // this overload should never be manually defined, it should always be inferred
+          TOptions extends ApolloClient.QueryOptions<
+            NoInfer<TData>,
+            NoInfer<TVariables>
+          > &
+            VariablesOption<
+              TVariables & {
+                [K in Exclude<
+                  keyof TOptions["variables"],
+                  keyof TVariables
+                >]?: never;
+              }
+            >,
+        >(
+          options: TOptions
+        ): Promise<
+          ApolloClient.query.ResultForOptions<TData, TVariables, TOptions>
+        > {
+    return this.apollo.use(this.client).query({
       ...options,
       query: this.document,
-    } as Apollo.QueryOptions<TData, TVariables>);
+    });
   }
 }

[phryneas]

Maybe with a second signature with options optional if {} extends TVariables, and otherwise casting options to never, forcing TS onto the next overload.

[jerelmiller]

I second this. We wanted to give most users an off-ramp from the useQuery<Data, Vars>() style APIs and enforce inference, hence why we had to support both modes and why its insanely complex. A major in Apollo Angular to flat out drop support for explicit generic args would make it much simpler for you, so definitely do that!

[PowerKiKi]

That makes a lot of sense to jump straight to modern signature only. Thanks for the idea. I gave it a try, starting with @phryneas example, but I quickly got stuck. Typings are super nice when they work, but I find them difficult to reason about when they are broken :-/

So I'll give up for now, in order to focus on other priorities. In the meantime, if somebody attempted to create a PR, I'd be very grateful.

[jerelmiller]

If I get some time, I'd be happy to take a crack at it. I can't guarantee anything right away, but will try my best to get to it as soon as I have a free gap of time.