Skip to content

Added optional argument / virtual getter to enable polymorphism#130

Closed
R3dByt3 wants to merge 7 commits into
hydrostack:mainfrom
R3dByt3:main
Closed

Added optional argument / virtual getter to enable polymorphism#130
R3dByt3 wants to merge 7 commits into
hydrostack:mainfrom
R3dByt3:main

Conversation

@R3dByt3

@R3dByt3 R3dByt3 commented Jun 5, 2025

Copy link
Copy Markdown
Contributor

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

R3dByt3 commented Jun 21, 2025

Copy link
Copy Markdown
Contributor Author

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

@kjeske

kjeske commented Jun 23, 2025

Copy link
Copy Markdown
Contributor

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?

Comment thread src/Configuration/ApplicationBuilderExtensions.cs Outdated
@R3dByt3

R3dByt3 commented Jun 23, 2025

Copy link
Copy Markdown
Contributor Author

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

kjeske commented Jun 23, 2025

Copy link
Copy Markdown
Contributor

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.

@R3dByt3

R3dByt3 commented Jul 16, 2025

Copy link
Copy Markdown
Contributor Author

@kjeske sorry for the delay - I have updated the code (hopefully) like you requested

@R3dByt3

R3dByt3 commented Aug 4, 2025

Copy link
Copy Markdown
Contributor Author

@kjeske bump

@kjeske

kjeske commented Aug 4, 2025

Copy link
Copy Markdown
Contributor

Thanks. Will run some tests shortly.

@kjeske

kjeske commented Aug 5, 2025

Copy link
Copy Markdown
Contributor

Moved to #136

I've added some changes to simplify the settings access, also added the docs.

@kjeske kjeske closed this Aug 5, 2025
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