y-crdt / ydotnet

.NET bindings for yrs.
MIT License
35 stars 9 forks source link

ToJson method for Output? #37

Closed SebastianStehle closed 10 months ago

SebastianStehle commented 11 months ago

Hi,

I would like to write some business logic, which observes changes on the background and invokes some business logic. I have tested it here: https://github.com/SebastianStehle/ydotnet/blob/main/Demo/Callback.cs

I think right now the type format is pretty complex for that. I would like to have a simple extension method like:

public static T To<T>(this Output output, Doc doc)
{
}

The idea would be to just loop over everything, write the content to a json buffer and deserialize it with System.Text.Json. Not very performance but better than writing your own serializer. What do you think?

LSViana commented 11 months ago

Hi, @SebastianStehle!

This kind of method could come in handy but it's hard to ensure it'll be reliable and will not throw runtime errors that could've been caught during compilation.

For example, creating such method could, theoretically, allow the usage like this:

var output = map.Get(transaction, "key");
var value = output.To<double>(); // Supposing this is possible

But how would this method be implemented to avoid cases like:

class User { ... }

var output = map.Get(transaction, "key");
var value = output.To<User>();

Where the generic type isn't available as one of the valid types an Output cell can hold?

Or, for example, if we make an extension method that takes a Doc instance, would it need to create a new Transaction for every read operation?

All these details have to be considered so the library remains useful for more people. These, among others, are the reasons why I haven't implemented these "shortcuts". Right now, the API is rigid but very safe to use because, for example, if the cell isn't holding the expected type, it'll just return null.

The kind of extension method you're asking for, as I see it now, should probably be implemented in your project for now. Also, please note that the API of the library doesn't have any syntax sugar but as the usage grows some might be added.

SebastianStehle commented 11 months ago

It is not casting, it is serialization. So you would get one of 2 errors;

  1. Xmltext not supported
  2. Json serialization exception

I think in general returning null is not a good idea. There are a lot of cases in the api where I would throw an exception instead.

LSViana commented 11 months ago

Hi, @SebastianStehle!

So if you're talking about serialization instead of casting, this should most likely live outside the YDotNet library.

I'm mentioning it here because, for example, the Yrs library uses lib0 for serialization but it's a separate part that acts on Input & Output cells, not something that's inherently part of the core library (as you see it here).

In any case, what's the scenario you need to serialize everything in a document? If you're doing it for synchronization purposes between clients (or server & client), it's most likely something that you shouldn't be doing with JSON.

SebastianStehle commented 11 months ago

So if you're talking about serialization instead of casting, this should most likely live outside the YDotNet library.

I would serialization to implement it. Because I think it does not make sense to write own converters from YArray to List and so on. Therefore write everything to Json and then use System.Text.Json

So something like

public static T To<T>(this Output output, Doc doc)
{
   var sb = new StringBuilder();
   var writer = new JsonWriter(sb);

   if (output is Number number)
      WriteJsonNumber(writer , number);
   else if (output is Object obj) {
      WriteJsonObject(writer , obj);
   }

   return Serializer.Deserialize<T>(sb);
}

In any case, what's the scenario you need to serialize everything in a document? If you're doing it for synchronization purposes between clients (or server & client), it's most likely something that you shouldn't be doing with JSON.

As said: Only for converting and to implement business logic on the client side. All the internal formats are byte arrays basically, I understand that. My use case is that I have YArrays for comments and I would send out notifications (or update other arrays in other documents) when a user is mentioned.

I can totally implement myself, but I think it would be useful because it would be relatively hard to work with the Objects and so on directly.

LSViana commented 11 months ago

Hi, @SebastianStehle!

Okay, I understand that. In this case, would it be acceptable to you if those extension methods were provided as a separate package? I already had plans to have separate packages for each network provider anyway, so this package could follow the same standard.

SebastianStehle commented 11 months ago

Sure. If you have a look to my repository, I have followed the same approach. Happy to provide a PR if you are interested.

LSViana commented 11 months ago

Hi, @SebastianStehle!

I'm definitely interested in it. I didn't have time to check it yet but I'll do it as soon as possible.

Since this is a side project for me at the moment, I don't have lots of time available or, some days, no time at all.

LSViana commented 10 months ago

Resolved by #55.