8000 Added optional argument / virtual getter to enable polymorphism by R3dByt3 · Pull Request #130 · hydrostack/hydro · GitHub
[go: up one dir, main page]
More Web Proxy on the site http://driver.im/
Skip to content

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

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

R3dByt3
Copy link
@R3dByt3 R3dByt3 commented Jun 5, 2025

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.

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.
@R3dByt3
Copy link
Author
R3dByt3 commented Jun 21, 2025

@kjeske it would be nice if you would find some time to look at this PR with me

@kjeske
Copy link
Contributor
kjeske commented Jun 23, 2025

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)
Copy link
Contributor

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

Copy link
Contributor

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.

Copy link
Author

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.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, definitely

Copy link
Contributor

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

@R3dByt3
Copy link
Author
R3dByt3 commented Jun 23, 2025

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?

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:

public abstract class A
{
  public int SomeProp { get; set; }
}

public class B : A
{
  public int SomeOtherProp { get; set; }
}

public class SomeComponent : HydroComponent
{
  public List<A> SomeObjects { get; set; } = new() { new B { SomeOtherProp = 5 } };
}

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?

@kjeske
Copy link
Contributor
kjeske commented Jun 23, 2025

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 AddHydro.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants
0