search

Cysharp / Ulid

Fast .NET C# Implementation of ULID for .NET and Unity.
MIT License
1.32k stars 59 forks source link

Coupling of Ulid and System.Text.Json #79

Open manfred-brands opened 3 months ago

manfred-brands commented 3 months ago

Even though there is a separate Ulid.SystemTextJson project, containing the JsonConverter for Ulid. However the converter is already in the Ulid project which now has a hard-coded reference on System.Text.Json

I know it is convenient to have this attribute, but it is coupling.

Can this be decoupled? We have users on both NewtonSoft.Json and System.Text.Json and don't want to pull in the 2nd when using the 1st. We also have code using protobuf which should not pull in any Json package.

neuecc commented 3 months ago

I don't see a problem since there is no reference to an additional System.Text.Json, what is your concern? https://github.com/Cysharp/Ulid/blob/master/src/Ulid/Ulid.csproj

manfred-brands commented 3 months ago

I was equally confused by that, but ILSpy shows that dependency. Also the JsonConverter wouldn't compile without that reference.

neuecc commented 3 months ago

I don't think it exists in NuGet's dependencies either. https://www.nuget.org/packages/Ulid#dependencies-body-tab Of course, the new .NET has System.Text.Json in corelib, so it is referenced by default. The Ulid.SystemTextJson project exists as a package to support environments where System.Text.Json does not exist in the standard, such as netstandard 2.0.