stiankroknes / VisNetwork.Blazor

Blazor component for vis-js/vis-network javascript library.
https://stiankroknes.github.io/VisNetwork.Blazor/
MIT License
24 stars 9 forks source link

[Discussion] Handling json that doesn't encode the full object #28

Open jbgh2 opened 1 year ago

jbgh2 commented 1 year ago

@stiankroknes I've come up with a potential solution to the problem that the output of the DOT parser isn't necessarily complete objects since vis.js supports simplified form for some of it's parameters.

Example with tests in #29

The basic problem is that the default JsonConverter is expecting an json object and throws an exception when it gets something else. The standard way around this to to write a custom JsonConverter<T> (which I did, see ArrowsJsonConverter in the PR) However, we want the default converter when there is an object but to do something special when there isn't.

Proposed solution: For types that have a simplified format (like Arrows and Font) create a class that derives from the proper object and knows how to construct a full object from the simplified form. Then create a converter for the wrapper class (and when it detects the simplified form it constructs from that), otherwise it can get the default converter and use that to deserialize the full object.

In the branch the Arrows and Font classes are now the wrapper classes around ArrowsInternal and FontInternal. I gave the wrapper classes the original names so when you are constructing the network in code it looks correct and you don't have to worry about the wrapper/internal versions unless you are deserializing from the simplified format.

I tried to add some high level checks where it looks for either a string or number token to convert from but this might be overkill and it should just take the string output and try and work with that.

Have a look and I'm happy to take any feedback, I feel like there are some ways to simplify it but my C# isn't quite good enough to know how to.

stiankroknes commented 1 year ago

Thanks for investigating and the proposed solution. :thumbsup:

I like the way you solved the conditional type handling. The only concern I have is that xxxInner types will be part of the public API. Probably possible to find some way around this.

When thinking more about this feature I wonder if the parsed network need to be returned back to Blazor component ? Something like this would parse and render the parsed data.

 const network: Network = getNetworkById(element.id);

    const parsedData: any = parseDOTNetwork(dot);

    if (!parsedData) {
        console.warn('VisNetwork.Blazor: [parseDotNetwork] no parse result.');
    }

    var data = {
        nodes: parsedData.nodes,
        edges: parsedData.edges
    };
    network.setData(data);

    if (parsedData.options) {
        network.setOptions(parsedData.options);
    }

This way we avoid the serialization issues but not providing the same API as vis-network, may be ok.

If the parsed network data must be returned we can investigate the serialization strategy further. Could we define an internal definition of the complete Network options using object type for conditional types/discriminated unions. Wonder if the conditionally type handling in WeatherForecast example would work. The one with

public class WeatherForecast
{
    public object? Data { get; set; }
    public List<object>? DataList { get; set; }
}
stiankroknes commented 1 year ago

Initial support for DOT parsing implemented in #30. Current implementation only parses and sets the data, no options returned.

Issue regarding serialization for options is still present.