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

Update SignalR service to .NET 8.0 #38

Open
wants to merge 6 commits into
base: master
Choose a base branch
from

Conversation

LanceMcCarthy
Copy link

There are two changes in this PR:

1. Update the SDK from netcoreapp2.1 to net8.0

This is done for both security and modernization considerations. After upgrading the SDK, some changes to how the routing is configured in Program and Startup is required. some highlights

  • Newtonsoft.Json is not required/supported.
  • SignalR hub registrations is done through app.MapHub<T>
  • AddCors() does not support the use of both wildcards and AllowCredentials() at the same time. this is a read-only hub, so we can remove AllowCredentials.
  • Use dependency injection for EntityFrameworkCore (see builder.Services.AddDbContext)

2. General housekeeping and modern C# expectations

There was a lot of C# 8 and C# 9 code. I cleaned this up a bit by using cleaner C# 10 considerations. Here are the results of the improvement.

  • Use file-scoped namespaces to significantly decrease visual clutter.
  • Use primary constructors where possible.
  • Use a proper Program.cs for .NET 8 (removes Startup.cs and unnecessary boilerplate).
  • Inject IWebHostEnvironment into SampleEntitiesDataContext to get App_Data directory's path
  • Use Dependency Injection for DbContent instead of manual instantiation (see ProductHub's primary contructor)

3. Testing

I've confirmed the SignalR service itself is running and operates as expected, however we should briefly test it using one of our real demos that consume the service.

@aleksandarevangelatov
Copy link

Huge thank you, Lance, for the effort in updating the service and apologies for the delayed PR review.

I just want to note that this PR should be merged together with PR updating the Kendo and Telerik MVC and Core demos.

@LanceMcCarthy
Copy link
Author

LanceMcCarthy commented Oct 11, 2024

Yes, the merging of this branch should be done at the same time as the other repo's update.

Important

We must also update the other projects, but I do not want to add those changes in this PR because it would introduce too many changes that need to be QA'd at the same time.
For example:

  • KendoCoreService needs a bump from .NET6 to .NET8 (this one is fast to do)
  • odata-v4 project is using vulnerable .NET Framework and dependencies (this one is probably 2-4 hours of work). It should also be changed to use csproj instead of packages.config, and there is alot of deprecated jquery/bootstrap

Bump EntityFramework Sqlite
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants