OrchardCMS / OrchardCore.Commerce

The commerce module for Orchard Core.
MIT License
212 stars 87 forks source link

Upgrade to Orchard Core 2.0 once it's released and remove Newtonsoft.Json from the code base (OCC-194) #362

Closed sarahelsaig closed 5 days ago

sarahelsaig commented 1 year ago

Orchard Core 2.0 will finally migrate to System.Text.Json and drop Newtonsoft.Json support. This much anticipated event should be followed up in Orchard Core Commerce as well.

Blocking:


Obsolete description:

Is your feature request related to a problem? Please describe

Currently OCC has serialization via Newtonsoft Json.NET and System.Text.Json. Both are actually used in different places. This makes things unnecessarily complicated as serializers have to be implemented in two different frameworks.

Describe the solution you'd like

Orchard Core still uses Json.NET at the core of its content system (see) and so do many other third party modules and libraries that build on OC. While the OC team has expressed intent to eventually switch over to STJ, this is not in the near future. Additionally based on previous comments (for example here) we might want to rewrite the STJ implementation from scratch if this ever happens. In the meantime, we should do the following to make the code more maintainable and approachable:

Describe alternatives you've considered

Alternatively we may fully commit to STJ-only, but I think at this point this would be premature, an enormous technical debt and a source of future headaches.

Jira issue

hishamco commented 1 year ago

Orchard Core is still using Newtonsoft Json.NET because of YesSQL but for sure if all the JSON stuff is implemented in STJ then the STJ wins

IMHO we could use STJ all over places if all agree

I might have a look to related issue in YesSQL to use STJ

sarahelsaig commented 1 year ago

Is there an open issue tracking the transition from Json.NET to STJ in Orchard Core? I can't find one, that's the main reason why I assumed it's off in the far future. I found an open YesSql issue for moving to STJ (https://github.com/sebastienros/yessql/issues/227), but it hasn't been touched since 2020.

If we can get the move to STJ started for YesSQL and OC then I'm all for it (and would like to get involved). But if it will be stuck in limbo for much longer then I'd still prefer to cut down on duplicate code in the meantime.

hishamco commented 1 year ago

I might check that issue again, so for now I think we can go with STJ if you don't mind or we will rid off STJ until we finish all the things in YesSQL

infofromca commented 4 months ago

Be careful, in my practice, not all kind of data would be processed by STJ easily. IMO, just keep using Newton, at least for a while. do not waste time to to modify the not high priority issues . after all the stuff of this project are ok, then consider transfer to STJ