nest icon indicating copy to clipboard operation
nest copied to clipboard

Allow ClassSerializerInterceptor to serialize plain object

Open zanminkian opened this issue 1 year ago • 4 comments

Is there an existing issue that is already proposing this?

  • [X] I have searched the existing issues

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

There is a waning in the doc of Nest. image

That means we can't return a plain object. All the handlers in controller must return value like this:

@UseInterceptors(ClassSerializerInterceptor)
@Get()
findOne(): UserEntity {
  if(xxx) {
    // first way
    return plainToClass(UserEntity, {
      id: 1,
      firstName: 'Kamil',
      lastName: 'Mysliwiec',
      password: 'password',
    })
  }
  // second way
  return new UserEntity({
    id: 1,
    firstName: 'Kamil',
    lastName: 'Mysliwiec',
    password: 'password',
  });
}

The first way and the second way are hard to use.

Describe the solution you'd like

What I'm expecting looks like below:

@ReturnType(UserEntity)
@UseInterceptors(ClassSerializerInterceptor)
@Get()
findOne(): UserEntity {
  return {
    id: 1,
    firstName: 'Kamil',
    lastName: 'Mysliwiec',
    password: 'password',
  };
}

Teachability, documentation, adoption, migration strategy

As for teachability, telling developer to add a decorator @ReturnType(UserEntity) is better than warning them not to return a plain object.

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

Reasons:

  • Let's say I have 1000+ handlers. I don't need to look into the methods. Just adding two decorators @ReturnType(Xxx) and @UseInterceptors(ClassSerializerInterceptor) before each handler.
  • Warning in document is not friendly. People may return wrong object but don't receive any compile error. Sometime it's hard to find out the bug.

zanminkian avatar Aug 09 '22 12:08 zanminkian

I'm no sure if we should add yet another decorator. There are too much decorators already :thinking:

What about:

@UseInterceptors(new ClassSerializerInterceptor({ returnType: UserEntity }))
@Get()
findOne(): UserEntity {
  return {
    id: 1,
    firstName: 'Kamil',
    lastName: 'Mysliwiec',
    password: 'password',
  };
}

isn't that great but yeah

micalevisk avatar Aug 09 '22 12:08 micalevisk

I'm no sure if we should add yet another decorator. There are too much decorators already 🤔

What about:

@UseInterceptors(new ClassSerializerInterceptor({ returnType: UserEntity }))
@Get()
findOne(): UserEntity {
  return {
    id: 1,
    firstName: 'Kamil',
    lastName: 'Mysliwiec',
    password: 'password',
  };
}

isn't that great but yeah

@micalevisk Also a good way. I think @ReturnType is a common decorator, which can work with swagger. For example:

@ReturnType(UserEntity)
@Get()
find(): Array<UserEntity> {
  return [{
    id: 1,
    firstName: 'Kamil',
    lastName: 'Mysliwiec',
    password: 'password',
  }];
}

Without @ReturnType, swagger plugin don't know the type of item in the array.

zanminkian avatar Aug 09 '22 13:08 zanminkian

So, if we were to implement this, we'd essentially just end up calling plainToClass in the interceptor before re-calling classToPlain. If you really wanted, this could be implemented in another interceptor that's specific to your use case, as interceptors, on the response side, are ran first-in last-out, so as long as the ClassSerializationInterceptor is registered before this ToClassInterceptor then there should be no problems with having something like

@Injectable()
export class ToClassInterceptor implements NestInterceptor {
  constructor(private readonyl reflector: Reflector) {}

  intercept(context: ExecutionContext, next: CallHandler) {
    const toClassRef = this.reflector.get('TO_CLASS_METADATA_KEY', context.getHandler());
    return next.handle().pipe(map(data => plainToClass(toClassRef, data));
  }
}

And there you have it. Just bind it globally after the ClassSerializationInterceptor and you should be good (make sure to handle for cases where you don't care about serialization or have an actual returned object)

jmcdo29 avatar Aug 09 '22 14:08 jmcdo29

@jmcdo29 Add a custom interceptor before ClassSerializerInterceptor is easy. But if Nest allow to serialize plain object out of box will be better imo. Just do serializing in ONE interceptor instead of two interceptors.

Please consider this PR #10087, thanks.

zanminkian avatar Aug 09 '22 14:08 zanminkian

Also a good way. I think @ReturnType is a common decorator, which can work with swagger

The example you presented already works with Swagger without any additional decorators if you have the NestJS CLI plugin enabled.

Let's track this conversation here https://github.com/nestjs/nest/pull/10087

kamilmysliwiec avatar Aug 11 '22 07:08 kamilmysliwiec