riganti / dotvvm

Open source MVVM framework for Web Apps
https://www.dotvvm.com
Apache License 2.0
749 stars 97 forks source link

RouteTable.AddRedirection helper method #462

Closed exyi closed 4 years ago

exyi commented 7 years ago

Often, the URLs on a website change, but you need to maintain compatibility with the old system. It could be quite helpful to have a helper method, that would simply register a route with a redirecting presenter. The usage could look like that:

routes.AddRedirection("article/{Id}/edit", "admin/article-edit/{Id}");

or redirection to route name:

routes.AddRouteRedirection("article/{Id}/edit", "Admin_EditArticle");

The redirection should be permanent by default, but there should be a optional permanent: bool = true parameter. And the name of the route is basically irrelevant (you probably don't want RouteLinks to a redirecing route), so they should also be optional.

djanosik commented 7 years ago

I am not sure this is really useful. Why not to use IIS UrlRewrite for Owin and rewriting middleware for ASP.NET Core. I think our own solution will be of little value.

tomasherceg commented 7 years ago

The classic URL rewriting solutions might not work properly in SPAs.

djanosik commented 7 years ago

Why? Redirecting happens far before any application middleware takes control. Our SPAs should handle that correctly.

exyi commented 7 years ago

@tomasherceg It should work with SPAs, as first request will be simply redirected to a new one, and the following requests will be redirected transparently to the Javascript.

I just think that this would allow you to keep the routing in one place, with the same routing syntax. I don't say it's a great win, but it feels more comfortable.

tomasherceg commented 7 years ago

The first request in SPA is not the problem, the subsequent requests are. SPA GET request expects JSON as a response. If you do a classic redirect, the XmlHttpRequest will receive HTML and the subsequent request processing will fail.

exyi commented 7 years ago

@tomasherceg You are right that there might be a problem with redirects outside of the SPA. But requests inside SPAs should respond with the JSON, so there should be no problem.

Actually, this seems to me like a non-trivial flaw of the SPAs, as it is basically impossible to link a non-dotvvm page on the same domain from a SPA site, if I understand it correctly.

djanosik commented 7 years ago

I think we should care only about redirects made in ViewModels for now. Even if we add our own redirecting solution there is no guarantee people will use it as there are more powerful alternatives.

We can handle this later by replacing XmlHttpRequest with Fetch API. It allows to handle redirects manually.

tomasherceg commented 4 years ago

In js-modules, we changed XmlHttpRequest for Fetch API, so now we may move on with this issue. We've also got rid of the hashbang URLs thanks to the History API.

I'll move the discussion about SPA and external URL rewriters/redirections in a separate issue.

The AddRedirect and AddRouteRedirect looks good to me - we've even implemented RedirectPresenter on some customer projects, so the feature makes sense in the framework.

We should also support redirection to a custom URL (even external URLs - these are useful when an app is split into multiple applications - basically the use cases correlate with RedirectToRoute, RedirectToUrl and RedirectToLocalUrl).