class-transformer
class-transformer copied to clipboard
Implement class-level @Type decorator polymorphism
Here is an option for improving the polymorphism in class-transformer. Closes #51.
This PR extends the existing @Type()
decorator to be usable at class level. This allows the user to decorate a base class, with a custom function to resolve the actual target type for the transform before mapping properties.
This allows polymorphic top-level transformations using plainToClass
and classToClass
variants, by adding a type mapping function to the parent (for example):
ie.
@Type(
opts => opts.object.discriminator === 'child1' ? Child1
: opts.object.discriminator === 'child2' ? Child2
: Parent
)
class Parent {
discriminator: 'child1' | 'child2' | 'parent';
id: number;
data: string;
}
class Child1 extends Parent {
child1Data: string;
}
class Child2 extends Parent {
child2Data: string;
}
const plainChild1 = {
discriminator: 'child',
id: 1,
data: 'mainData',
child1Data: 'extraChildData',
};
// obj will be a Child1 object
const obj = plainToClass(Parent, plainChild1);
My main reason for implementing this functionality is for use in transforming and validating polymorphic view models from JSON API request bodies.
This PR also allows for polymorphic transformations of array or map items, such as those detailed in PR #125, but implemented with a different (IMHO more simple) methodology which better fits within the existing class-transformer
infrastructure:
const getPhotoType = (value: any) => {
let resolvedType =
value.panorama === undefined ? LandscapePhoto
: value.person !== undefined ? PortraitPhoto
: value.depth !== undefined ? UnderWater
: undefined;
if (resolvedType === undefined) {
throw new Error(`Unable to determine photo type`);
}
return resolvedType;
};
@Type(opts => getPhotoType(opts.object))
export abstract class Photo {
id: number;
filename: string;
}
export class LandscapePhoto extends Photo {
panorama: boolean;
}
export class Person {
name: string;
}
export class PortraitPhoto extends Photo {
@Type(() => Person)
person: Person;
}
export class UnderWaterPhoto extends Photo {
depth: number;
}
export class Album {
id: number;
name: string;
@Type(() => Photo)
topPhoto: Photo;
@Type(() => Photo)
photos: Photo[];
}
let album = plainToClass(Album, albumJson);
In this case, both album.topPhoto
and the items in album.photos
will be instances of one of the Photo
subclasses.
I'm happy to do more work on more unit tests and documentation but wanted to check that this solution is acceptable before doing further work on it.
@yazshel Hi, I have check your PR, but it fail at test,
Could you fix these, and we will go forward.
It appears that commit ff5fcefb163bde31a70d2f7792fe4022ba6c95c0 has introduced significant changes to MetadataStorage
, which break this PR ... I'll try to have a look and fix it over the next few days.
I've just fixed it - the new Map
-based MetadataStorage
actually makes this a whole lot easier; I've rebased to the latest version and removed the MetadataStorage
change in favour of using MetadataStorage.findTypeMetadata()
.
@cojack or @NoNameProvided could one of you also review this.
Still conflicting. Can you rebase it once more @yazshel? At first glance, I like the API surface 👍, but I would like to review the final state, so nothing can go wrong during the rebase/merge.
Hi, any update about this PR? I really need this feature.
I've rebased it as requested but it's still pending... The rebase was necessary to work around conflicting changes introduced in PR #125 (for which I originally intended this PR as an alternative method for implementing polymorphic functionality).
It should be working now - all the tests passed at the time I force-pushed the rebased changes. Can it be approved again as originally planned please?
@NoNameProvided Could you take a look otherwise I'd merge it on sunday
I'm really excited for this one! 👍 Been bashing my head over this very problem.
Would also love to see this get merged for some stuff I've been working on lately. @MTschannett is there anything still blocking this from getting closed?
Hi . No there should be nothing that is blocking this . I forgot to merge it . sorry ^^
I'll merge this PR as soon as i got back to my pc. I can't merge it on my mobile.
It looks like i cannot merge this until a second review has been made. I'm sorry guys. Maybe one of you could make one?
I'm sorry for the delay. Happy new year to everyone :)
It looks completely dead. @MTschannett Can you promote someone of us to reviewer? Just for this PR only. I think we really need this feature.
@pankyka Sure . I'll try to add you today or tomorrow if that's ok for you.
I'm ready!
I'm still waiting for your response :)
@pankyka When is this going to be merged? :)
@branimir93 I don't know. I have still not been appointed for review :(
@MTschannett There's a number of people hoping that this can be merged through. Any chance that you can give us a hand?
I do need this feature too for my project! Is it possible to get this in several days or weeks?
Love to see this merged asap, need this too!
I hope this will be merged one time
@MTschannett What can I do to help this get merged? Would be supremely useful for me right now.
Guys I'd say this repo is abandoned, sadly. I've been following this for a while and there is just no progress. #314
Hi team,
Nice to see a fair bit of commit activity on this project over the past few months 👍
I'm just following up this PR - it's been over 2 years since it was ready to be merged but no second approver for the changes was available, and there are now several merge conflicts due to subsequent changes on master
. Is this PR likely to get merged If I fix these? I've put in a fair bit of effort into syncing with breaking changes in master
several times previously but would be happy to sync again if I know it's definitely going to make it in.
Cheers,
Timshel
Hi guys,
I think a lot of people really need this feature. It's really sad to see that the work have already been made but the PR was never merged for no reason..
Can you please answer to @yazshel so that he can know if it worth it to fix the current conflicts ?
Thank you in advance guys :)
Looks like this PR needs a merge from the latest master. If I can resolve the merge conflict, are there any maintainers who would be willing to review and approve? This feature is sorely needed for almost 2 years.
Guys please, this one is really needed! Currently it requires a lot of time to implement workarounds and maintain them all over the places to do what this feature can do with a few lines of code.
If there is any help that I can provide in moving it forward, I'm ready to contribute
A question - defining discriminator on the parent assumes importing subclasses in the parent class which introduces a circular dependency which breaks the entire logic. How do you overcome that?