Discussion: bringing back @helper for views

[NickCraver]

In porting some existing applications (from ASP.NET MVC 5), we're seeing immense amounts of pain with views that use @helper today. @helper, from the user view point, is a very simple way to use Razor templating to re-use some code. Perhaps to render a date a certain way, or a chart, or a graph, or whatever. As an example, Stack Overflow's codebase has 458 @helpers today.

Some history:

• @helper was removed in #281 after findings in Add support for using @helper across views aspnet/Mvc#1130
• Parser errors were added in Add temporary parse error for @helper directive to avoid confusion. aspnet/Razor#472,
• A lot of user discussion in Add an HTML-formatted constructions support into Razor instead of the removed @helper directive. aspnet/Razor#715, ultimate closed as "We have no plans to do this"

But @helper still lacks an equally (or even close) usable replacement. The alternatives I'm aware of today are:

• Tag Helpers which involve writing C# per helper
• View Components which involve writing C# and a view per helper
• Using a dynamic workaround which isn't type safe and I'm not sure on the performance implications of
• Build the string yourself, and hope you encode everything correctly

There are 2 ways @helper worked in ASP.NET MVC 5: globally from App_Data and in individual views. I am only advocating for support of the latter. IMO, Tag Helpers or view components (pick your flavor!) are excellent replacements for the global use case. That makes total sense and having another view file is also reasonable.

Let's say we have a view today with 5 @helper sections in ASP.NET MVC 5. In ASP.NET Core, it's a regression. We have a few options:

• Copy the code everywhere
• Write a function rendering HTML strings (basically no Razor or safety for the helper)
• Have a child view per helper, going from 1 view to 6, and 1 file to 6
• Create a tag helper for each (5 classes...and again mostly removing HTML safety, or suffering overhead)
• Use the dynamic approach, eat the performance, and hope it doesn't go boom at runtime

Note: I am not sure about the performance differences on these. That needs to be tested. I'll try and get benchmarks going when time allows. I have a hard time imagining any of these would be as fast as what @helper used to do, though. But I take nothing for a given here...we need to benchmark, even if @helper isn't a candidate yet.

A lot of users have chimed in on closed issues above. A lot of migration pain is being felt. Probably much more than was envisioned when @helper was removed. This is one of the largest issues I see in our migrations and it's something I'm really feeling right now. Is there a possibility we bring @helper or equivalent functionality (re-using some Razor templating inside the same view) back?

cc @DamianEdwards @rynowak

[m0sa]

Just found this porting endeavor in our codebase. The dynamic workaround you mention doesn't need to be dynamic. It can be strongly typed:

Before:

@helper SomeHelper(SomeType item) {
/* CODE */
}

After:

@{Func<SomeType, IHtmlContent> SomeHelper = @<text>@{
/* CODE */
}</text>;}

[mkArtakMSFT]

Thanks for contacting us, @NickCraver.
The suggestion @m0sa has provided is the recommended alternative for the individual view case you're referring to. As you can see, it's very close to what it looked like with @helper.

[johnhargrove]

The workaround is a syntactic mess. A good short-term workaround, but hardly a replacement.

[NickCraver]

I need to setup benchmarks here (won't happen before the break), but we'd be allocating the function on every razor render, aren't we? That'd be a near-linear scale of cost with the size of the helper. Going to try and play with more options here to see what's allowed.

Note: I think the Func<T, IHtmlContent> approach above only works for a single item. I agree it's a mess though, this can't be the recommendation can it?

[rynowak]

If you have a good representative benchmark it would be great to add something to https://github.com/aspnet/Mvc/tree/master/benchmarkapps/RazorRendering - @benaadams contributed what's there, and it's immensely useful to us to get real examples.

[NickCraver]

@rynowak Thanks for the pointer! We dug into this today and were a little surprised there were no render time benchmarks directly runnable in the Razor repo - it looks like everything these is compiler performance. In the Mvc repo you can do some benchmarking with external tool captures, but we're thinking there should be some BenchmarkDotNet benchmarks readily runnable here.

I get that Razor -> .cs -> compiled is a 2 phase pass the latter of which is Roslyn and that's why things are the way they are, but when we want to test how things actually run (what really impacts us at scale) the compiled performance is the part we care about. To that end, we went through options and @m0sa is going to try over the next few days to get that benchmark pipe setup in the Razor repo in the existing benchmarks project so that we can compare the various helper workarounds and have it setup for anyone to test anything with boxing, allocations, etc. Hopefully we can make this easy for everyone and help advance discussions and options here with some data.

If for some reason we can't do that, we'll just razor gen the various options and throw the .cs in a benchmark repo to continue with data here. But hopefully we can get the much more generally useful .cshtml benchmark pipeline going.

[rynowak]

@m0sa is going to try over the next few days to get that benchmark pipe setup in the Razor repo in the existing benchmarks project so that we can compare the various helper workarounds and have it setup for anyone to test anything with boxing, allocations, etc.

FYI in case you haven't seen the announcements - we're in progress of moving repos around. MVC and Razor's runtime support libraries will be moving to aspnet/AspNetCore and the Razor compiler features will be moving somewhere else.

[m0sa]

I don't even...

https://github.com/aspnet/Mvc/blob/a2c8537dd86474a1cb5bb3ea12ada4ecabd6ee4c/src/Microsoft.AspNetCore.Mvc.Razor/HelperResult.cs#L58

[m0sa]

I've gotten the Benchmark app to compile & execute a razor page:

m0sa/Razor@7b50c33

but this doesn't help much, since all of the things that actually run are implemented in https://github.com/aspnet/Mvc. Would it be OK to pull in MVC package refs for benchmarks?

[rjgotten]

@johnhargrove
The workaround is a syntactic mess.

You can make it less of a mess as well as support multiple parameters by using an extension method.
That incurs some added cost at runtime as well. Not sure how much of an impact it will have.

[m0sa]

Looks like benchmarking render times only makes sense with MVC in play, too. I've forked my benchmarks off there:

https://github.com/m0sa/Mvc

[rynowak]

My preferred solution for a new feature to supplant @helper would be to allow Razor to be used in method bodies inside @functions. For those uninitiated, @functions is the place to put members of the generated class. So the new feature would be "you can define a private method using Razor syntax inside of it". The nicest thing about this to me is that it's just C#.

One of the things that's clunky is that you have to put it inside a code block since it's void-returning. We don't have a way to know at codegen time if a method is void or not.

In views, this is a really simple solution because all of the state that Razor needs is part of the instance. This also has the benefit that it doesn't need to allocate an IHtmlContent to hold the output. The fact that we're not returning something is what messes with the usability.

In components, I can't think of a reasonable solution other than allocating a RenderFragment since the state needed to output markup isn't in a field. We've already explored options in this space and it seems somewhat orthogonal. We already decided to support Razor inside @functions as part of the Razor components effort, and this just raises the priority. However this does mean that the components story will work differently, and require users to understand the difference. We could try to massage the story for views to be more inline if necessary.

---

In a view

<ul>
@for (var i = 0; i < 3; i++)
{
    SayHello("Hi Nick", i);
}
</ul>

@functions {
    void SayHello(string message, int index)
    {
        @<li>@message - you are number @index</li>
    }
}

---

Related to this proposal, the only major pivot here I see is related to async. For components, the answer is clear, everything in the render path is sync.

For views it seems at first that we have a few options:

1. Require sync
2. Require async
3. Be flexible based on the method signature

Doing either 1 or 2 requires us to parse method signatures (ugh) and are more limited so doing 3 would be less work. The perils that you run into as a user if we make it flexible are the same as one might experience with normal C# async methods, but viewed through a fog.

a. If you define an async method but don't await the calls, you have a bug
b. If you use a tag helper (even for ~/ URLs) then your code is async (this is a strong reason to reject idea 1 above)
c. If you define a method as async to be safe and don't have tag helpers, then you'll get CS1998

Let's think about how we can mitigate all of these.

a. We could address this with an analzyer. I'm not sure what our plans are like in the shared framework world for delivering analyzers 😞 A slightly bigger hammer would be to overload Write(Task ...) with [Obsolete(...)] and make it throw at runtime. This won't help at design-time because the design-time codegen doesn't do the same things.

b. The nice thing about this is that using a tag helper doesn't add the async modifier to your method, so you can't accidentally end up with async void - you have to make the mistake. This would be another thing that would be nice to have an analyzer for because it's still too easy of a mistake to make.

c. I don't really have any idea for how to mitigate this. My usability concern with this is that tag helpers like ~/ are magical. You use ~/ and require async. You move that hardcoded URL into a variable and now you have a diagnostic CS1998. I guess my beef is with C# all up.

---

Here's some perf results:

BenchmarkDotNet=v0.10.13, OS=Windows 10.0.17763
Intel Xeon CPU E5620 2.40GHz, 1 CPU, 4 logical cores and 4 physical cores
.NET Core SDK=3.0.100-preview-009750
  [Host]     : .NET Core 3.0.0-preview1-26907-05 (CoreCLR 4.6.26907.04, CoreFX 4.6.26907.04), 64bit RyuJIT
  Job-HUYTCC : .NET Core 3.0.0-preview1-26907-05 (CoreCLR 4.6.26907.04, CoreFX 4.6.26907.04), 64bit RyuJIT

Runtime=Core  Server=True  Toolchain=.NET Core 3.0  
RunStrategy=Throughput  

Method	ViewPath	Mean	Error	StdDev	Op/s	Gen 0	Gen 1	Allocated
RenderView	~/Views/HelperDynamic.cshtml	113.33 us	2.2368 us	3.1357 us	8,823.6	0.3662	0.1221	22.62 KB
RenderView	~/Views/HelperExtensions.cshtml	78.15 us	1.5593 us	3.9120 us	12,795.7	0.2441	0.1221	28.9 KB
RenderView	~/Views/HelperPartialAsync.cshtml	770.84 us	14.5555 us	13.6152 us	1,297.3	1.9531	0.9766	182.62 KB
RenderView	~/Views/HelperPartialSync.cshtml	826.26 us	16.4214 us	19.5485 us	1,210.3	2.9297	0.9766	182.62 KB
RenderView	~/Views/HelperPartialTagHelper.cshtml	1,177.60 us	32.1279 us	31.5539 us	849.2	1.9531	-	216.18 KB
RenderView	~/Views/HelperSyntheticAsync.cshtml	43.87 us	0.8513 us	0.9462 us	22,795.7	0.1221	0.0610	10.79 KB
RenderView	~/Views/HelperSyntheticSync.cshtml	44.65 us	0.9041 us	1.5834 us	22,397.7	0.1221	0.0610	10.79 KB
RenderView	~/Views/HelperTyped.cshtml	89.35 us	1.6915 us	1.8099 us	11,192.5	0.2441	-	22.62 KB

#InliningWin

[NickCraver]

Noting here: we didn't get pinged on this milestone change (I guess GitHub doesn't do that). Stack Overflow's ASP.NET Core migration is still blocked by this. The delay to preview 3 is very frustrating as it hoses a lot of downstream plans indefinitely.

When will this land?

[danroth27]

Hi @NickCraver,

Unfortunately this didn't land for Preview 2. We've been redoing our build infrastructure and unfortunately that took more time than expected.

Since this feature will require VS work to enable the corresponding Razor tooling, we need to align this with work with a corresponding VS release. It's too late at this point to add this to the initial release of VS2019, so our new plan is to get this into the first preview of the first update to VS2019, which should align with .NET Core 3.0 Preview 4. I've update the target milestone accordingly.

Sorry about the inconvenience! Let us know if you have any further questions or concerns.

[KLuuKer]

We also used this allot in exactly the same way
having some local function to build nested html for some big tree's
and other stuff that was just repeated a bunch of times
great to see this feature come back!

[NTaylorMullen]

Added the ability to write markup in @functions {...} and local functions (@{ void Foo() { ... } }

dotnet/razor@5ffd84e