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

Remove Newtonsoft.Json from the shared framework #4260

Closed
7 tasks done
natemcmaster opened this issue Nov 27, 2018 · 38 comments · Fixed by #9476
Closed
7 tasks done

Remove Newtonsoft.Json from the shared framework #4260

natemcmaster opened this issue Nov 27, 2018 · 38 comments · Fixed by #9476
Assignees
Labels
accepted This issue has completed "acceptance" testing (including accessibility) area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework breaking-change This issue / pr will introduce a breaking change, when resolved / merged.

Comments

@natemcmaster
Copy link
Contributor

natemcmaster commented Nov 27, 2018

The following shared-framework libraries pull in Newtonsoft.Json:

  • - Microsoft.AspNetCore.Mvc.ViewFeatures
  • - Microsoft.Extensions.Configuration.Json
  • - Microsoft.Extensions.DependencyModel
  • - Microsoft.IdentityModel.JsonWebTokens
  • - Microsoft.AspNetCore.SignalR.Protocols.Json
  • - Microsoft.AspNetCore.Authentication.OAuth
  • - Microsoft.Extensions.Logging.EventSource

These assemblies and dependencies need to be updated to remove their dependency on Newtonsoft.Json.

@natemcmaster natemcmaster added breaking-change This issue / pr will introduce a breaking change, when resolved / merged. feature-platform Deprecated: Cross-cutting issues related to ASP.NET Core as a platform labels Nov 27, 2018
@natemcmaster natemcmaster added this to the 3.0.0-preview2 milestone Nov 27, 2018
@natemcmaster
Copy link
Contributor Author

natemcmaster commented Nov 27, 2018

Tracking external issues:

Internal changes needed:

  • @Tratcher - update Microsoft.AspNetCore.Authentication.OAuth
  • @rynowak @mkArtakMSFT - we need to update Microsoft.AspNetCore.Mvc.ViewFeatures to make sure it doesn't pull in Newtonsoft.Json.
  • Update our dependency on IdentityModel.

@natemcmaster
Copy link
Contributor Author

Also adding @mkArtakMSFT to help with MVC's dependency on Json.net and myself to coordinate with partners on getting Json.net removed.

@Tratcher Tratcher added the blocked The work on this issue is blocked due to some dependency label Nov 30, 2018
@natemcmaster
Copy link
Contributor Author

Pushing out to preview3. We are still blocked on https://github.com/dotnet/core-setup/issues/3311 and dotnet/extensions#568

@Tratcher Tratcher removed the blocked The work on this issue is blocked due to some dependency label Jan 23, 2019
@Tratcher
Copy link
Member

Partially unblocked by dotnet/corefx#34485

@joshfree
Copy link
Member

/cc @ahsonkhan and @steveharter who will have a porting-guide.md (soon) to assist with this

@muratg
Copy link
Contributor

muratg commented Jan 24, 2019

Adding @BrennanConroy for Microsoft.AspNetCore.SignalR.Protocols.Json

@BrennanConroy
Copy link
Member

JsonDocument doesn't help us, we need the serializer and deserializer :)

@BrennanConroy
Copy link
Member

Also, was there ever a discussion/decision for the netstandard issue?

@muratg
Copy link
Contributor

muratg commented Jan 24, 2019

@davidfowl
Copy link
Member

Yea there’s a source version of the reader and writer for ns2.0

@ahsonkhan
Copy link
Member

ahsonkhan commented Jan 24, 2019

Also, was there ever a discussion/decision for the netstandard issue?

Yea there’s a source version of the reader and writer for ns2.0

https://dotnet.myget.org/feed/dotnet-core/package/nuget/Microsoft.Bcl.Json.Sources

It contains the JsonDocument as well.

For guidance on consuming the source pacakge, visit: https://aka.ms/json-source-package-guide.

@BrennanConroy
Copy link
Member

JsonDocument is now available to our builds.

@natemcmaster
Copy link
Contributor Author

Looks like DependencyModel has removed Json.Net, Oauth and Config.Json are in the works (#7105, dotnet/extensions#1028) and I believe SignalR is done (#6977). I think we'll be able to complete this for Preview 3.

@bradygaster
Copy link
Member

Perhaps if scaffolding existed to add SignalR would be a good middle ground, but even that would have costs.

I'd prefer just litter all the templates with "opt-in" functionality for SignalR, which'd drop in what they need in Startup.cs and a sample Hub class. Yes, there's a cost, but I think we should start thinking about when to pay that cost. I'll set up another issue for us to use to discuss this idea, so we can re-focus this thread on the issue at hand.

@davidfowl
Copy link
Member

A SignalR template would be fine tbh, we resisted it for long but it would be a useful starting point as there's lots of moving pieces

@BrennanConroy
Copy link
Member

Added Microsoft.Extensions.Logging.EventSource to the list at the top of this issue

cc @pakrym

@Tratcher
Copy link
Member

Tratcher commented Feb 5, 2019

Microsoft.AspNetCore.Authentication.* are done.

@natemcmaster
Copy link
Contributor Author

This did not make it into preview3 and is being pushed out to preview4.

@HaoK
Copy link
Member

HaoK commented Feb 28, 2019

Config PR merged dotnet/extensions#1028

@pakrym
Copy link
Contributor

pakrym commented Mar 7, 2019

Logging done dotnet/extensions#1227.

@BrennanConroy
Copy link
Member

FYI need to update ResolveReferences.targets file after this is done
https://github.com/aspnet/AspNetCore/blob/17344cd37f0b43b28c93c58cba667e55bd680624/eng/targets/ResolveReferences.targets#L88

@Eilon Eilon added the blocked The work on this issue is blocked due to some dependency label Apr 5, 2019
@natemcmaster natemcmaster removed their assignment Apr 8, 2019
@mkArtak
Copy link
Contributor

mkArtak commented Apr 17, 2019

@pranavkm can you please confirm whether this is done already?

we need to update Microsoft.AspNetCore.Mvc.ViewFeatures to make sure it doesn't pull in Newtonsoft.Json.

@natemcmaster natemcmaster removed the blocked The work on this issue is blocked due to some dependency label Apr 17, 2019
@BrennanConroy BrennanConroy added the accepted This issue has completed "acceptance" testing (including accessibility) label Apr 25, 2019
@ghost ghost locked as resolved and limited conversation to collaborators Dec 3, 2019
@JunTaoLuo JunTaoLuo added area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework and removed feature-platform Deprecated: Cross-cutting issues related to ASP.NET Core as a platform labels Jan 28, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
accepted This issue has completed "acceptance" testing (including accessibility) area-infrastructure Includes: MSBuild projects/targets, build scripts, CI, Installers and shared framework breaking-change This issue / pr will introduce a breaking change, when resolved / merged.
Projects
None yet