Issue with newly introduced OpenAPI extension

[nfroidure]

I use this handy module but the recent changes (d0241d6) broken my code here: https://github.com/nfroidure/whook/blob/master/packages/whook-http-transaction/src/index.ts#L53-L62

Indeed, before that update, I was able to access the OperationObject interface keys to extend it but now, it only gives string | number, here is a portion of code to understand the problem:

interface Test1 {
  id: number;
  text: string;
}
type Test1Keys = keyof Test1; // gives type: "id" | "text"
interface Test2 {
  id: number;
  text: string;
  [extension: string]: unknown;
}
type Test2Keys = keyof Test2; // gives type string | number

I think the commit should be reverted since it is a breaking change and instead create a major version with something like this to allow a better customization and keep a chance to access the keys of a parameter object to users:

interface BaseOperationObject {
  tags?: string[];
  summary?: string;
  description?: string;
  // ...
}
// If you prefer tie to interfaces
interface OperationObject<T = unknown> extends BaseOperationObject {
  extension: T;
}
// Or by using types (better imo since it allows to specify key names too)
type OperationObject<
  T extends Record<string, unknown> = Record<string, unknown>
> = BaseOperationObject & T;

Best would be to propagate the extension type from the OpenAPI.Document type to OperationObject downstream.

I can create a PR if you wish, just let me know.

[jarruda]

Interesting edge case you've found. I wasn't aware of this behavior with the keyof operator when an index signature is added.

Can you describe your use case? What about the keyof behavior difference is breaking for you? The change produces a superset of the type that was previously produced on OperationObject. It's not obvious to me what your linked code is trying to produce.

[nfroidure]

Can you describe your use case?

It creates a DereferencedOperationObject type where all properties that may contain a ReferenceObject are guaranteed to be dereferenced. It helps to avoid to have to tell the compiler everywhere that the ReferenceObject has already been handled somewhere else in the code.

That said, I think that the library should let users extend its types the way they want. The proposed API does without much extra complexity.

[jarruda]

How would your proposal do that in practice? Can you supply an example with several third-party extensions?

With HEAD:

const operationObject: OperationObject;
const apiGatewayIntegration = operationObject['x-amazon-apigateway-integration'] as ApiGatewayIntegration | undefined;
const expressRouter = operationObject['x-express-router'] as ExpressRouterExtension | undefined;
// ... any number of vendor-supplied extension types

If I have N extensions that I want to access on the OperationObject, it seems like I would have to create N extensions of OperationObject and access them individually?

I was trying to avoid forcing a user to do something like this, where they have to create a new type:

interface UserSuppliedOperationObject extends OperationObject {
    'x-apigateway-integration'?: ApiGatewayIntegration;
    'x-express-router'?: ExpressRouterExtension;
}
const pathItemObject: PathItemObject;
const operationObjectWithExtensions = pathItemObject.get as UserSuppliedOperationObject;
const apiGatewayIntegration = operationObjectWithExtensions['x-amazon-apigateway-integration'];

[nfroidure]

@jarruda Here (#704) is a sample config that would allow to do this:

type myAPI = OpenAPI.Document<{
    'x-apigateway-integration'?: ApiGatewayIntegration;
    'x-express-router'?: ExpressRouterExtension;
}>;

And fixes my problem:

type Test = keyof OpenAPI.OperationObject<{}>; // type T = "tags" | "summary" | "description" | "externalDocs" | "operationId" | "parameters" | "requestBody" | "responses" | "callbacks" | "deprecated" | "security" | "servers"

It also avoid weak typing by giving control to users. They can opt-in to OpenAPI extensions but, per default, the compiler will complain if one uses an unknown property.

[nfroidure]

If you prefer default to weak types, just replace AType<T extends {} = {}> occurrence per <T extends {} = Record<string, unknown>> but best would be to handle it on your own code since you only know what you can assume to sit in your OpenAPI instance.

[jarruda]

Interesting approach. I think you would want to extend it to support adding extensions at any supported level in the spec: https://swagger.io/specification/ (look for "This object MAY be extended with Specification Extensions.") Otherwise, it could create a backwards-incompatible scenario when one wants to add an extension to a new class.

...best would be to handle it on your own code since you only know what you can assume to sit in your OpenAPI instance.,

I think, ideally, an extension vendor would write a package that adds their extension types to a Document. I would prefer that I, as a user, weren't able to accidentally add an x-amazon-apigateway-integration extension to e.g. a PathInfoObject where it wouldn't be valid. I'd like to add a dependency on e.g. api-gateway-openapi-extensions and somehow be able to add all of its extensions to my Document.