Drop our openapi-spec packages, use openapi3-ts instead

[bajtos]

https://github.com/metadevpro/openapi3-ts looks more feature complete than our own openapi packages. Should we use openapi3-ts instead?

If we want to support both Swagger v2 and OpenAPI v3 then we probably cannot drop our packages, unless there is a replacement available.

UPDATE:

Looks like we have already dropped most of our custom type definitions. As of today (0411811aa), openapi-v3-spec-types provides only few helpers:

• OpenApiSpec as an alias for OpenAPIObject
• createEmptyApiSpec()
• isSchemaObject() and isReferenceObject() (type guards)

Acceptance criteria

• contribute type guards to openapi3-ts - see Add type guards for SchemaObject and ReferenceObject metadevpro/openapi3-ts#46 and Refactor openapi guards to use the upstream version #2974
• try to upstream createEmptyApiSpec, e.g. to src/dsl/OpenApiBuilder.ts
• move the rest to our openapi-v3 package (createEmptyApiSpec should go to openapi--spec-builder package) - see Remove usages of "@loopback/openapi-v3-types" and deprecate the package #3220
• and finally remove openapi-v3-types - see feat: remove openapi-v3-types package #3385

[bajtos]

Related: #182.

[dhmlau]

@jannyHou , with your openapiv3 work, is this ticket still valid?

[bajtos]

From what I remember, our OpenAPI v3 packages are using a 3rd party dependency under the hood, but adding some extra features/types on top of that dependency. I would personally like more if we could contribute our improvements to that 3rd party package and start using it directly. IIRC, @jannyHou had good arguments against that, could you @jannyHou please re-post them here please?

[virkt25]

@jannyHou any updates for this?

[jannyHou]

@bajtos @virkt25 Nice to know we already have an issue to continue the discussion. Copy paste from my V3 prototype PR:

---

From @bajtos
It seems to me we are duplicating a lot of types from openapi3-ts package. I consider it as unnecessary work and extra maintenance cost for the future. Can we keep the number of custom types as small as possible, and contribute any missing features/types to openapi3-ts to have even less code to maintain ourselves?
Ideally, we should be able to leverage openapi3-ts as-is and remove our openapi-v3-types package entirely. I understand that contributing changes to upstream dependencies take time, so I am ok with the current proposal as a quick solution for MVP.

---

From @jannyHou

Interesting..by relying on the community module, I have two concerns:

• For anything not consistent with OpenAPI spec, keep patching a community module VS overriding them in our own package, which one is more convenient for us?
  This is not a doubt for the maintainer of openapi3-ts, he replies very quickly actually, he merged my patch in a few minutes after created, but as a long term solution, we import the benefit but also the potential risk.

• People may take different approaches to describe a property, like ParameterLocation:

  export type ParameterLocation = 'query' | 'header' | 'path' | 'cookie';
  export interface ParameterObject extends ISpecificationExtension {
    name: string;
    // `openapi3-ts` define it as string by `in: string`
    in: ParameterLocation;
    // ...other properties
  }

Essentially ParameterLocation is just a string, we use a union type to describe it like an enum, but I don't see any problem if people loose the restriction and just treat it as a string in their project. Patching the community module will break other projects using it without caring about the restriction.

So as a compromised solution, what I did is:

Override important spec types by ourselves, export spec interfaces from the community module if missing in our package

---

And I got some feedback from @raymondfeng

If the module is in a good shape and the maintainer is collaborative, +1 to use it. We can folk it any time if things won't work out any more.

[bajtos]

People may take different approaches to describe a property, like ParameterLocation:

  export type ParameterLocation = 'query' | 'header' | 'path' | 'cookie';
  export interface ParameterObject extends ISpecificationExtension {
    name: string;
    // `openapi3-ts` define it as string by `in: string`
    in: ParameterLocation;
    // ...other properties
  }

Essentially ParameterLocation is just a string, we use a union type to describe it like an enum, but I don't see any problem if people loose the restriction and just treat it as a string in their project. Patching the community module will break other projects using it without caring about the restriction.

My opinion: if the OpenAPI spec mandates a well-defined list of valid in values, then the type definition should reflect that and use ParameterLocation type instead of a generic string. I hope we can convince maintainers of openapi3-ts to follow this thinking and make the type definitions more accurate.

[bajtos]

BTW, looks like we have already dropped most of our custom type definitions. As of today (0411811aa), openapi-v3-spec-types provides only few helpers:

• OpenApiSpec as an alias for OpenAPIObject
• createEmptyApiSpec()
• isSchemaObject() and isReferenceObject() (type guards)

I am proposing to:

• contribute type guards to openapi3-ts
• try to upstream createEmptyApiSpec
• then move the rest to our openapi-v3 package
• and finally remove openapi-v3-types

[jannyHou]

I am proposing to:
contribute type guards to openapi3-ts
try to upstream createEmptyApiSpec
then move the rest to our openapi-v3 package
and finally remove openapi-v3-types

The proposal sounds great to me 👍 better to organize the spec-gen codes into one package

[dhmlau]

@jannyHou , looks like we're done with this task? Is it good to close? Thanks.

[virkt25]

It's not ... this package still exists https://github.com/strongloop/loopback-next/tree/master/packages/openapi-v3-types and createEmptyApiSpec hasn't been upstreamed AFAIK.

[bajtos]

Let me copy the items from #301 (comment) into acceptance criteria of this story.

[jannyHou]

@dhmlau +1 for Taranveer's comment it's not done yet. And +1 for Miroslav's acceptance criteria 👍