Skip to content
This repository was archived by the owner on Apr 8, 2020. It is now read-only.

Should Html.PrimeCache be named Html.PrimeCacheAsync? #246

Closed
justinvp opened this issue Aug 6, 2016 · 4 comments
Closed

Should Html.PrimeCache be named Html.PrimeCacheAsync? #246

justinvp opened this issue Aug 6, 2016 · 4 comments

Comments

@justinvp
Copy link

justinvp commented Aug 6, 2016

MVC APIs that return Task/Task<T> have the Async suffix, e.g. Html.PartialAsync.

For consistency with MVC APIs and as an indicator to developers that they should @await the result, shouldn't Html.PrimeCache be named Html.PrimeCacheAsync?

@SteveSandersonMS
Copy link
Member

Yes, I think you're right! Will add that as a new overload and will mark the old one as obsolete.

@justinvp
Copy link
Author

justinvp commented Aug 8, 2016

While you're at it, maybe the new overload should return Task<IHtmlContent> instead of Task<HtmlString> (like Html.PartialAsync), which would allow the results to be written to a TextWriter instead of having to concatenate/format/allocate a (potentially large) string to return with HtmlString? HtmlString itself implements IHtmlContent so the existing implementation could remain as-is for now; returning Task<IHtmlContent> leaves the door open to perf improvements in the future.

@justinvp
Copy link
Author

justinvp commented Aug 8, 2016

(To give you an idea, I prototyped some potential changes to the existing implementation earlier this weekend: 4c7bdcf)

@SteveSandersonMS
Copy link
Member

Implemented. As per your suggestion, also changed the return type to Task<IHtmlContent>. Currently it's still using HtmlString but could change to a custom less-allocatey alternative like your prototype in the future.

geirsagberg added a commit to geirsagberg/JavaScriptServices that referenced this issue Aug 17, 2016
* upstream/dev:
  Add ability to configure environment variables for Node instances, plus auto-populate NODE_ENV based on IHostingEnvironment when possible. Fixes aspnet#230
  Rename PrimeCache to PrimeCacheAsync (keeping older name as obsolete overload). Fixes aspnet#246.
  Prerenderer now passes original (unescaped) URL to Node - fixes aspnet#250
  In WebpackDevMiddleware, allow configuration of ProjectPath (implements aspnet#262)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants