Skip to content

Add System.Net.ServerSentEvents to runtime shared framework #112930

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

Merged
merged 3 commits into from
Feb 27, 2025

Conversation

captainsafia
Copy link
Member

This enables us to support SSE in minimal APIs for dotnet/aspnetcore#56172.

@Copilot Copilot AI review requested due to automatic review settings February 25, 2025 23:11
Copy link
Contributor

@Copilot Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot reviewed 1 out of 1 changed files in this pull request and generated no comments.

@stephentoub
Copy link
Member

@ericstj, how should we think about whether System.Net.ServerSentEvents should be in netcoreapp vs aspnetcoreapp?

@ericstj
Copy link
Member

ericstj commented Feb 26, 2025

If the library is generally useful to all .NET customers and it should be available to libraries that target .NET then it should be in the base netcoreapp shared framework.

If it's only needed for folks building aspnet core applications or libraries that are only used in aspnetcore then it can go in the ASP.NET shared framework.

My understanding of this library is that it has nothing to do with ASP.NET and can be used by anyone making Http requests - that leads me to believe it should be in the base shared framework, so that would be my suggestion.

@captainsafia
Copy link
Member Author

I think I've done the needful to move the library to the runtime shared framework. Let me know if i missed something.

@captainsafia captainsafia changed the title Add System.Net.ServerSentEvents to ASP.NET Core transport package Add System.Net.ServerSentEvents to runtime shared framework Feb 26, 2025
@ViktorHofer
Copy link
Member

Now that you switched the library to be part of the Microsoft.NETCore.App shared framework, you need to define its inbox dependencies in the ref and source project.

@captainsafia
Copy link
Member Author

Pushed a change with the applicable dependencies here.

image

@captainsafia captainsafia merged commit 478cfe2 into main Feb 27, 2025
80 of 83 checks passed
@jkotas jkotas deleted the safia/snsse-aspnet branch March 16, 2025 05:27
@ericstj
Copy link
Member

ericstj commented Apr 4, 2025

@captainsafia @eiriktsarpalis - should we add a note to dotnet/core#9824 about SSE being part of the framework and no longer requiring a package?

@ViktorHofer
Copy link
Member

The library is missing from the PackageOverrides.txt file which is the input to NuGet package pruning and conflict resolution. It should get added.

I filed an issue recently to add automation for generating that file here in runtime.

@stephentoub
Copy link
Member

@captainsafia @eiriktsarpalis - should we add a note to dotnet/core#9824 about SSE being part of the framework and no longer requiring a package?

For the most part this doesn't really affect people, other than their app might be a bit smaller.

@github-actions github-actions bot locked and limited conversation to collaborators May 5, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants