-
-
Notifications
You must be signed in to change notification settings - Fork 27
Added optional argument / virtual getter to enable polymorphism #130
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
base: main
Are you sure you want to change the base?
Conversation
Hydro currently does not support inheritance / polymorphism in compontents. During the re-hydration from json it could be possible to declare custom converters in order to deserialize abstract or base types correctly. Since various conventions are possible to achieve this behaviour this commit aims to enable the user of this library to adapt as required.
@kjeske it would be nice if you would find some time to look at this PR with me |
Thank you for the contribution. Could you post an example what this PR exactly solves? Maybe there are some important conventions that should be put into Hydro by default? |
/// <returns></returns> | ||
public static IApplicationBuilder UseHydro(this IApplicationBuilder builder, IWebHostEnvironment environment, Assembly assembly) | ||
public static IApplicationBuilder UseHydro(this IApplicationBuilder builder, IWebHostEnvironment environment, Assembly assembly, Action<JsonSerializerSettings> serializerSettingsMutator = null) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any configurations should be a part of the HydroOptions
object that is initialized with the UseHydro
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The mutation of the default configurations is usually solved with the following syntax:
services.AddHydro(options =>
{
options.JsonSerializer.Converters.Add(customConverter);
});
So we would need to expose the JsonSerializer defaults as a property of HydroOptions
and then user can mutate it as they need.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe it would also make sense to unify and reuse some logic for the creation of serialization + deserialization logic, so a single setting is exposed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, definitely
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Any configurations should be a part of the HydroOptions object that is initialized with the UseHydro
I meant AddHydro
Imagine the scenario to have an abstract model (maybe bound to the frontend, maybe not). Currently hydro seems to serialize all properties within the component, which is fine by me, but if there are abstract classes being used in lists for example an issue arrises:
NewtonSoft.Json will serialize an Instance of Class B without any issues. But when Hdyro attempts to deserialize this class again, it becomes a bit tricky. Per default NewtonSoft will just throw an exception, because it cannot create an instance of an abstract type A. Using the type A as non abstract results in loss of 'SomeOtherProp's value or even disturbes deserialzing other types. A common pattern that I know would be, to add some custom converters, in order to add type flags. These can be used to determine the runtime type. There might be other viable solutions or use cases to modify this (de)-serialization settings. That's why I thought to expose them like this. What do you think about this? |
Ok, I get it. So yes, it sounds like something to extend manually during the startup in |
Hydro currently does not support inheritance / polymorphism in compontents. During the re-hydration from json it could be possible to declare custom converters in order to deserialize abstract or base types correctly. Since various conventions are possible to achieve this behaviour this commit aims to enable the user of this library to adapt as required.