Open PowerKiKi opened 10 months ago
Hello,
I agree that using observables in the library seems like the best plan. I do however have some questions about using apollo-angular together with signals. There are a few things that I'm running into that make it harder to work with than I want it to be, hopefully you have some ideas how to remedy these.
When I want to consume the query results in my component I use the provided toSignal
from Angular. I don't want to check for null values everywhere, so I would like to use { requireSync: true }
with an initialValue.
I tried using useInitialLoading: true
but that isn't sync. That would also give me back data: Data | null
which isn't really what I want.
Now I solve it by adding a startWith({data: initialData, loading: true, errors: undefined })
to the pipe. Before that I have a map operator that removes the queryname property from the data field
map((res: ApolloQueryResult<T>) => ({
...res,
data: res.data.queryname
}))
because having that extra property only gets in the way (especially in the types, because its dynamic). But that's probably there to accommodate the __typename field I guess?
But having to do this for every query is a bit cumbersome. I would love for an initialValue
option in (Watch)QueryOptions that would do the work of the startWith operator above.
Now it might seem a bit weird to have initial data when loading is true, but when you create a signal from your query, I would prefer to not have to null check every computed
that is derived from that data. eg:
query = toSignal(...);
multiplied = computed(() => query().data.selection * 2
instead of
query = toSignal(...)
multiplied = computed(() => (query().data?.selection ?? 1) * 2)
Got any tips op workarounds for how best deal with these situations?
For your use-case I think it would be possible to write your own function that wraps everything you need. Probably something similar to:
function apolloToSignal<T, Q extends keyof T, U>(
query : Observable<ApolloQueryResult<T>,
queryName: Q,
initialValue: U
): Signal<T[Q] | U>
It's only pseudo-code, but it should be possible to make it work.
For the time being I'd probably avoid introducing signal-related things in this package, unless we have a clear and strong need for it.
I'm not sure if that's a real use case or if that would be good architecture. But something like a combination of watchQuery and computed/effect could be helpful. This could look something like this.
query = signal('');
searchResult: Signal<ApolloResponse<SearchResult>> = this.apollo.signalQuery({query: this.query});
So the signalQuery would attach to the signal query
and every new value to the query would set the variables on the watchQuery and this would respond with GraphQL response.
I have an actual use case such as the one you describe. My component has an input signal, and the query uses that signal value on its variables. I am currently using an effect which calls fetch. I would like to be able to use the signal on the query variables for a watchQuery, and to also be able to call refetch on the QueryRef.
@DaSchTour,
every new value to the query would set the variables
I assume you meant "every new value to the variables" ? Because your goal is to react when variables change ? So your prototype would be more like:
const query = gql`query myQuery($myVar: Int)`;
const variables = signal({myVar: 123});
const searchResult: Signal<ApolloResponse<SearchResult>> = this.apollo.signalQuery({query: query, variables: variables});
variables.set({myVar: 456});
This kind of thing, watching variables, does not exist anymore in apollo-angular
. It was removed before v1 landed. And the removal caused some trouble back then.
In our private projects, we use this kind of pattern a lot. It is super convenient to watch variables. So I wouldn't be against re-adding it. But the question is with what kind of API ?
Most often you need to debounce variables, otherwise there is a high chance you send too many XHR in a very short period of time. That sounds more like a job for Rxjs than for signals to me.
Also, this.apollo.signalQuery()
means that an XHR will be sent immediately upon calling that function, because signals cannot be asynchronous. In some cases, where you already have the variables ready, it could be fine. But in our projects we very often setup the observable and subscribe later, when we have the needed variables and when other things around are setup. This would not work anymore.
Firing XHR too soon sounds like big pitfall to me. That's why I still cannot see how an integration with signals would improve the DX.
Also I think we would lose the ability to control when we unsubscribe. While unsubscribing when the component is destroyed can be ok in basic cases, there are a lot of cases where things are not that straightforward. For instance a query inside a service would never be unsubscribed to. That sounds like a huge issue.
@rafagsiqueira,
If I guess your code correctly, that means an XHR is fired every time the variables changes, potentially extremely often, and the previous XHRs are never cancelled. That can quickly end up being a performance issue.
What I think is a more robust solution, and happens to be doable only with Rxjs AFAIK, is the following:
export class FooComponent {
private readonly apollo = inject(Apollo);
public readonly variables = input.required<UsersVariables>();
public readonly result: Observable<ApolloQueryResult<Users>> = toObservable(this.variables).pipe(
debounceTime(20), // Not too often
switchMap( // Cancel previous XHR
variables =>
this.apollo.watchQuery<Users, UsersVariables>({
query: usersQuery,
variables: variables,
}).valueChanges,
),
);
}
I have been trying to reproduce something similar where this.result
would be a Signal, but I actually couldn't. Even if giving up the debounce and switchMap, I am not sure how to do it... Can you convert my sample into something that looks more like your code ?
tld;dr, I still thinks that signals are not the right tool for handling XHR, because the timing of XHR subscription, cancelling and unsubscription (from watching cache) are critical. And the whole reason for signal to exist is to be purely synchronous, and thus making it impossible to control the timing. Unless by doing hacks to re-expose things, but then you'd be re-implementing Rxjs while fighting signal nature.
Does it cancel XHR though? Looking at angular http client they implemented an abort controller when unsubscribing from the source observable. In query we call the Apollo client with a from promise.
Does it cancel XHR though?
Yes, apollo-angular will cancel XHR if you use our HttpLink
, or our HttpBatchLink
. And apollo-client will reject their promise if our link is erroring.
Angular v17 includes a stable API for Signals. I am wondering if that might have an impact for
apollo-angular
or not.So far I would say that because
apollo-angular
is mostly doing XHR, and that XHR must be cancellable, then we must keep using observable. And if somebody would rather work with non-cancellable signal (which does not sound like a great idea to me), then it is up to them to adapt their consuming code. So there is nothing to be done within the lib.But I'd like to hear the thoughts of the community and see if I am maybe missing something ?