Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Microsoft.AspNetCore.JsonPatch.JsonPatchDocument should use System.Text.Json as the default JSON formatter #14035

Closed
ebizupnorth opened this issue Sep 16, 2019 · 6 comments
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates

Comments

@ebizupnorth
Copy link

Is your feature request related to a problem? Please describe.

A clear and concise description of what the problem is.
Example: I am trying to use Microsoft.AspNetCore.JsonPatch.JsonPatchDocument but it requires Newtonsoft.Json

Describe the solution you'd like

Since, NET Core is moving away from Newtonsoft.Json dependency as default JSON formatter, it is logical to upgrade JsonPatchDocument. Yes, we can simply add AddNewtonsoftJson() on the service collection instance, but it replaces the default System.Text.Json formatter for all JSON which is very unfortunate and defeat the purpose of moving away from using Newtonsoft.JSON.

Is there any workaround to configure JsonPatchDocument to use Newtonsoft.Json without calling .AddNewtonsftJson()?

@pranavkm pranavkm added the area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates label Sep 16, 2019
@mkArtakMSFT
Copy link
Member

Thanks for contacting us, @ebizupnorth.
This is not something we plan to do in the near future.

@pranavkm
Copy link
Contributor

@eakoning #12994 (comment) has an example of configuring the JsonPatchInputFormatter and the System.Text based formatter.

@Paul-Dempsey
Copy link

(originally from dotnet/AspNetCore.Docs#15332 asked to move here by Rick Anderson)

I really appreciate the followup.

The key point in #14035 is this:

Yes, we can simply add AddNewtonsoftJson() on the service collection instance, but it replaces the default System.Text.Json formatter for all JSON which is very unfortunate and defeat the purpose of moving away from using Newtonsoft.JSON.

Which is disappointing. I’m hoping there are coherent plans to move to a full utf-8 string / json stack and avoid much of the transcoding and allocation in the pipeline (see Utf8Json benchmarks).


Please reconsider this, as requiring the older package and bringing legacy forward is not moving in the right direction.

@RehanSaeed
Copy link
Contributor

Please consider re-opening this for 3.1.

@mkArtakMSFT
Copy link
Member

@RehanSaeed we won't be changing this in 3.1. Feel free to file a new issue and we can re-evaluate this ask for 5.0 release.

@RehanSaeed
Copy link
Contributor

New issue raised in #16968

@ghost ghost locked as resolved and limited conversation to collaborators Dec 11, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates
Projects
None yet
Development

No branches or pull requests

5 participants