nest icon indicating copy to clipboard operation
nest copied to clipboard

[MVC] Render multiple views for single handler in controller without using @Res

Open SwinX opened this issue 3 years ago • 14 comments

Feature Request

Is your feature request related to a problem? Please describe.

As per documentation if user needs to render view dynamically he have to do it manually by using platform-specific response object provided by @Res decorator. This is not quite handy because in this case all the wrapping (exception filters, interceptors, etc) infrastructure around nest is disabled and this functionality missed

Describe the solution you'd like

Allow user to specify what view to render in the data returned from the controller. The behaviour similar to redirect, when user is able return redirect url in special url field. Something like this:

import { Get, Controller, Render } from '@nestjs/common';

@Controller()
export class AppController {
  @Get()
  @Render('index')
  async root() {

  try {
    const indexData = await this._getIndexData();

    return indexData;
  catch (e) {
    return { view: 'error-view' };
  }
}

Since returning arbitrary views from handler might conflict with declaring view via @Render decorator, it may make sense to allow to declare multiple views in decorator and throw some kind of exception if user decided to return something which is not declared. For example:

import { Get, Controller, Render } from '@nestjs/common';

@Controller()
export class AppController {
  @Get()
  @Render(['index', 'unauthorized-error'])
  async root() {

  try {
    const user = await this._userService.getUser();
    
    if (!user) {
      return { view: 'unauthorized-error' }; // will render unauthorized user page
    }
    
    return { user }; // will render index page
  catch (e) {
    return { view: 'error-view' }; // Nest will throw error on attempt to render unspecified view
  }
}

In this case user will be required to always return a view. However the old behaviour will not change: if view set as @Render('view'), Nest will continue to pick info from metadata associated via method.

What is the motivation / use case for changing the behavior?

Possible use-cases are:

  • rendering specific error pages for business-logic errors
  • A/B Experiments
  • business process requires different pages to be rendered on a same URL and we do not want to go SPA way

SwinX avatar Jan 15 '21 14:01 SwinX

This sounds like it could be useful. Is there any reason why you couldn't use res.render() in a filter after throwing an error?

In the meantime, if you don't mind doing some of the lifting yourself, you can follow the example of this repo for how you can dynamically generate the html for the page.

I created that example for using a rendered view for browsers and JSON output for things like curl and Postman, but you could probably implement something very similar for dynamically changing which view is rendered

jmcdo29 avatar Jan 15 '21 15:01 jmcdo29

@kamilmysliwiec could we do something similar to the @Redirect() decorator, where we read data.url of the response if it is to be a dynamic response, and if not just use what's in the decorator? Maybe have something like

export interface RenderResult<T> {
  view: string;
  data: T
}

So that later we can do res.render(view, data) to dynamically render. This is, of course, if the @Render() decorator exists on the route in the first place

jmcdo29 avatar Jan 19 '21 23:01 jmcdo29

@jmcdo29 thanks for proposing alternative way with exception filters. This may help in a case with rendering error pages. However here we're mixing contexts of business logic errors with technical errors. The modern approach for exceptions is to use it only in case when something broke, not when some kind of business logic conditions were not fulfilled. For other use-cases like A/B experiments or rendering different pages for single route using exceptions and filters seems to be wrong at all cause exceptions are not designed for controlling program flow.

@kamilmysliwiec do you have anything against implementing this in general? If not, may you please comment about proposed API? Personally I like option 2 (with listing all possible views in @Render) more.

SwinX avatar Jan 21 '21 09:01 SwinX

Asking here because it is somewhat related, I'm using an explicit parameter @Res() res and then res.render to display view files. Also, I have data transfer objects set up to ensure that valid data is flowing as expected.

But whenever the DTO throws an error, it just sends it's default 400 : Bad Request json response. How can I show a view for that?

YashKumarVerma avatar Jan 21 '21 12:01 YashKumarVerma

Is there any reason you can't inject & use the Response object? cc @SwinX

But whenever the DTO throws an error, it just sends it's default 400 : Bad Request json response. How can I show a view for that?

Either register a global exception filter that will process 400 Bad Request exceptions or create a custom class that inherits from the ValidationPipe and adapt the original behavior to what your application needs.

kamilmysliwiec avatar Jan 25 '21 08:01 kamilmysliwiec

Closed by accident

kamilmysliwiec avatar Jan 25 '21 08:01 kamilmysliwiec

@kamilmysliwiec technically using Response explicitly can be done but this but this makes user to handle the response explicitly. This may be not so trivial. For example in our case we're building internal SDK, renderer is a part of it. It requires data to be prepared after returning from controller. Projects, which integrate with our SDK using custom Render decorator, which is using nest Render among with some interceptors, which are responsible for preparing data. So if user decides to use Response directly he will have to learn our internal data format, which should not be exposed, how decorators applied and so on. All this leads not only to the problems of using hidden internals, but also will break if we will introduce some changes to renderer. Even if we were using something simple for rendering it seems a bit cumbersome to always use Response explicitly cases I described in initial post.

SwinX avatar Jan 26 '21 13:01 SwinX

this makes user to handle the response explicitly. This may be not so trivial.

Still, Isn't it more trivial than returning an object that must match a specific shape and this shape would determine whether the framework should propagate the data to the view (as properties) or render a different site?

if user decides to use Response directly he will have to learn our internal data format, which should not be exposed, how decorators applied and so on

As long as you extract a logic responsible for parsing the data to your internal data format to a separate service/parser/any class which you expose from your SDK, this shouldn't be an issue.

kamilmysliwiec avatar Jan 27 '21 08:01 kamilmysliwiec

Still, Isn't it more trivial than returning an object that must match a specific shape and this shape would determine whether the framework should propagate the data to the view (as properties) or render a different site?

Instead of a specific field view we may introduce some kind of ViewModel class, which instance may be returned from the controller handler. So we will not have to rely on object shape. This should remove backwards compatibility issues

As long as you extract a logic responsible for parsing the data to your internal data format to a separate service/parser/any class which you expose from your SDK, this shouldn't be an issue.

Sure, but in our case renderer is a private thing, which designed to integrate with nest but not to be used manually. Our goal was to use standard nest approach as much as possible, reducing custom parts as we can. Exposing renderer API will conflict with this approach.

SwinX avatar Jan 27 '21 09:01 SwinX

Instead of a specific field view we may introduce some kind of ViewModel class, which instance may be returned from the controller handler. So we will not have to rely on object shape. This should remove backwards compatibility issues

I like the idea of having a dedicated class for this. Would you like to create a PR with this feature?

kamilmysliwiec avatar Jan 27 '21 09:01 kamilmysliwiec

I like the idea of having a dedicated class for this. Would you like to create a PR with this feature?

Sure, we may do this.

What about other parts of the solution? Is option with listing all possible views in @Render looks good for you?

SwinX avatar Jan 27 '21 10:01 SwinX

What about other parts of the solution? Is option with listing all possible views in @Render looks good for you?

I'm not sure if this won't overcomplicate things a little bit. Imagine having a few common error pages that might be rendered by multiple routes. In this case, you'll have to specify them for all routes in all decorator occurrences (= a lot of additional boilerplate code). I'd say we should think about @Render() as a "default view to render" in case X object (instance of a new class we want to expose) wasn't returned from that route and nothing else.

kamilmysliwiec avatar Jan 27 '21 10:01 kamilmysliwiec

Ok, sounds reasonable. So the logic will look something like this:

  • if object returned from handler, pick view from metadata set by @Render, use returned object as data for this view
  • if ViewModel returned from handler, pick view and data from it. If some view was set by @Render, discard it.

Seems we have everything to start implementation. We will come up with PR later, thank you!

SwinX avatar Jan 27 '21 11:01 SwinX

if using express you can return res.render ex: return res.render( { layout: 'login'} ).

I seeking way for fastify :(((

chinhtrandn avatar Dec 03 '21 18:12 chinhtrandn

Closing this issue as you can achieve what we discussed here using the Response object directly.

kamilmysliwiec avatar Feb 01 '23 13:02 kamilmysliwiec