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

can discriminator be optional ? #2710

Closed
qiaozha opened this issue Nov 30, 2023 · 2 comments
Closed

can discriminator be optional ? #2710

qiaozha opened this issue Nov 30, 2023 · 2 comments

Comments

@qiaozha
Copy link
Member

qiaozha commented Nov 30, 2023

Not sure if it's a bug or intentional, in the below case, public typespec playground link

@discriminator("kind")
model Pet {
  name: string;
  weight?: float32;
}

the OpenAPI3 emitter output, will not put kind into required list

    Pet:
      type: object
      required:
        - name
      properties:
        name:
          type: string
        weight:
          type: number
          format: float
        kind:
          type: string
          description: Discriminator property for Pet.
      discriminator:
        propertyName: kind

However, in swagger emitter, it will put kind into the required list, see azure playground link.
But on the other hand, we also have test cases in cadl ranch to testify missing discriminator's case cadl ranch link.

If it's intentional, I wonder why? if it's not, I am curious why we should allow discriminator decorator on a property that doesn't exist ?

/cc @xirzec

@timotheeguerin
Copy link
Member

timotheeguerin commented Dec 1, 2023

We did have some discussion about that, more when a discriminated union gets used in json merge patch where technically it could allow an optional discriminator.

What you are seeing though is a bug but also I think we want people to move awayt from the implicit discriminator property(where we magically inject it) See this issue #2589

@markcowl
Copy link
Contributor

markcowl commented Jan 8, 2024

Once the linting rule is added to require the discriminator to be specified: Azure/typespec-azure#97 this should be resolved, so closing as duplicate

@markcowl markcowl closed this as not planned Won't fix, can't repro, duplicate, stale Jan 8, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants