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

feat: add x-explicitMappingOnly extension #1215

Merged
merged 1 commit into from
Mar 27, 2020

Conversation

zeapo
Copy link
Contributor

@zeapo zeapo commented Mar 26, 2020

This replaces #774

When the mapping is partial, here the dog mapping was removed:

      discriminator:
        propertyName: petType
        mapping:
          cat: '#/components/schemas/Cat'
          kitten: '#/components/schemas/Cat'
          bee: '#/components/schemas/HoneyBee'

The type Dog will be included in addition to the mappings:
dropdown_2

The idea is to have an exhaustive mapping, in that case we'd like to ignore non-mapping child classes. I've added a new extension x-limitToMapping of type boolean. When used, it excludes the class Dog in the following case:

      discriminator:
        propertyName: petType
        x-limitToMapping: true
        mapping:
          cat: '#/components/schemas/Cat'
          kitten: '#/components/schemas/Cat'
          bee: '#/components/schemas/HoneyBee'

will yield the following result:
dropdown_3

@adamaltman
Copy link
Member

Thanks for rewriting this clearly and referencing the original issue. 👍 👍

@RomanHotsiy
Copy link
Member

RomanHotsiy commented Mar 26, 2020

Ooh! @zeapo sorry for not responding 🙈 . Tons of work here.

This PR is so much shorter 😍! Good job!

I'm not just fully sold on the name x-limitToMapping

@adamaltman do you have any other ideas? @zeapo maybe you have some alternative?

@zeapo
Copy link
Contributor Author

zeapo commented Mar 26, 2020

Thanks :)

Maybe x-explicitMappingOnly? As you've named them explicit/implicit.

@RomanHotsiy
Copy link
Member

RomanHotsiy commented Mar 26, 2020

Well, it is how they call it in the spec:

image

I like x-explicitMappingOnly more. Could you rename it and also add some short docs to https://github.com/Redocly/redoc/blob/master/docs/redoc-vendor-extensions.md

As soon as you're done I am ready to merge this 👍

@zeapo
Copy link
Contributor Author

zeapo commented Mar 26, 2020

@RomanHotsiy , It's done. Let me know if anything is missing.

@adamaltman
Copy link
Member

I think this is a good addition.

@RomanHotsiy RomanHotsiy changed the title Add limit to mapping extension feat: add x-explicitMappingOnly extension Mar 27, 2020
@RomanHotsiy RomanHotsiy merged commit ea5b0aa into Redocly:master Mar 27, 2020
@RomanHotsiy
Copy link
Member

@zeapo awesome! Thanks 🙇

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 this pull request may close these issues.

3 participants