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.

Sign up for GitHub

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

Jump to bottom

Why doesn't the ViewComponentResultExecutor use the IHttpResponseStreamWriterFactory #4885

Closed
davidfowl opened this issue Apr 5, 2018 · 7 comments · Fixed by #9395
Closed

Why doesn't the ViewComponentResultExecutor use the IHttpResponseStreamWriterFactory #4885

davidfowl opened this issue Apr 5, 2018 · 7 comments · Fixed by #9395
Assignees
@pranavkm
Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed
Milestone
3.0.0-preview5

Comments

@davidfowl
Copy link
Member

I'm trying to replace the HttpResponseStreamWriter with a (potentially) more efficient version and it turns out the ViewCompomentResultExector doesn't use it https://github.com/aspnet/Mvc/blob/133dd964abb1c2a4167cf38faa38fe0319b7b931/src/Microsoft.AspNetCore.Mvc.ViewFeatures/ViewFeatures/ViewComponentResultExecutor.cs#L108. Is that intentional or just an oversight?

/cc @rynowak @pranavkm
@pranavkm
Copy link
Contributor

pranavkm commented Apr 5, 2018

Likely an oversight. @rynowak was profiling view execution code paths - aspnet/Mvc#3248, might have missed view components.

@davidfowl
Copy link
Member Author

@mkArtakMSFT 2.2? Can't I just send a PR for this now?

@benaadams
Copy link
Member

Can't I just send a PR for this now?

@mkArtakMSFT
Copy link
Member

@davidfowl, do it please!

@davidfowl
Copy link
Member Author

I’ll do it

@mkArtakMSFT
Copy link
Member

@davidfowl 2.2, or 3.0?

@davidfowl
Copy link
Member Author

3.0 at this point.

@aspnet-hello aspnet-hello transferred this issue from aspnet/Mvc Dec 14, 2018
@aspnet-hello aspnet-hello assigned davidfowl and unassigned davidfowl Dec 14, 2018
@aspnet-hello aspnet-hello added this to the 3.0.0-preview2 milestone Dec 14, 2018
@aspnet-hello aspnet-hello added 1 - Ready area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. labels Dec 14, 2018
@pranavkm pranavkm modified the milestones: Backlog, 3.0.0-preview5 Apr 16, 2019
@pranavkm pranavkm assigned pranavkm and unassigned davidfowl Apr 16, 2019
@pranavkm pranavkm mentioned this issue Apr 16, 2019
Merged
@pranavkm pranavkm added Done This issue has been fixed and removed 2 - Working labels Apr 16, 2019
pranavkm added a commit that referenced this issue Apr 16, 2019
@pranavkm
Prevent synchronous writes when using Razor (#9395)
2be8052 
* Do not perform synchronous writes to the Response TextWriter after a Razor FlushAsync
* Use ViewBuffer to perform async writes to the response when using ViewComponentResult

Related to #6397 

Fixes #4885
@ghost ghost locked as resolved and limited conversation to collaborators Dec 4, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Assignees

@pranavkm pranavkm

Labels
area-mvc Includes: MVC, Actions and Controllers, Localization, CORS, most templates bug This issue describes a behavior which is not expected - a bug. Done This issue has been fixed
Projects
None yet
Milestone
3.0.0-preview5
Development

Successfully merging a pull request may close this issue.

Prevent synchronous writes when using Razor dotnet/aspnetcore
5 participants
@davidfowl @pranavkm @benaadams @mkArtakMSFT @aspnet-hello