hey-api / openapi-ts

🚀 The OpenAPI to TypeScript codegen. Generate clients, SDKs, validators, and more. Support: @mrlubos
https://heyapi.dev
Other
1.28k stars 102 forks source link

Partial options function for tanstack-query #924

Open hougesen opened 2 months ago

hougesen commented 2 months ago

Description

We have the authorization header required in our openapi spec, which sadly means that any endpoint that requires auth cannot use the tanstack-query generated code, since the bearer token has the type string | undefined.

// ...
const bearerHeader = ref<string | undefined>(undefined);
// ...

const { data } = useQuery(
  computed(() =>
    getSomethingThatRequiresBearerTokenOptions({
      // Type 'string | undefined' is not assignable to type 'string'.
      // The expected type comes from property 'authorization' which is declared here on type '{ authorization: string; }'
      headers: { authorization: bearerHeader.value },
    }),
  ),
);

All pages in the application are locked behind login, which means that we know that the bearer token exists, but since TypeScript is not aware of that we have to use a fallback dummy value if we wish to use the generated code (or write the query function ourself).

const { data } = useQuery(
  computed(() =>
    getSomethingThatRequiresBearerTokenOptions({
      headers: { authorization: bearerHeader.value ?? 'missing-bearer-token' },
    }),
  ),
);

I believe the simplest way to accomplish the above would be to have a function that wraps the query function.

const canTriggerPartialGetSomethingThatRequiresBearerTokenOptions = (
  options: Options<Partial<GetSomethingThatRequiresBearerTokenOptionsData>>,
): boolean => {
  /// ...
};

export const getSomethingThatRequiresBearerTokenPartialOptions = <
  FallbackValue,
>(
  options: Options<Partial<GetSomethingThatRequiresBearerTokenOptionsData>> & {
    fallback?:
      | FallbackValue
      | (() => FallbackValue)
      | (() => Promise<FallbackValue>);
  },
) =>
  queryOptions({
    queryFn: async ({ queryKey }) => {
      if (
        !canTriggerPartialGetSomethingThatRequiresBearerTokenOptions(options)
      ) {
        if (options.fallback) {
          if (typeof options.fallback === 'function') {
            return options.fallback();
          }

          return options.fallback();
        }

        return null;
      }

      const { data } = await getSomethingThatRequiresBearer({
        ...options,
        ...queryKey[0].params,
        throwOnError: true,
      });
      return data;
    },
    queryKey: [
      {
        params: createQueryKeyParams(options),
        scope: 'getSomethingThatRequiresBearer',
      },
    ],
  });
hougesen commented 2 months ago

This partial function would also be useful for any query that depends on another query.

mrlubos commented 2 months ago

Okay @hougesen, there are a few topics in this thread. Let's focus on the authorization header issue. The most basic way to bypass the compiler would be to use a non-null assertion. This is okay, since in your own words, you know the token exists and it's purely a TypeScript issue.

const { data } = useQuery(
  computed(() =>
    getSomethingThatRequiresBearerTokenOptions({
      headers: { authorization: bearerHeader.value! },
    }),
  ),
);

If you want to clean up the types, I recommend this article if you haven't read it before https://tkdodo.eu/blog/react-query-and-react-context. With TanStack Router, you can even pass the header in route context and it would be typed properly since the route would not render if it's undefined.

hougesen commented 2 months ago

The most basic way to bypass the compiler would be to use a non-null assertion. This is okay, since in your own words, you know the token exists and it's purely a TypeScript issue.

You are right, null assertions is definitely an option in this case.

With TanStack Router, you can even pass the header in route context and it would be typed properly since the route would not render if it's undefined

Sadly TanStack Router does not support Vue (yet?).

A more common use case is in situations where your query depends on another query's data. Having a waterfall based api can be a bad practice, but in some cases it might be hard to avoid, especially when dealing with human readable urls.

flowchart TB
    visit["Go to user profile"]
        --> userinfo["Fetch user from username in url"]
            --> data["Fetch data from user.id" ]

The data query above would have to be written manually since user.id it is not guaranteed to exists. But this is of course a nice to have, not need to have 😄

mrlubos commented 2 months ago

I'm also curious about the premise of requiring an authorization token. Shouldn't it be the case that if it's not provided, the endpoint simply returns access not allowed?

hougesen commented 2 months ago

Yes, if a token isn't provided it will simply return a 401. The header was added to our apis to differentiate between endpoints that are public and authenticated.

The apis are used by multiple internal and external applications - both client and server side, in various languages.

We used to use interceptors in our internal applications, but found that it was more transparent to add the header to our OpenAPI specification.

mrlubos commented 2 months ago

We have 2 options here effectively, the way I see it. Either modify the types, or modify your OpenAPI spec on client side so that it results in modified types. Not proposing to touch your OpenAPI spec at source. I'll file this away for later exploration as I think modifying OpenAPI spec will be desired for other use cases too

mrlubos commented 2 months ago

For your use case, non-null assertion works. You could also explore overriding the offending interface and see if you can make that field optional that way?