feat: enable header parameters

[Turtlator]

No description provided.

[tristanmenzel]

I think it's worth adding functionality to only include specific headers in the request factories as I think most of the time, headers will not be something we want to have to pass in with each request. In this example the acceptVersion param is something you'd specify in the proxy class rather than specify on each request.

Perhaps we could add something like:

// Include all headers in request factories
openapi-tsrf generate ... --include-headers *

// Include specific headers in request factories
openapi-tsrf generate ... --include-headers header1 header2 header3

It'd also be worth including a specific example of this behaviour via a new spec in the examples dir and adding a matching test in tests/index.spec.ts

[tristanmenzel]

commander docs for variadic options

[Turtlator]

Yeah that makes sense. I'll add that in.

[pleb]

I think headers come in two flavours. Ones you supply with every request and ones that are defined by the spec per operation.

In the case of the one defined by the spec, I don't know if you want to have the option to disable it.

Take this spec, for example:

@@ -0,0 +1,1227 @@
{
  "openapi": "3.0.0",
  "paths": {
    "/receipts": {
      "post": {
        "operationId": "receipts",
        "parameters": [
          {
            "name": "some-value",
            "in": "path",
            "required": true,
            "schema": {
              "type": "string",
            }
          },
          {
            "name": "X-Platform",
            "in": "header",
            "required": true,
            "schema": {
              "type": "string",
            }
          }
        ],
        "requestBody": {
          "content": {
            "application/json": {
              "schema": {
                "type": "object",
                "required": [
                  "app_user_id",
                ],
                "properties": {
                  "app_user_id": {
                    "type": "string",
                  },
                }
              }
            }
          }
        },
        "responses": {
          "200": {
            "content": {
              "application/json": { }
            }
          }
        },
        "deprecated": false
      }
    }
  }
}

The spec advertises that you can only call the endpoint if you supply the required header. So, I think roughly the call would be

  const response = await API.postReceipts({ someValue: 'hi',  xPlatform: 'pleb os' })

[tristanmenzel]

It's not a choice of whether or not to include the header - it's whether we force the consumer to provide it at the call site (as per your example) or whether we implicitly supply it when constructing the request in our api proxy factory.

We can't determine which option is better from the spec alone, so we should make it configurable so the user can decide.

[Turtlator]

As discussed, I've added a --include-headers flag to the cli, allowing you to specify * or a whitelist of header names like --include-headers header1 header 2

[Turtlator]

default behaviour is to not include any headers

[pleb]

Would it be cleaner if headers worked a little more like?

static subscribers({ app_user_id, xPlatform, xIsSandbox, }: { app_user_id: string, xPlatform?: string, xIsSandbox?: boolean, }): GetRequest<Subscriber> {
    return {
      method: 'GET',
      url: `/subscribers/${app_user_id}`,
      headers: { 'X-Platform': xPlatform, 'X-Is-sandbox': xIsSandbox }, // 👉 no transformation via toHeaders
    }
  }

const factory = new ApiProxyFactory<RequestConfig>(
  async <TResponse>(
    { url, method, ...rest }: AnyRequest<TResponse>,
    config?: RequestConfig,
  ) => {
    const init: RequestInit = {
      // ...
      headers: {
        Authorization: `Bearer ${config?.authToken}`,
        Accept: 'application/json',
      },
    }

    const data = (rest as any)?.data
    if (['POST', 'PUT', 'PATCH'].includes(method) && data !== undefined) {
        // ...
        init.headers = { 
          ...init.headers, { 
          'Content-Type': 'application/json',
          } // 👉 spread headers to add this additional header
        }
      }
    }

    const headers = (rest as any)?.headers
    if (headers) {
      init.headers = {  
        ...init.headers, 
        ...headers // 👉 spread init.headers and headers to combine
      }
    } 

    const fetchResult = await fetch(
      // ...    
    )

     // ...
  },
)

[Turtlator]

@pleb I've just updated how headers work so you can now use the spread operator in the proxy, rather than appending new headers to a header object.

[tristanmenzel]

Should be able to use Record<string,string> for typing I think?

[tristanmenzel]

Actually I'm not sure this is even needed. We only filter undefined options for query params so we don't end up with ?blah=undefined. It should be fine to just specify headers in the factory as headers: { header1, header2 }

[Turtlator]

So rather than calling toHeaders({'x-header-name': xHeaderName}) we just return {'x-header-name': xHeaderName} ?

[Turtlator]

Because we have Record<string, string> on the Request interfaces, we can't just return the header values as they are, we need to convert them as strings. The other option is to have any header parameters as string variables in the client to convert them to string before calling any generated methods. This is a little less type safe though because it means people can pass any string in as a value rather than the type that's specified in the swagger doc.

[Turtlator]

Just resolved our usage with @pleb