feat: generate schema from paths OpenAPI prop #230

faizplus · 2024-04-30T14:05:08Z

oas2json looks fine but oas2ts is not creating correct types references/imports
introduces breaking changes as generated schema/types files now reside inside their node folder e.g. components.schemas or paths
tests will fail as output path has changed and types references/imports are not always correct.

`oas2json` looks fine but `oas2ts` is not creating correct types references/imports

github-actions · 2024-04-30T14:05:19Z

No linked issues found. Please add the corresponding issues in the pull request description.
Use GitHub automation to close the issue when a PR is merged

faizplus · 2024-04-30T14:57:25Z

example/output/types/paths/Pet.d.ts

@toomuchdesign @ilteoood, Can you have a look here as I don't think there is any way to figure out which import statements to keep and which one to remove as @bcherny/json-schema-ref-parser returns all references. for example here we needed to import only Pet from '../components.schemas/Pet' but it also included Category and Tags which are only required in '../components.schemas/Pet'. So wherever there is reference to ../components.schemas/Pet it also imports Category and Tags

to be honest I don't understand why we need this. this is the typescript representation of a path, but in which case are we going to use it?
IMO we'll use just the objects related to it, like: Pet, Category, Tag...but not the structure of the entire endpoint

hey @ilteoood, paths have only the structure and references to actual types through components/schemas. We are already generating the types like Pet, Category and Tag in components.schemas. If we don't want the structure of path then I am not sure what else is left to convert to a type.

What about items like query params and path params?

ok, to convert /pet/findByStatus get parameter, what should be the type name and file name?

Will this work?

export interface PetFindByStatusGetParameters { /** * Status values that need to be considered for filter */ status?: 'available' | 'pending' | 'sold'; [k: string]: unknown; }

You can retrieve paths entities as named components only when the openAPI definition fully abstracts its return entities as components.schemas objects.

Paths return definitions are often a mixture of inlined and ref definitions. eg:

{ "type": "array", "items": { "$ref" : "#/components/schemas/Pet" } }

.. where Pet is abstracted but the the array is inline.

I personally find openapi-typescript output very convenient, replicating the original openAPI definition structure. But it's 100% subjective.

type SuccessResponse = paths["/my/endpoint"]["get"]["responses"][200]["content"]["application/json"]["schema"]; type ErrorResponse = paths["/my/endpoint"]["get"]["responses"][500]["content"]["application/json"]

...in our specific case we are trying to get a step further and split the type output in different files.

Hey, we can work with mixture of inlined and ref definitions but do we really want each type to be in a separate file? For example for a particular path there can be multiple methods like get, post, put which can have multiple types for responses, parameters, requestBody etc.

Before delving into the implementation I'd suggest to focus on a single library method (oas2json or oas2ts) and write down the expected output given a moderately complex openAPI definition. This way we could better validate the solution we want to aim to.

Such output could be very different based on different design decisions.

Once decided what the output is supposed to be, it would be easier to find a viable implementation. You might even consider writing tests first to support your development effort.

Here is one question we could start from: should generated .json files contain purely valid JSON schemas (as component schemas currently do) or they could even contain non-JSON schema structures representing the original OpenAPI definition structure? OpenApi paths object is not a simple JSON schema object, but a structured object containing JSON schemas at different nesting levels.

toomuchdesign · 2024-04-30T15:29:50Z

Hi @faizplus! A few questions:

Is this PR trying to provide supports to other props than components.schemas for all 3 methods oas2json, oas2ts, oas2tson?
How would it work? Would openapi-transformer-toolkit generate schemas for any prop found in the OpenAPI definition or just a specific subset of it?
Could you briefly explain the conversion flow this PR tries to implement?

I'd personally suggest to try and implement such feature method by method (maybe starting from oas2json or oas2ts) and make sure that we find a usable UX/API to start with.

Thanks!

faizplus · 2024-04-30T16:22:54Z

Hi @faizplus! A few questions:

Is this PR trying to provide supports to other props than components.schemas for all 3 methods oas2json, oas2ts, oas2tson?

How would it work? Would openapi-transformer-toolkit generate schemas for any prop found in the OpenAPI definition or just a specific subset of it?

Could you briefly explain the conversion flow this PR tries to implement?

I'd personally suggest to try and implement such feature method by method (maybe starting from oas2json or oas2ts) and make sure that we find a usable UX/API to start with.

Thanks!

Hey @toomuchdesign, I am currently trying to add support for paths and you can see the output of oas2json in example/output/json and output of oas2ts in example/output/types

With this PR, i have added one more option to oas2json and oas2ts as -p where you can pass other prop names like paths from open api yml file. Then raw yml content is parsed const parsedOpenAPIContent = YAML.parse(openAPIContent) then schema is generated

const generatedSchema = fromSchema(parsedOpenAPIContent, {
   definitionKeywords
})

then I have created a function (convertOpenApiPathsToSchema) specific for paths which accepts generatedSchema.paths which generates separate schema file for each path in paths property from openapi yml.

ilteoood · 2024-05-02T08:50:48Z

src/commands/json2ts.ts

+      )
+    })
+    .map(refSchema => {
+      let importFrom = ''


Suggested change

let importFrom = ''

let importFrom

ilteoood · 2024-05-02T08:51:48Z

src/commands/json2ts.ts

+    ])
+  ]
+
+  type pathInfoItem = {


Suggested change

type pathInfoItem = {

type PathInfoItem = {

ilteoood · 2024-05-02T08:52:38Z

src/commands/oas2json.ts

@@ -1,3 +1,4 @@
+/* eslint-disable  @typescript-eslint/no-explicit-any */


we should avoid disabling eslint rules, especially the one related to any

ilteoood · 2024-05-02T08:53:02Z

src/commands/oas2json.ts

+  name: string
+  in: string
+  required?: boolean
+  schema?: any


json schema has its own type definition

ilteoood · 2024-05-02T08:53:39Z

src/commands/oas2json.ts

+  schema?: any
+}
+
+interface OpenApiPath {


do you need to create your own type? IMO if you look through npm you'll be able to find something like this, but officially maintained by the OAS team

hey, can you provide the link please!

Literally the second entry if you search for openapi on npm: https://www.npmjs.com/package/openapi3-ts

ilteoood · 2024-05-02T08:53:51Z

src/commands/oas2json.ts

+  const pathObjects: { [key: string]: any } = {}
+  for (const [path, methods] of Object.entries(paths)) {
+    const pathObject = formatFileName(path)
+    const schema: any = {


avoid any pls

ilteoood · 2024-05-02T08:54:25Z

src/commands/oas2json.ts

+      }
+
+      if (description) {
+        schemaMethodProperties['description'] = description


there's no need for square brackets here.
same for summary

ilteoood · 2024-05-02T08:55:51Z

src/commands/oas2json.ts

      const isArray = Array.isArray(_get(parsedOpenAPIContent, key))
      processSchema(schema, schemasPath, key, isArray)
    })
  } catch (error) {
+    console.log(error)


there's no need for additional console log when you already have a logger

ilteoood · 2024-05-02T08:58:54Z

example/output/types/paths/Pet.d.ts

to be honest I don't understand why we need this. this is the typescript representation of a path, but in which case are we going to use it?
IMO we'll use just the objects related to it, like: Pet, Category, Tag...but not the structure of the entire endpoint

feat: generate schema from paths OpenAPI prop

ccfa24d

`oas2json` looks fine but `oas2ts` is not creating correct types references/imports

fix: fixed linting issues

3104f7f

faizplus commented Apr 30, 2024

View reviewed changes

ilteoood reviewed May 2, 2024

View reviewed changes

This was referenced Aug 11, 2024

oas2tson assumes strict camelcase naming -- bug #294

Closed

Fix/revert pr224 #304

Merged

What does -p option do? #305

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: generate schema from paths OpenAPI prop #230

feat: generate schema from paths OpenAPI prop #230

faizplus commented Apr 30, 2024

github-actions bot commented Apr 30, 2024

faizplus Apr 30, 2024

ilteoood May 2, 2024

faizplus May 2, 2024

ilteoood May 2, 2024

faizplus May 2, 2024

toomuchdesign May 2, 2024

faizplus May 2, 2024

toomuchdesign May 2, 2024 •

edited

Loading

toomuchdesign commented Apr 30, 2024

faizplus commented Apr 30, 2024

ilteoood May 2, 2024

ilteoood May 2, 2024

ilteoood May 2, 2024

ilteoood May 2, 2024

ilteoood May 2, 2024

faizplus May 2, 2024

ilteoood May 2, 2024

ilteoood May 2, 2024

ilteoood May 2, 2024

ilteoood May 2, 2024

ilteoood May 2, 2024

		@@ -1,3 +1,4 @@
		/* eslint-disable @typescript-eslint/no-explicit-any */

feat: generate schema from paths OpenAPI prop #230

Are you sure you want to change the base?

feat: generate schema from paths OpenAPI prop #230

Conversation

faizplus commented Apr 30, 2024

github-actions bot commented Apr 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toomuchdesign May 2, 2024 • edited Loading

Choose a reason for hiding this comment

toomuchdesign commented Apr 30, 2024

faizplus commented Apr 30, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

toomuchdesign May 2, 2024 •

edited

Loading