Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Does it work with @Controller in nestjs? #2

Closed
CatsMiaow opened this issue Jan 3, 2022 · 2 comments
Closed

Does it work with @Controller in nestjs? #2

CatsMiaow opened this issue Jan 3, 2022 · 2 comments
Labels
question Further information is requested

Comments

@CatsMiaow
Copy link

https://github.com/CatsMiaow/typescript-starter/blob/master/src/app.controller.ts#L7
In the sample code above, test2 outputs normally, but if you add (uncomment) @DecorateAll, the controller's route outputs a 404 page.

https://github.com/Papooch/decorate-all/blob/main/src/lib/decorate-all.decorator.ts#L35
If remove Object.defineProperty it doesn't throw a 404, but the decorator doesn't work.

@Papooch
Copy link
Owner

Papooch commented Jan 3, 2022

Since you're replacing the entire function definition as opposed to only adding metadata (which was the original intended purpose), any other previously defined metadata (those defined by the @Get) are lost, because class decorators are applied after methdo decorators.

You can work around it by explicitly copying the metadata in your custom decorator using this helper function:

/**
 * Copies all metadata from one object to another.
 * Useful for overwriting function definition in
 * decorators while keeping all previously
 * attached metadata
 *
 * @param from object to copy metadata from
 * @param to object to copy metadata to
 */
export function copyMetadata(from: any, to: any) {
    const metadataKeys = Reflect.getMetadataKeys(from);
    metadataKeys.map((key) => {
        const value = Reflect.getMetadata(key, from);
        Reflect.defineMetadata(key, value, to);
    });
}

and in your debug-log.decorator.ts:

const newMethod = async function (...args: unknown[]): Promise<unknown> {
      const result: unknown = await originalMethod.apply(this, args);

      console.log(context, Date.now());
      return result;
};
copyMetadata(originalMethod, newMethod);
descriptor.value = newMethod;

This is clearly an oversight on my end and I'll probably incorporate it into the package itself.

EDIT: you also need to install the reflect-metadata package

@Papooch Papooch added bug Something isn't working question Further information is requested and removed bug Something isn't working labels Jan 3, 2022
@Papooch
Copy link
Owner

Papooch commented Jan 3, 2022

It is now fixed in v1.1.0 and published to npm.

@Papooch Papooch closed this as completed Jan 3, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Projects
None yet
Development

No branches or pull requests

2 participants