Skip to content
This repository has been archived by the owner on Dec 19, 2018. It is now read-only.

Change TagHelperOutput Content, PreContent and PostContent to all be TextWriters to avoid large string allocations. #296

Closed
NTaylorMullen opened this issue Feb 6, 2015 · 4 comments

Comments

@NTaylorMullen
Copy link
Contributor

This issue is the more concrete version of #295.

What should be done to TagHelperOutput:

  1. Change TagHelperOutput.PreContent to be TagHelperOutput.PreContentWriter.
  2. Change TagHelperOutput.Content to be TagHelperOutput.ContentWriter.
  3. Change TagHelperOutput.PostContent to be TagHelperOutput.PostContentWriter.

All new property names here should be of type https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNet.Mvc.Core/Rendering/StringCollectionTextWriter.cs but exposed as ITextWriter. This means that type will need to be transitioned into the Microsoft.AspNet.Razor.Runtime repo.

Extra additions to make this change more consumable:

  1. ITextWriter should have an extension method to write on ITextWriter into another ITextWriter.
  2. TagHelperContext's GetChildContentAsync should have an override to provide your own ITextWriter to write to.

The impact of this change will end up affecting all existing TagHelper's and will greatly affect how TagHelpers used (NOTE: Lots of impact in MVC to make this plausible).

@NTaylorMullen NTaylorMullen added this to the 4.0.0-rc1 milestone Feb 6, 2015
@NTaylorMullen
Copy link
Contributor Author

@sornaks: @yishaigalatzer, @DamianEdwards, @dougbu, and I felt like this is a pretty large feature that should make it in sooner rather than later (since it has huge impact). We can sit down together tomorrow or when you're ready to pair program the beginning parts of this to give you enough context on what needs to be done (lotssss of pieces to this 😄).

@dougbu
Copy link
Member

dougbu commented Feb 6, 2015

All new property names here should be of type https://github.com/aspnet/Mvc/blob/dev/src/Microsoft.AspNet.Mvc.Core/Rendering/StringCollectionTextWriter.cs.

Do you mean "of type ITextWriter which we (in MVC) back using StringCollectionTextWriter"? If yes, StringCollectionTextWriter might not need to move.

@NTaylorMullen
Copy link
Contributor Author

Yes, muchas gracias 😄

@sornaks sornaks changed the title Change TagHelperOutput Content, PreContent and PostContent to all be ITextWriters to avoid large string allocations. Change TagHelperOutput Content, PreContent and PostContent to all be TextWriters to avoid large string allocations. Feb 19, 2015
@sornaks
Copy link

sornaks commented Mar 4, 2015

Checked in. aspnet/Mvc@284eb9a, bd9d57d

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants