Aspnetcore: Redesign the ViewEngine to be async

[mkArtakMSFT]

• IViewEngine could be designed to be made without relying on HTTP. This may allow for not-HTTP scenarios such as email generation in background tasks. An alternative to this would be to recommend using components for this.

• Requirements for resdesign:

  • Existing scenarios must work
  • Have a package that does runtime compilation. Cannot be shipped with the shared fx, but can depend on it.

• IViewEngine - FindView \ GetView could be merged if we can flow the ExecutingFilePath thru.

```C#
interface IViewFactory
{
IViewTemplatingSystem FindViewAsync(ViewFactoryContext); // This is the combined API
}

public readonly ViewFactoryContext
{
public ActionContext ActionContext { get; set; }

public string ExecutingFilePath { get; set; }

public string Name { get; set; }

public bool IsMainPage { get; set; }

}

IViewTemplatingSystem.ExecuteAsync(Options)
```

[mkArtakMSFT]

Moving this to the Backlog

[pranavkm]

Possible feature to consider: Allow for async view location expanders. See https://github.com/aspnet/Mvc/issues/7792

[effyteva]

Any updates on this one?
Took me a few hours to figure out we suffer from deadlocks caused by DisplayFor and EditorFor methods.
Please also see https://github.com/aspnet/KestrelHttpServer/issues/2375, which I believe is also affected from this issue.
Perhaps you should reconsider pushing this into the 2.2.0 release?

[Tratcher]

@davidfowl

[effyteva]

@mkArtakMSFT any chance this will get into the 2.2 release?

[effyteva]

I looked at the engine code, and believe I could easily pull a request for changing the ViewEngine to support async on the EditorFor/DisplayFor methods.
Would this help speeding it up?
I'm not used to open-source at all - not sure if the ASP.NET MVC source code is publicly managed, or actually controlled only by Microsoft employees...

[pranavkm]

@effyteva you're welcome to try, but the issue is that doing this without making a breaking change (so you keep the sync and async overloads at every abstraction) can become challenging.

[effyteva]

Great @pranavkm, I've created pull request for that

[mkArtakMSFT]

Reducing the priority, as this impacts only the non-default case, which is runtime-compilation scenario.

[effyteva]

Hi,
Any chance this will still make it to the 3.0.0 release?
There's a simple PR for that which I sent a few months ago: https://github.com/aspnet/Mvc/pull/8554

[pranavkm]

@effyteva I'd be happy to consider another PR if you send one. I did a fair bit of work to update the RazorViewEngine to be async here - https://github.com/aspnet/AspNetCore/commit/306c47f10004099200a92b148b9bc1e4704bfc77. You could consider using that as starting point.

[effyteva]

Thanks @pranavkm,
I believe you covered it on all https://github.com/aspnet/AspNetCore/commit/306c47f10004099200a92b148b9bc1e4704bfc77, as long as the EditorForAsync and the DisplayForAsync are there and working.
Did your commit merge into the 3.0 branch already?

[pranavkm]

The changeset doesn't cover tests or code review, there's a fair bit of that. Since the sync version of some of the HtmlHelper methods would continue to exists, we want some coverage for those. This would be the stretch goal, but we would also want something like the Html.Partial analyzer for DisplayFor \ EditorFor methods

[effyteva]

I have to say that's very disappointing.
The ASP.NET Core Razor engine currently suffers from deadlocks (see this https://github.com/aspnet/AspNetCore/issues/4755) due to having sync-over-async code.
We've been using a custom build of the ViewEngine for more than a year, and we'll have to keep creating new custom builds for each ASP.NET core release.

[mkArtakMSFT]

Closing as this is not something we plan to do in the near future.