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

Add .NET 7 and drop .NET 5 #463

Closed
Jericho opened this issue Aug 23, 2022 · 8 comments
Closed

Add .NET 7 and drop .NET 5 #463

Jericho opened this issue Aug 23, 2022 · 8 comments
Assignees
Labels
Breaking Change This change causes backward compatibility issue(s)
Milestone

Comments

@Jericho
Copy link
Owner

Jericho commented Aug 23, 2022

.NET 7 targeted to be Released November 2022 replaces .NET 5 as current release.

@Jericho Jericho added the Breaking Change This change causes backward compatibility issue(s) label Aug 23, 2022
@Jericho Jericho self-assigned this Aug 23, 2022
@Jericho Jericho changed the title Add .NET 7 Support and drop .NET 5 Add .NET 7 and drop .NET 5 Aug 23, 2022
Jericho added a commit that referenced this issue Nov 13, 2022
Jericho added a commit that referenced this issue Nov 13, 2022
@vyrotek
Copy link

vyrotek commented Nov 30, 2022

Hi @Jericho, When is this expected to be released?

We believe we're running into issues with our .NET 7 app using this library which is still targeting .NET 6.

@Jericho
Copy link
Owner Author

Jericho commented Nov 30, 2022

Is the issue you are facing similar to #476 by any chance?

That's the only problem I am aware of that's due to net6 vs net7.

My sense of urgency to change the framework targets has been low due to the fact that the only problem that I am aware of has been resolved and based on the fact than I didn't get any feedback (neither positive nor negative) on this issue. All this to say that I had not scheduled a specific date for releasing this breaking change.

Having said that, I'm curious to hear more details about the problem you are facing and I'm willing to adjust my sense of urgency based on what I learn.

@ninjapiratica
Copy link

ninjapiratica commented Nov 30, 2022

I work with @vyrotek and I'll give a few more specific details.

Yes, it is related to #476. We use dynamic templates and the DynamicData property is what is causing the issue. I don't know if .NET 7 targeting would solve it, or if your serializerOptions need to include the recommended action of adding the fallback as per the dotnet issue you referenced in #476 (dotnet/docs#30755).

But since it looks like that type doesn't even exist in 6, .NET 7 might be needed to resolve the problem?

The DynamicData property can't be added as a [JsonSerializable] attribute since you don't know the type it will be, and when we tried adding our own TypeInfoResolver property on the DynamicDataSerializationOptions you provided, it didn't seem to work, giving the same error.

If you want, I can provide a sample project and open a new Issue for the DynamicData property serialization to track it.

@Jericho
Copy link
Owner Author

Jericho commented Dec 1, 2022

Thanks for letting me know the problem was related to DynamicData. It allowed me to narrow down my investigation. Here's what I learned:

  • the root of the problem is not so much the fact that StrongGrid is targeting .net6
  • the problem is due to the fact that your app targets .net7 which forces System.Text.Json version7 to be used (despite the fact that StrongGrid was built with a reference to System.Text.Json version6)
  • System.Text.Json version 7 no longer falls back to reflection-based serialization by default (which is necessary to serialize dynamic data)
  • The fact that reflection-based serialization is not used means that StrongGrid does not know how to serialization your dynamic data and therefore explains why you are getting a serialization exception

The fact that a different version of the Json serializer/deserializer is used depending on the version of .net used in your app explains why we see different behaviors/problems when an app targets .net7 (this fact eluded me for a long time and I'm glad I finally figured this out. It was bothering me that I was unable to explain why the behavior was different in .net6 vs .net7). However, changing StrongGrid to target .net7 would not resolve the problem you are experiencing. It would simply ensure that System.Text.Json version7 would always be used and therefore all developers would experience the same problem you are reporting, even when using .net6. At least the problem would be consistent no matter the version of .net!

All this to say that to solve the problem, I need to make two changes:

  1. I need to upgrade System.Text.Json to version7
  2. I need to indicate that I want to use reflection-based serialization when processing dynamic data

I might as well change the target frameworks to include .net7 and to drop .net5 at the same time but I want to be clear that it's neither the root of the problem nor the solution.

I published a beta version with these changes to my MyGet feed. give me some fedback when you have a chance to test it.

@ninjapiratica
Copy link

That beta version seems to work for our use cases. No noticeable issues in our testing.

@Jericho
Copy link
Owner Author

Jericho commented Dec 1, 2022

Thanks for confirming. I'll publish the new version to NuGet as soon as I have some free time (probably this weekend).

@Jericho Jericho closed this as completed in a8c2707 Dec 2, 2022
@Jericho Jericho added this to the 0.96.0 milestone Dec 2, 2022
@Jericho
Copy link
Owner Author

Jericho commented Dec 2, 2022

🎉 This issue has been resolved in version 0.96.0 🎉

The release is available on:

Your GitReleaseManager bot 📦🚀

@vyrotek
Copy link

vyrotek commented Dec 2, 2022

@Jericho Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking Change This change causes backward compatibility issue(s)
Projects
None yet
Development

No branches or pull requests

3 participants