No defaultOptions support for subscriptions

[fabis94]

Even on the latest version (Apollo Client v4.07) there's no defaultOptions support for subscriptions:

apollo-client/src/core/ApolloClient.ts

Line 1011 in 231de94

public subscribe<

If you look at mutate() and query(), those do:

apollo-client/src/core/ApolloClient.ts

Line 979 in 231de94

this.defaultOptions.mutate

A very obvious oversight! Wanted to set a custom errorPolicy for all subs, now I can't do this and have to set it manually for all of them...

There was an older issue for this already: #11411
People suggested creating wrappers, but that's horrible DX. Should not have to create wrappers for this!

[phryneas]

Hi @fabis94!

I'm gonna argue in the opposite direction for a moment here - if it were up to me, I'd remove all of the defaultOptions.
We have chosen to keep them in in 4.0 where they already existed in previous versions, but they pose a serious problem with TypeScript and will make your development less safe.

In Apollo Client 4.0, hooks like useQuery have overloads that make them return different results depending on how you call them.

You can play around with that in this playground but it boils down to these two scenarios:

Scenario 1: no default options

const client = new ApolloClient({
  link,
  cache,
});

{
  // usage in a component:
  const result = useQuery(MY_QUERY);
  const data = result.data!;
  //     ^? data: { hello: string; }
  const dataState = result.dataState;
  //     ^? "empty" | "complete" | "streaming"
}
{
  // usage in a component, but with `returnPartialData`:
  const result = useQuery(MY_QUERY, { returnPartialData: true });
  const data = result.data!;
  //     ^? { hello: string; } | DeepPartialObject<{ hello: string; }>
  // note how this now contains `DeepPartial`.
  const dataState = result.dataState;
  //     ^? "empty" | "complete" | "streaming" | "partial"
}

Scenario 2: default options

const client = new ApolloClient({
  link,
  cache,
  defaultOptions: { watchQuery: { returnPartialData: true } },
});

{
  // usage in a component:
  const result = useQuery(MY_QUERY);
  const data = result.data!;
  //     ^? data: { hello: string; }
  const dataState = result.dataState;
  //     ^? "empty" | "complete" | "streaming"

  // this is incorrect now - it *should* be `DeepPartial`, too, but there is no way for TS to figure this out
}

You will notice that the moment you start using defaultOptions, all of that additional information breaks away and the types become incorrect.
Similarly, useSuspenseQuery will start returning wrong types once you start setting an errorPolicy default option.

It's very possible that we will add more of this inference in the future.
I believe that adding more defaultOptions here would in the long run hurt our users more than it would help.

So... I'm not convinced we should add this. I'll bring it up at our next team meeting and we'll discuss it a bit further, but my gut feeling is that we'll likely not be adding this.

[fabis94]

@phryneas I don't really have the full context for what you're talking about, but surely setting default options for operations is an important use case that should be supported as much as possible?

Ultimately doing a force TS type cast (dataState as DeepPartial<...>) in an edge case scenario is a much easier pill for me to swallow than having to duplicate the same setting in tens or hundreds of operation invocations or having to write custom wrappers, which are a PITA typing wise especially when the thing being wrapped has multiple overloads.

[fabis94]

Even just supporting errorPolicy, if not any other default options, would make me happy. I dont think that one changes return types?

[phryneas]

Ultimately doing a force TS type cast (dataState as DeepPartial<...>) in an edge case scenario is a much easier pill for me to swallow than having to duplicate the same setting in tens or hundreds of operation invocations or having to write custom wrappers, which are a PITA typing wise especially when the thing being wrapped has multiple overloads.

My argument here is not that you'd be forced to do a type cast.
You would miss a runtime bug that could otherwise be statically caught.

If you set defaultOptions: { watchQuery: { returnPartialData: true } }, TypeScript will not be aware of that and still tell you that it is safe to access result.data.some.nested.property (assuming that some and nested are non-nullable in the schema), but the reality is that they might be undefined, and your app might crash if you try to access them.
To deal with this, developers would have to be very thorough and essentially write

-result.data.some?.nested?.property
+(result.data as DeepPartialObject<typeof result.data>).some?.nested?.property

everywhere they use the useQuery hook and access it's return value - which then will completely undo the "savings" you get from defining a central default value. It's also incredibly easy to forget doing this cast, and if you only forget it once in your app, you get another runtime crash.

Even just supporting errorPolicy, if not any other default options, would make me happy. I dont think that one changes return types?

Depending on the errorPolicy, data returned by useSuspenseQuery could suddenly become null, and if you set that errorPolicy in a defaultOption, TypeScript can't warn you about the runtime crash you're likely to incur if you try to access result.data.property without checking for the existance of result.data every time - because TypeScript is convinced it is there. After all you didn't pass in the errorPolicy option ignore or all, so of course it has to be there.

---

This whole argument can be made pretty much for every single option - even if it's an option that doesn't have influence on the types today, we might improve on our call-site function types tomorrow and it might be causing a similar problem tomorrow.

[jerelmiller]

Hey @fabis94 👋

We discussed this in our team meeting so I wanted to fill you in on where we are at.

We are at a crossroads trying to decide whether we want to keep defaultOptions long-term or deprecate them in v5. Their biggest issue, as @phryneas has said, is type safety. It's much too easy to introduce runtime crashes in your application, especially with Suspense-enabled hooks, where TypeScript can't help you since it isn't aware of the defaults.

We'd like to solve for the type safety issue before introducing subscriptions into defaultOptions to avoid the glaring runtime risk. If this becomes something too tricky to solve at a type-level, thats when we will deprecate defaultOptions (though I think we'd like to have a replacement if we go that route).

So for now we aren't planning to add subscription into the defaultOptions mix but we'll continue to experiment with alternative solutions that help solve for type safety.