search

event-driven-dotnet / EventDriven.EventBus.Dapr

Event bus abstraction over Dapr pub/sub
MIT License
22 stars 5 forks source link

Suggestions around JSON #1

Closed tonysneed closed 3 years ago

tonysneed commented 3 years ago

From @rynowak:

Here's some feedback based on my experiences supporting ASP.NET Core and Dapr for .NET. These are just my opinions so feel free to ignore my ideas if they are a bit fit for what you're trying to accomplish.

Registering options

Rather than registering the JsonSerializerOptions - think about piggybacking on DaprClient. DaprClient exposes the JSON settings as a property now. This gives the user one thing to configure. It will probably surprise people if two Dapr-related featuresets behave differently.

It will be non-obvious to users that you're registering a BCL type with DI. In fact since you're using AddSingleton instead of TryAddSingleton you might actually overwrite something the user did. A reusable library should use TryAdd... where it registers services. This way if someone calls your method twice, it behaves predictably.

Choosing serializer options

If you must have your own copy of the JsonSerializerOptions then use new JsonSerializerOptions(JsonSerializerDefaults.Web). This is the set of defaults we (ASP.NET Core, .NET BCL, Dapr) are standardizing on for web/microservices scenarios. This allows the JSON team to add new settings and all of us to be consistent. You will need to reference the System.Text.Json package if you are targeting 3.1.

Furthermore, if you have your own copy of options and they will be used for the users's data types - then you need to make it configurable. S.T.J doesn't have good support by default yet for F# - and folks need access to the settings for the workaround.

Polymorphic serialization

FYI there's nothing special about dynamic - using object does the same thing but without involving the DLR. (referring to this). This is a meme that was started somewhere and keeps getting repeated. At runtime, the type system sees object because dynamic is not a runtime type.

tonysneed commented 3 years ago

@rynowak: Thanks for this. Piggybacking on DaprClient for JsonSerializerOptions is a good idea.

FYI there's nothing special about dynamic. This is a meme that was started somewhere and keeps getting repeated.

Removing dynamic. BTW, I lifted this code verbatim (including the comment) from the eShopOnDapr project. 😄