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

Removed referencing System.Text.Json when targeting modern .NET #497

Merged

Conversation

thompson-tomo
Copy link
Contributor

@thompson-tomo thompson-tomo commented Apr 16, 2024

Description

Resolves #496.

This removes System.Text.Json as a dependency for dotnet 6 due to it being natively provided by the framework hence the explicit dependency is not need for that TFM.

Checklist

  • [ ] Tests added
  • Version bumped
  • Changelog updated

@abatishchev
Copy link
Member

Can you cite any docs which would confirm that STJ is available without referencing it? If it is then which version? How to update it in the future, e.g. in case of a security vulnerability?

@abatishchev abatishchev self-assigned this Apr 16, 2024
@abatishchev abatishchev changed the title #496 tweak condition for STK to reduce dependencies Tweaked condition for referencing STJ to reduce dependencies Apr 16, 2024
@abatishchev abatishchev changed the title Tweaked condition for referencing STJ to reduce dependencies Tweaked condition for referencing System.Text.Json to reduce dependencies Apr 16, 2024
@thompson-tomo
Copy link
Contributor Author

@abatishchev easiest way to confirm it is look in Visual Studio, and expand out the framework under dependencies and that will reveal what is included in the framework. In the case of security vulnerability this made even easier to manage as when the framework used to compile the application gets a new release of STJ (within the same major), the client applications automatically get provided this based on their framework. Hence less maintenance effort.

@abatishchev
Copy link
Member

Let me check with the .NET team :) I'll circle back.

@abatishchev abatishchev changed the title Tweaked condition for referencing System.Text.Json to reduce dependencies Removed referencing System.Text.Json when targeting modern .NET Apr 18, 2024
@abatishchev
Copy link
Member

Spoke to the .NET team, was told this:

Don't reference it unless for the TFMs you target that don't have it implicitly.

Thanks for educating me, I dind't know. Will re-review the PR and get it going soon.

@abatishchev
Copy link
Member

@thompson-tomo can you please check why the build fails?

@thompson-tomo
Copy link
Contributor Author

thompson-tomo commented Apr 18, 2024

@abatishchev have tweaked the conditions which has resolved the build issue

abatishchev
abatishchev previously approved these changes Apr 28, 2024
@abatishchev abatishchev force-pushed the chore/#496_OnlyKeepSTJForNetStd2.0 branch from 2684905 to cfa2888 Compare April 28, 2024 16:09
@abatishchev abatishchev merged commit 0f31b2f into jwt-dotnet:main Apr 28, 2024
1 check passed
@abatishchev
Copy link
Member

Awesome, thank you again, @thompson-tomo!

@abatishchev
Copy link
Member

Published all packages to NuGet.org as beta1

@thompson-tomo thompson-tomo deleted the chore/#496_OnlyKeepSTJForNetStd2.0 branch April 28, 2024 21:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

System.Text.Json should only be a dependency for .NET standard
2 participants