dotnet / aspnetcore

ASP.NET Core is a cross-platform .NET framework for building modern cloud-based web applications on Windows, Mac, or Linux.
https://asp.net
MIT License
35.45k stars 10.03k forks source link

Plugin System—Providing PartialViewResult with no Model #4886

Closed ghost closed 5 years ago

ghost commented 6 years ago

Could be related: https://github.com/aspnet/Mvc/issues/6984 https://github.com/aspnet/Home/issues/343 https://github.com/aspnet/Mvc/issues/4572#issuecomment-216351033

What about providing RenderPartialAsync/RenderPartialCoreAsync versions without object model, ViewDataDictionary viewData? Coupled with Application Parts, we would have, out of the box, everything ready to add a Plugin System. (Each Plugin with its own MVC, embedded in the "webapp-host" pages.)

mkArtakMSFT commented 6 years ago

@NTaylorMullen, any thoughts? Thanks! /cc @DamianEdwards

NTaylorMullen commented 6 years ago

Hi @ckams, could you provide an example as to what you're trying to accomplish? I'm a little confused because Html.RenderPartialAsync without passing the model or viewData is already a thing .

ghost commented 6 years ago

Hi, @NTaylorMullen, well, I mean without any model or ViewData. A kind of "pure raw output" from the source.

Here the context.

I have built a plugin system for my Asp.Net Core MVC webapp.

Diagram

plugin

Embedded Plugins

Each plugin have its own MVC components (With Get and Post… so View Components are not useful here). The plugins are of two kinds:

"Local plugin" "Api Provider based plugins"
Signalr based messenger Microsoft Api
Webmail Google Api
Statistics etc…

How it looks

plugin_system_architecture

Api Provider based plugins

I use Ajax call to load the content of "Api Provider based plugins" in div tags, as they are potentially "deadlocks" for my app (depending on the Api request, and failures, It can take at least 3 seconds to load the page, or unlimited time…).

"Local plugins"

Here is the problem. For example, my Signalr based or Statistics plugins and so, need their own model.

But, as we can see in the provided link, "the caller" of RenderPartialAsync feed the called controller with its own model or viewData, leading to mismatch between what the plugin controller need, and what the caller of RenderPartialAsync give.

For now I use for any plugin Ajax Call. But if we had something like "No Model at all RenderPartialAsync", we could use it as embed like tag for Asp.Net Core MVC.

Hope I am clear enough.

NTaylorMullen commented 6 years ago

Are you suggesting a different method that doesn't default Html.RenderPartialAsync(viewName)'s model to viewData.Model?

If so, is Html.RenderPartialAsync(viewName, model: null) sufficient?

ghost commented 6 years ago

Are you suggesting a different method that doesn't default Html.RenderPartialAsync(viewName)'s model to viewData.Model?

Yes

If so, is Html.RenderPartialAsync(viewName, model: null) sufficient?

As long as it does not feed with null object the called controller (always the mismatch problem between the needed model for the callee and the null object send…).

mkArtakMSFT commented 6 years ago

@NTaylorMullen, is there anything else you need to add here, or should this be closed already? Thanks!

ghost commented 6 years ago

@NTaylorMullen No comment?

ghost commented 6 years ago

@davidfowl What do you think about this? Do you have any suggestion? I have posted a topic on the ASP.NET forum if you want more details.

pranavkm commented 6 years ago

@ckams it sounded like there's an existing API that works for your scenarios today. Is this specifically about having an overload without a model parameter? That doesn't seem particularly high value.

As a side note, we also introduced a PartialTagHelper in 2.1.0, which we'd recommend over HtmlHelper.Partial* going forward. You should be able to specify a null model with it <partial model="null" name="..." />. Do these cover your concerns?

NTaylorMullen commented 6 years ago

@NTaylorMullen No comment?

@ckams I apologize for the deaf ear. We get a load of issues that we need to get through daily. I was under the assumption that the RenderPartialAsync alternative was sufficient. Is it not?

Also, @pranavkm's suggestion of partial TagHelper is a fine alternative as well.

ghost commented 6 years ago

@ckams it sounded like there's an existing API that works for your scenarios today. Is this specifically about having an overload without a model parameter? That doesn't seem particularly high value.

Indeed! And I prefer to depend the least possible to API, and keep, to the MAX, pure ASP.NET CORE code.

As a side note, we also introduced a PartialTagHelper in 2.1.0, which we'd recommend over HtmlHelper.Partial* going forward. You should be able to specify a null model with it . Do these cover your concerns?

Well, sounds perfect!!! Is there any documentation about this? I want to try this :)

pranavkm commented 6 years ago

https://docs.microsoft.com/en-us/aspnet/core/mvc/views/tag-helpers/built-in/partial-tag-helper?view=aspnetcore-2.1

ghost commented 6 years ago

@pranavkm @NTaylorMullen

Well it does not work. The problem resides here, — forgot to put this before —or here.

First, let me coming back to this:

That doesn't seem particularly high value.

Let me explain again. If you take a look at the discussion on the asp.net forum, you will see that I have embedded Plugins in pages (you can see, as well, this on this issue).

If I use <partial model="null" name="_viewName" /> or Html.RenderPartialAsync(viewName, model: null)

I will get: NullReferenceException: Objectreferencenot set to an instance of an object.

If I use <partial model="Model name="_viewName" /> or Html.RenderPartialAsync("_viewName", Model)

I will get: InvalidOperationException: Themodelitem passed into the ViewDataDictionary is of type …

In picture

mismatch

The benefits

So here the benefits ASP.NET CORE is modular. Applications Parts are about modularity. If we want to get a modular plugin system, we need a kind of Html.RenderPartialRawAsync(). And plugins are present in almost every CMS now (very basic feature in any CMS). So it seems logic to have a built in feature in ASP.NET CORE.

Why?

  1. For ASP.NET Core is about modularity!,
  2. The plugins will be portable "up to the hilt",
  3. We can built modular system with Plugins (for CMS, etc…) in pure ASP.NET Core,
  4. No needing on third party API,
  5. No dependence on third party API,
  6. Solid system, as it relies on a method built in ASP.NET CORE,
  7. Simple plugin system, yet power full, as everything is ready (Application Parts + Html.RenderPartialRawAsync())
  8. Plugins are present in almost every CMS, not because it is cool, but because it is useful (SOLID principle, maintainability, etc…).

All of this, with a simple function that return the raw output from the called controller.

ghost commented 6 years ago

I can even say that lazy loading shows why it can be useful, as it

allows you to load different areas of your app client-side from the server as needed.

If the "Blazor team" see any benefits to provide this kind of feature (and developers will enjoy! Me too :D), why ASP.NET Core MVC team and developers will not find this useful? It's illogical.

Because I only try to achieve the same thing in "full" ASP.NET Core MVC.

ghost commented 6 years ago

No more problem like this

pranavkm commented 6 years ago

I will get: NullReferenceException: Objectreferencenot set to an instance of an object.

I don't see this - https://github.com/pranavkm/partial-null/blob/master/Pages/Index.cshtml#L4.

ghost commented 6 years ago

@pranavkm You do not understand. You have to reproduce the same kind of architecture. And then, you will really see what I mean. The problem can be seen as 1 controller that send to another controller its Model.

Anyway, if you look at HtmlHelperPartialExtensions.cs, you will see that a Model/ViewData is sent.

With Plugin built around Application Parts, with its own MVC components, the caller feed the called with its Model. Hence the errors.

pranavkm commented 6 years ago

Is there a repro app you could share with us? Your original requirement was to have a way to pass in a null model to a partial which, like I showed, works fine,

Beyond that, for your plugin requirements, we generally recommend using a module system like the one OrchardCore projects. It's generally more well tested and addresses the kinds of problem you're attempting to solve in your hand rolled implementation.

ghost commented 6 years ago

@pranavkm

Is there a repro app you could share with us?

No, only on local server. But I will try to setup something on github, with pleasure if you ask. Even a PR :p

Your original requirement was to have a way to pass in a null model to a partial which works,

No, in my original requirement, I stated:

What about providing RenderPartialAsync/RenderPartialCoreAsync versions without object model, ViewDataDictionary viewData?

I wrote without object model, ViewDataDictionary viewData meaning empty. Sorry for this misunderstanding.

Beyond that, for your plugin requirements, we generally recommend using a module system like the one OrchardCore projects. It's generally more well tested and addresses the kinds of problem you're attempting to solve in your hand rolled implementation.

Well with a "small" Html.RenderPartialRawAsync() method provided in ASP.NET Core MVC:

  1. Such intricate project (like using OrchardCore projects only for plugins requirement) will become useless;
  2. ASP.NET Core MVC will concern potentially every ASP.NET Core MVC developers, not OrchardCore…
  3. ASP.NET Core MVC is much more trusted/stable than OrchardCore;
  4. More people work with ASP.NET than with OrchardCore… as OrchardCore projects does not feat to everybody needs (OrchardCore is much more a CMS than a framework, and most of us build modular webapps. Anyway, I work on ASP.NET Core MVC, like most of ASP.NET Core developers, not OrchardCore);
  5. I do not think that I am the sole guy in this case, and the, very recent, issue 7825 shows it;
  6. This Html.RenderPartialRawAsync() feature will be simple, yet powerful and unbreakable, which may not be the case with OrchardCore…
  7. No need to worry with another CMS, API, etc…
  8. Application parts are somehow incomplete without such feature;
  9. Sounds crazy that such simple and basic feature is missing in ASP.NET Core MVC, and being forced to use a bloatware, or anything else, just because one little method is missing.

The cost for the ASP.NET Core MVC is null, but the benefit for developers is huge.

You know, you can find the same kind of feature on other projects. From memory, Joomla provides such feature on the CMS and its framework…

Their plugins are mini MVC apps, that you can embed in any page, etc…

ghost commented 6 years ago

@NTaylorMullen @pranavkm

Your original requirement was to have a way to pass in a null model to a partial which works,

If you take a look at the topic on the forum, you will see that my requirement has never changed…. You did not understand it, I can conceive that it's my fault, but I insist, I never change anything in my request.

So to summarize I ask for a kind of Html.RenderPartialRawAsync(), and:

  1. I do not see any reason to not provide such feature in ASP.NET Core MVC (from what I see on ASP.NET Core MVC code, I repeat, this does not cost anything now, nor in the future — nothing hard to maintain, etc…);
  2. This design feat perfectly with Application Parts, and it is very simple, yet powerful;
  3. It will solve a lot of time for developers and ASP.NET Core MVC team, and meets a need of lot of ASP.NET Core MVC developers;
  4. Blazor Lazy loading of application areas seems similar;
  5. It is illogical to use any crAPI :p, or bloatware, for something that is so needed, and so easily implemented in ASP.NET Core MVC.

Finally, I repeat, if you ask, I can setup what you asked for, and even a PR.

pranavkm commented 6 years ago

I've a sample that shows using a PartialTagHelper, you're able to do essentially what this new API requires. If there's a feature gap with the tag helper, which you have yet to have demonstrate, we can consider addressing it. Beyond that, we do not plan on adding new APIs to HtmlHelper.

ghost commented 6 years ago

I've a sample that shows using a PartialTagHelper, you're able to do essentially what this new API requires.

But this is, here, useless, as null object is sent to the Plugin Controller leading to rise an exception… The behavior is the same as in 2.0…

If there's a feature gap with the tag helper, which you have yet to have demonstrate, we can consider addressing it.

Well, I will try to put something that you can test. Will it be enough?

But you wrote:

Beyond that, we do not plan on adding new APIs to HtmlHelper.

Does it mean that under no circumstances you will add a new APIs to HtmlHelper? But here the problem… And if it is the case, I will not waste my time to show you something, while it is hopeless…

mkArtakMSFT commented 6 years ago

Hi @ckams.

It seems there is some kind of confusion here and from reading through your forum post, it seems you really haven't understood that folks here were trying to help you. Not really best way to speak about people without understanding them.

Anyway, the bottom line here is that we still don't understand the exact problem that you want us to fix. So a repro app which would explicitly detail out the scenario as well as the problem you're facing would definitely help. Please go ahead and provide the requested info so we can try to help.

ghost commented 6 years ago

Hi @mkArtakMSFT First, you right mkArtakMSFT, @NTaylorMullen I sincerely apologize for what i wrote.

But, it is, for me, so obvious, and only by looking at the code, that it becomes annoying.

Anyway, I will do my best to detail the problem.

ghost commented 6 years ago

Hi, @mkArtakMSFT @pranavkm @NTaylorMullen, see here a repo.

pranavkm commented 6 years ago

The stack trace says that the exception if from running the view because it assumes a non-null model instance:

image

Making it null conditional - <h1>@Model?.Name</h1> fixes the issue.

ghost commented 6 years ago

@pranavkm No it does not fixes the issue, as:

I always see the same problem… The private async Task RenderPartialViewAsync(TextWriter writer, object model) always sends something that breaks the plugin…

It is obvious, read the code.

And here, this is a simple "plugin"… In any way similar to SMS, SignalR, Analytics Plugins, etc… where their Model will be dropped too…

ghost commented 6 years ago

In my humble opinion, this "trick" looks more like a View Components alternative.

ghost commented 6 years ago

See my request like a TagHelper — coupled with a Html.RenderPartialRawAsync() or more up to date way, with a RenderPartialViewRawAsync() — that behaves like an html object or embed or iframe tag (without injecting all the html/body structure).

From what I read in the ASP.NET Core MVC code, there is, for now, no existing way, in ASP.NET Core MVC, to achieve what I want.

Again, read your own ASP.NET Core MVC code.

ghost commented 6 years ago

@pranavkm to clarify the situation, I often point to Html.RenderPartialAsync(), but the same problem resides with private async Task RenderPartialViewAsync(TextWriter writer, object model). https://github.com/aspnet/Mvc/blob/d47e686cf10ef4c49692627f07ed434cb06af8c6/src/Microsoft.AspNetCore.Mvc.TagHelpers/PartialTagHelper.cs#L139 So to be clear, we need a kind RenderPartialViewRawAsync(). Do not limit your understanding to some methods, but rather think about conceptual limitation in ASP.NET Core MVC.

And whatever you will propose that differs, will not feat the expectation of most of ASP.NET Core MVC developpers. I will not repeat myself, you will find enough arguments here and in the forum.

mkArtakMSFT commented 6 years ago

Hi @ckams.

Just to confirm that my understanding is correct, are you looking for a new mechanism, which will allow you to pass in arbitrary model to a view, which you try to load using RenderPartialViewasync or some alternative API, which will allow you to do that?

ghost commented 6 years ago

Hi @mkArtakMSFT An alternative API, but like this: private async Task RenderPartialViewAsync(TextWriter writer)

mkArtakMSFT commented 6 years ago

@ckams, so you want the mechanism to end up loading views, even thought the model doesn't match. So that you can define in the view which model to use dynamically?

ghost commented 6 years ago

@mkArtakMSFT

@ckams, so you want the mechanism to end up loading views, even thought the model doesn't match. So that you can define in the view which model to use dynamically?

Well, that depends of what you mean by 'the model doesn't match'. It would be a simple mechanism to end up loading views from plugins like this:

As you can see on the repo.

mkArtakMSFT commented 6 years ago

Ok. I now understand your ask. There is, however, still no any plans to provide that type of functionality in the near future. For now, I'll move this issue to the Backlog. We'll review it after a while, and depending on the community involvement with this issue as well as our priorities by the time we'll decide how to move forward.

ghost commented 6 years ago

Finally! :) Thanks for you patience! Could it helps, if I provided you with some code, on how I planned this in a PR? Not for now, I am overloaded :s

mkArtakMSFT commented 5 years ago

Hi. Thanks for contacting us. We're closing this issue as there was not much community interest in this ask for quite a while now.