Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for 'definitions' in json-schema #675

Open
2 tasks done
asc11cat opened this issue Oct 7, 2022 · 5 comments · May be fixed by #676
Open
2 tasks done

Support for 'definitions' in json-schema #675

asc11cat opened this issue Oct 7, 2022 · 5 comments · May be fixed by #676

Comments

@asc11cat
Copy link

asc11cat commented Oct 7, 2022

Prerequisites

  • I have written a descriptive issue title
  • I have searched existing issues to ensure the feature has not already been requested

🚀 Feature Proposal

#639 (comment)

The 'definitions' keyword in the component(3.0)/definition(2.0) item schema itself is not allowed at all, it is allowed only in top-lvl as 'definition' if we are using the OpenAPI 2.0 version. The changes introduced in #472 (https://github.com/fastify/fastify-swagger/pull/472/files#diff-3faaf157cccc3cc534ec616affca5b4f5df9817c937b7ada4f110afd2770aae2R35) is deleting the 'definitions' key from the schema item object, therefore preventing any possible errors on the schema validation, but also preventing the correct use of json-schema with this keyword.

In the case of json-schema coming from fastify, it seems like we have two ways to solve this problem, first one is to mutate 'definitions' keyword to 'properties' as stated in #524 (comment) workaround, but i have a suspicion that this can cause troubles when resolving some deep-nested schema, currently I'm in the process of checking it.

The second one is to move all nested definitions from 'definitions' obj to top-lvl of components/definitions, but that doesn't seem to follow the intended behaviour from original json-schema(?)

Motivation

Addressing the issue in #639

Example

No response

@asc11cat
Copy link
Author

asc11cat commented Oct 7, 2022

@mikaelkaron as for now I'm taking the first approach, and I see a few caveats here:

If we have both 'properties' and 'definitions' in json-schema - we need to merge them, in usual cases that's not a problem, but I can see a case like that, where one of the properties refers to the definition on top-lvl:

 "address": {
      "$id": "/schemas/address",
      "$schema": "http://json-schema.org/draft-07/schema#",

      "type": "object",
      "properties": {
        "street_address": { "type": "string" },
        "city": { "type": "string" },
        "state": { "$ref": "#/definitions/state" }
      },
      "required": ["street_address", "city", "state"],

      "definitions": {
        "state": { "enum": ["CA", "NY", "... etc ..."] }
      }
    }

So we need some type of a merging strategy?

@asc11cat
Copy link
Author

asc11cat commented Oct 8, 2022

Finished the fixes for openapi, seems to work fine. Definitions are now being merged with properties at transformDefsToComponents helper, solutions is kind of hacky, but I don't really see any other choices here

I was thinking about case that I mentioned earlier at #675 (comment), but it looks like we also lack support for a relative path which was the issue in #612, so I guess we can't do better yet

@mcollina would appreciate if you can take a look at #676, so I can proceed further with the swagger part if everything seems fine

@Fdawgs Fdawgs linked a pull request Oct 18, 2022 that will close this issue
4 tasks
@hongkongkiwi
Copy link

When will this get merged?

I'm using prisma-json-schema-generator to generate a json schema from my prisma file, but this file has definitions in it and at the current time this completely breaks this plugin.

@melroy89
Copy link
Contributor

When will this get merged?

I'm using prisma-json-schema-generator to generate a json schema from my prisma file, but this file has definitions in it and at the current time this completely breaks this plugin.

I also hope the related PR will be merged rather sooner than later. Fastify Swagger is lagging behind the industry standards.

@navalex
Copy link

navalex commented May 14, 2024

When will this get merged?

I'm using prisma-json-schema-generator to generate a json schema from my prisma file, but this file has definitions in it and at the current time this completely breaks this plugin.

Exactly same issue here, since I needed a clean way to reference my prisma schema for almost all my endpoints, I guess I'll make a little script to try migrate those definitions to components for the moment. Did you found any workaround on your side ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants