Polymorphism

[janis91]

Added the possibility to provide an array of discriminator functions in case of an array of different types.

If you have any other proposal for doing this, comment here or in the related ticket: #51

[NoNameProvided]

Before I can review, please write some docs about it in the readme with examples. So I can understand from the user point of perspective what you tried to achieve.

[janis91]

Ok, I will provide it asap. One question: travis is complaining, but I cannot reproduce it locally, is this a wrong configuration of CI or did I do something wrong?

[NoNameProvided]

One question: travis is complaining, but I cannot reproduce it locally

Only Node 4 is failing with

SyntaxError: Block-scoped declarations (let, const, function, class) not yet supported outside strict mode

Don't care about it, it's already removed from routing-controllers, so I will remove it from here as well.

Update: Done, you can merge master into your branch.

[janis91]

@NoNameProvided I wrote the doc part and rebased the commit.

[janis91]

@NoNameProvided any thoughts on this already?

[janis91]

It has been quite some time since I updated. Is there anything left why this does not get integrated?
@NoNameProvided

[NoNameProvided]

Hi @janis91

It's because I have 100+ Github issue, and short on time. I am also not fully engaged with class-transformer yet, as I want to kick class-validator to 1.0 first then working on class-transformer.

We can ask a review from @pleerock, if he finds it ok, we can merge.

[NoNameProvided]

Will try to go through the PRs during the easter. :) No promises tho.

[janis91]

@NoNameProvided Please do not get me wrong. I just wondered if I have to to do something else or if it's just because you are busy (no problem with that, I have that problem for my projects as well). Still I would appreciate it if you have the time during easter :-)

[mustafaekim]

Hi @janis91, the PR seems to cover only arrays having different type of objects. However a single property can also have a value of different object instance. Maybe my first example mislead-ed.

[janis91]

@mustafaekim You're right, the PR covers arrays only. But this is because it is already possible to give a function that determines the type based on the given TypeOptions for single properties. The only thing that was missing, was the possibility to differentiate objects inside of an array.

But in order to be able to provide multiple discriminator functions also for a single property, I could try to add the logic for this case as well.

[mustafaekim]

OK thanks. I would prefer adding a property (whos key can be set) to the json (discriminator property) indicating the class type.

[janis91]

@mustafaekim could you provide an example, I think I am not understanding what you mean.

[mustafaekim]

That's how you discover the constructor function:

export abstract class Photo {
    id: number;
    filename: string;
}

export class Landscape extends Photo {
   panorama: boolean;
}

function isLandscape(value: any): Function | false {
    return value.panorama !== undefined ? Landscape : false;
}

export class Album {
    id: number;
    name: string;
    @Type(() => [ isLandscape, isPortrait, isUnderWater ])
    topPhoto: Landscape | Portrait | UnderWater;
}

What I propose is, when transforming from class to json, it may be better to add a discriminator property (like __type) to the json so that when we transform back, we can simply check the value of the discriminator property and find the constructor. Your solution may be optional but by default a adding a discriminator property will work well for 99% of cases

That's how typedjson, Jackson (Java) work

[janis91]

@mustafaekim I tried what I think was your idea. Please check it out.

[mustafaekim]

Hi, I've read the code, it's like the way I was trying to describe.

can't the code below

topPhoto: Landscape | Portrait | UnderWater;

be

topPhoto: Photo;

?

[janis91]

@mustafaekim Well, actually the topPhoto can only be of type Photo, when the json input doesn't match the other types (because it is used as a fallback).
But in general I would say, that you lose the support of typescript in terms of auto completion and so on, if you just type Photo (it is most likely that you will not only get a Photo, but a Landscape, Portrait or UnderWater photo and you may want to use the different properties they introduce.

[mustafaekim]

I didnt know that in Typescript if you use the type Photo, you do not get support for tooling. That's interesting. That is not how it works in tooling for other languages. Anyway thank you for clarification.

Is there any schedule when this feature will be merged into the core of the library? (@NoNameProvided )

[cojack]

Hi
@janis91 thanks for your work, I have accept your PR and will be published into next version as 0.1.11

Thanks for your time 😍
Please feel free to contribute for another PR!

[lucaswxp]

Hello guys.

Was this solved? Is it already stable?

[agersea]

+1 any ETA on this?

[github-actions]

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.