Optional "parameters" field (#1335) breaks header components

[denisw]

Description

The change in #1335 that makes the parameters property optional if all parameters are optional (thanks!) seems to not work with reusable header definitions in the components section. Trying to use a header component leads to the following type error:

error TS2339: Property 'example' does not exist on type '{ example?: string | undefined; } | undefined'.

1382         "Example"?: components["parameters"]["example"];

Reproduction

Create an OpenAPI file which:

• defines a header in the components.header section
• refers to that header in the parameters of an operation

Checklist

• [x] I’m willing to open a PR (see CONTRIBUTING.md)

[duncanbeevers]

I suggest reverting #1335; it appears it wasn't sufficiently tested, breaking already-working production code. This is definitely not a trivial fix.

[drwpow]

We can revert it, but I’m not sure if we should publish a new version just for a rollback, since that change was fairly recent. The only patch release since v6.6.0 was a discriminator edge case that hasn’t been affecting anybody as far as I know.

But if we have another fix incoming before this issue, I can rollback #1335

[denisw]

Now that I looked more closely at #1335, the issue is that it addresses the wrong parameters object. In #1334, I was talking about the parameters of an operation object, while the "fix" changes the components object (where the optional parameters does not help, and introduces issues as seen here).

[drwpow]

Ah good catch @denisw—you’re right. No direct child of the components object should be potentially optional. 🤔 I’m surprised more tests didn’t throw, but I guess this just slipped through the current coverage.

[drwpow]

I’m going to attempt another fix to this; maybe we still see issues. But I think it’s an improvement? Happy to ship another patch or just revert this entirely if not.

[drwpow]

Also this is offtopic, but one thing I’m excited about for the v7 rewrite is what an AST lets you do:

const allOptional = paramLocType.every((node) => !!node.questionToken);

In other words, after you’ve built the AST, you can go back through and inspect what got built. Which… I know I’m just describing how ASTs work but in the context of this library I hadn’t considered how much that’d come in handy

[MellKam]

I have also encountered this problem, but in my case it is parameters.query type. Here is example

UsersController_get: {
  parameters?: {
    query?: {
      tier_id?: string;
      patreon_username?: string;
      discord_username?: string;
      page?: number;
      limit?: number;
    };
  };
  responses: { ... }
};

I am trying to access the query type, but I am getting ts error

type GetPaginatedUsersQuery =
	operations["UsersController_get"]["parameters"]["query"]
	
// Property 'query' does not exist on type '{ query?: { tier_id?: string | undefined; 
// patreon_username?: string | undefined; discord_username?: string | undefined; page?: 
// number | undefined; limit?: number | undefined; } | undefined; } | undefined'.

This worked fine in previous versions because the parameter was not an optional type.

[drwpow]

Yeah this may just need to be reverted. Optional objects are generally pretty friendly to use, but optional nested objects are not. Will revert the top-level parameters being optional and bump a minor version.

[drwpow]

The upcoming v6.7.0 will revert to previous behavior where parameters is never optional, but query/header/cookie may be if they have no required params (path params are always required by their nature).

[drwpow]

I believe this issue has been fixed both in the latest version of 6.x and 7.x but please let me know if anyone else is encountering an issue.