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

fix: DiscriminatorDescriptor interface now accepts number, too #582

Closed
wants to merge 3 commits into from

Conversation

MickL
Copy link

@MickL MickL commented Feb 12, 2021

When using an enum as name it works fine by adding //@ts-ignore but the interface didnt allow that.

@ValidateNested()
  @Type(() => MyDto, {
    discriminator: {
      property: 'type',
      subTypes: [
        {value: MySubDto1, name: MyEnum.Something},
        {value: MySubDto2, name: MyEnum.SomethingOther},
      ],
    },
    keepDiscriminatorProperty: true,
  })
  cupData: MySubDto1 | MySubDto2;

Description

I changed the interface to accept string OR number.

Checklist

  • the pull request title describes what this PR does (not a vague title like Update index.md)
  • [ x the pull request targets the default branch of the repository (develop)
  • the code follows the established code style of the repository
    • npm run prettier:check passes
    • npm run lint:check passes
  • tests are added for the changes I made (if any source code was modified)
  • documentation added or updated
  • I have run the project locally and verified that there are no errors

When using an enum as `name` it works fine by adding `//@ts-ignore` but the interface didnt allow that. 

```
@ValidateNested()
  @type(() => MyDto, {
    discriminator: {
      property: 'type',
      subTypes: [
        {value: MySubDto1, name: MyEnum.Something},
        {value: MySubDto2, name: MyEnum.SomethingOther},
      ],
    },
    keepDiscriminatorProperty: true,
  })
  cupData: MySubDto1 | MySubDto2;
```
@MickL MickL changed the title DiscriminatorDescriptor should accept number, too fix: DiscriminatorDescriptor interface now accepts number, too Feb 12, 2021
@MickL
Copy link
Author

MickL commented Feb 14, 2021

Not sure what I can do about the semantic pull request but if one of the maintainers might fix it or tell me what to do that would be awesome :)

@MickL
Copy link
Author

MickL commented Feb 23, 2021

Can this be merged?

@MickL
Copy link
Author

MickL commented Apr 15, 2021

@NoNameProvided

@MickL
Copy link
Author

MickL commented Jun 22, 2021

Sorry @NoNameProvided I dont understand what is meant with semantic commit and pr title. This is just a very very simple change in the TS interface but the PR is open for more than 4 months now :(

@MickL
Copy link
Author

MickL commented Jun 14, 2023

Its been 2.5 years and the issue still persists. Can this PR finally be merged? @NoNameProvided

@NoNameProvided
Copy link
Member

Fixed in #1683.

Copy link

github-actions bot commented Jun 4, 2024

This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 4, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants