Map ctx.request.params to corresponding method parameters

[OrkhanAlikhanov]

It would be great if we could map ctx.request.params to same named method arguments:

  @Get('/:id/:name/:value')
  index(name, value, ctx, id) {
    return { /// Considering #505 is implemented
      name,
      value,
      id,
      params: ctx.request.params
    };
  }

It would be better if we could map them irrespective of order, based on names.

[LoicPoullain]

Thank you for suggesting this issue @OrkhanAlikhanov .

I had many issues in previous coding experiences when managing method parameters based on names. They are supposed to be dummy variables and are often treated as is during compilation or code minifying, causing the app to fail.

From what I understand, the point here would be not to write every the time the whole ctx.request.params.id to keep the code more concise.

routing-controllers have their own way to solve this: the @Param decorator.

index(@Param('id') id: number) {
  // ...
}

This would solve the naming issue but I'd rather not to go with a heavy usage of decorators (which is often a criticize of Nest).

I'm moving this to Maybe / Pending / To think for now.

[OrkhanAlikhanov]

Well, we can make it completely minification proof if we pass params in order. We can infer param names from path (/:id/:name/:value) and pass them to the method in order like:

@Get('/:id/:name/:value')
index(ctx, myId, myName, myValue) {
   return ...
}

What do you think?

[LoicPoullain]

Interesting. This would solve the naming issue indeed. But in this case, Foal will have to manually parse the path /:id/:name/:value to know the parameters which is already done under the hood by Express.

This may also generate conflicts in large apps with many sub-controllers (see example below).

@Get('/users/:userId')
class UserController {
  @Post('/friends/:friendId')
  deleteUserRelationShip(ctx, friendId) {
    // Is it `friendId` or `userId`?
  }
}

Maybe we could also pass the params object as second (optional) argument and take advantage of es6 syntax like this?

@Get('/:id/:name/:value')
index(ctx, { id, name, value }) { // equivalent to index(ctx, params)
  // do something with the variables "id", "name" and "value".
   return ...
}

@Get('/users/:userId')
class UserController {
  @Post('/friends/:friendId')
  deleteUserRelationShip(ctx, { friendId }) { // or { friendId, userId }
    ...
  }
}

[OrkhanAlikhanov]

Maybe we could also pass the params object as second (optional) argument and take advantage of es6 syntax like this?

Amazing idea! That eliminates lots of maintenance and reliability headache

[LoicPoullain]

The query could also be given like this:

class ProductController {
  @Get('/products')
  @ValidateQueryParam(...)
  @ValidateQueryParam(...)
  findProducts(ctx, params, { from, to }) {
    ...
  }
}

[OrkhanAlikhanov]

It would be great to have a config file to configure order of those params. Kindof a js config file where we can define a method to return array of method arguments to be passed to controller functions, defaulting to [ctx, ctx.request.params, ctx.request.query, ctx.request.body]

[OrkhanAlikhanov]

I think having a transformCtxToParams method per controller would give more control for us.
More eloquent name would be desirable.

class ProductsController {
  transformCtxToParams(ctx) {
   return [ctx, ctx.request.params, ctx.request.query]
  }

  @Get('/products')
  @ValidateQueryParam(...)
  @ValidateQueryParam(...)
  async findProducts(ctx, params, { from, to }) {
    ...
  }
}

I think inheritance should not affect in this case and the array returned from transformCtxToParams or default [ctx] should be used in calling endpoint methods:

class AppController {
   subControllers = [
    ProductsController /// uses its own transformCtxToParams
  ],

  transformCtxToParams(ctx) {
   return [null]; /// does not affect subControllers
  }
}

---

Alternatively a metadata on ctx object could be written using hooks where foalts would use that metadata and call endpoint methods accordingly:

const MyParamMapper = Hook(ctx => {
  /// foal uses value of `methodParams` in calling endpoint methods.
  ctx.state.foal.methodParams = [ctx, ctx.request.params, ctx.request.query];
});

@MyParamMapper
class ProductsController {
  @Get('/products')
  @ValidateQueryParam(...)
  @ValidateQueryParam(...)
  async findProducts(ctx, params, { from, to }) {
    ...
  }
}

This way applying the hook on a parent controller would affect entire children.

Just sharing ideas came to my mind.

[LoicPoullain]

#490 (comment)

[geekflyer]

following up on my comment on #490:

The issue is basically with the current design of FoalTS encourages type-unsafe access of context properties.

Let's describe this using an example taken from the docs:

import { Contains, Length } from 'class-validator';

export class SocialPost {

  @Length(10, 20)
  title: string;

  @Contains('hello')
  text: string;

}

import { Context, HttpResponseCreated, Post } from '@foal/core';
import { ValidateBody } from '@foal/typestack';
import { SocialPost } from './social-post.validator';

export class SocialPostController {

  @Post()
  @ValidateBody(SocialPost)
  createSocialPost(ctx: Context) {
    const body = ctx.request.body;
    // do something with the body
    return new HttpResponseCreated();
  }
}

In the example above ctx.request.body and body is basically implicitly any.

This means I can accidentally type stuff like:

body.message (which would yield undefined) instead of body.text.

Normally I would expect the Context interface to have a generic type parameter to pass in the body type, but even this isn't the case, so I can't do things like:

export class SocialPostController {

  @Post()
  @ValidateBody(SocialPost)
  createSocialPost(ctx: Context<SocialPost>) {
    const body = ctx.request.body;
    // do something with the body
    return new HttpResponseCreated();
  }
}

In other frameworks this is usually solved more elegantly by having parameter decorator, i.e.:

export class SocialPostController {
  @Post()
  createSocialPost(@ValidateBody(SocialPost) body: SocialPost) {
    // do something with the body
    return new HttpResponseCreated();
  }
}

I'm aware I could do

const body = ctx.request.body as SocialPost but I find that rather unwieldy, especially since the validation and type cast now appear on very different lines in the code, making it more likely that one would accidentally not keep them in sync (i.e. validate as B, but cast to A).

Imo the nicest way I've seen this solved is in plumier:
https://plumierjs.com/docs/refs/validation#validate-request-body

[LoicPoullain]

Feature implemented in v1.9.