Allow for mixing optional and required parameters in query parameters

[0237h]

Hello,

I've noticed that the code generation doesn't allow for having both required and optional parameters for an endpoint.

Example

"parameters": [
  {
    "name": "test_required",
    "in": "query",
    "required": true,
    "schema": {
      "type": "string"
    }
  },
  {
    "name": "test_optional_1",
    "in": "query",
    "required": false,
    "schema": {
      "type": "string"
    }
  },
  {
    "name": "test_optional_2",
    "in": "query",
    "required": false,
    "schema": {
      "type": "string"
    }
  }
]

will generate

parameters: {
  query: { test_required: string; test_optional_1: string; test_optional_2: string };
};

If all parameters are set with required: false, the codegen works as expected:

parameters: {
  query: Partial<{ test_required: string; test_optional_1: string; test_optional_2: string }>;
};

I believe that the issue comes from this code that checks for lists.query.every, is there a way to fix this behavior perhaps ?
https://github.com/astahmer/typed-openapi/blob/main/packages/typed-openapi/src/map-openapi-endpoints.ts#L88

// Make parameters optional if none of them are required
if (params) {
  const t = createBoxFactory({}, ctx);
  if (params.query && lists.query.length && lists.query.every((param) => !param.required)) {
    if (!params.query) params.query = {};
    params.query = t.reference("Partial", [t.object(params.query)]) as any;
  }
[...]

[0237h]

I think I have a solution that would preserve the current behavior while allowing for mixing optional and required parameters in the codegen:

[...]
      if (params) {
        const t = createBoxFactory({}, ctx);

        // Iterate through `params` keys instead of duplicating `if/else` for `query`, `path` and `header`
        // but same idea
        let k: keyof typeof params;
        for (k in params) {
          if (k !== "body") {
            // Same check as before (e.g. `params.query && lists.query.length`)
            if (params[k] && lists[k].length) {
              // Same case as before here, mark the whole object `Partial`
              if (lists[k].every((param) => !param.required)) {
                params[k] = t.reference("Partial", [t.object(params[k]!)]) as any;
             // New case where we loop through the params and apply `t.optional` if they are not required
              } else {
                // Works since `params` and `lists` basically work with the same objects 
                // (just arranged differently) `params` has them as `key: value` where 
                // `lists` has them as array of objects
                for (const p of lists[k]) {
                  if (!p.required) {
                    params[k]![p.name] = t.optional(params[k]![p.name] as any);
                  }
                }
              }
            }  
          }
        }
        // No need to pass empty objects, it's confusing
        endpoint.parameters = Object.keys(params).length ? (params as any as EndpointParameters) : undefined;
      }
[...]

Testing on the repo's Docker example, we have a definition like this:
docker.openapi.yaml

[...]
      parameters:
        - name: id
          in: path
          description: ID or name of the container
          required: true
          schema:
            type: string
        - name: path
          in: query
          description:
            "Path to a directory in the container to extract the archive’s
            contents into. "
          required: true
          schema:
            type: string
        - name: noOverwriteDirNonDir
          in: query
          description: |
            If `1`, `true`, or `True` then it will be an error if unpacking the
            given content would cause an existing directory to be replaced with
            a non-directory and vice versa.
          schema:
            type: string
        - name: copyUIDGID
          in: query
          description: |
            If `1`, `true`, then it will copy UID/GID maps to the dest file or
            dir
          schema:
            type: string
[...]

Important

Notice that for query parameters path has required: true while the others, having no required field, are optional by default

Currently, this example is getting parsed like this:

parameters: {
  query: { path: string; noOverwriteDirNonDir: string; copyUIDGID: string };
  path: { id: string };

  body: string;
};

whereas the proposed change will turn it into:

parameters: {
  query: { path: string; noOverwriteDirNonDir: string | undefined; copyUIDGID: string | undefined };
  path: { id: string };

  body: string;
};

- query: { path: string; noOverwriteDirNonDir: string; copyUIDGID: string };
+ query: { path: string; noOverwriteDirNonDir: string | undefined; copyUIDGID: string | undefined };

which I think is more aligned with the optional characteristic of the original schema.

[0237h]

One thing I noticed though about this implementation is that, at least for Zod codegen (the one I use), the conversion of T | undefined is translated explicitly. From the example above, that would look like:

query: z.object({
  path: z.string(),
  noOverwriteDirNonDir: z.union([z.string(), z.undefined()]),
  copyUIDGID: z.union([z.string(), z.undefined()]),
})

I would have expected to have simply:

query: z.object({
  path: z.string(),
  noOverwriteDirNonDir: z.string().optional(),
  copyUIDGID: z.string().optional(),
})

But that's probably related to typebox and outside the scope of this project.

[astahmer]

But that's probably related to typebox and outside the scope of this project.

I can confirm this, typed-openapi doesn't take care of anything but the TS output