gremo/nest-winston

[Question]: Should documentation be updated?

bezenson opened this issue · 8 comments

Description

I've picked "Replacing the Nest logger (also for bootstrapping)" to setup logger.

In the end of section there are 2 requirements:

  1. Add Logger to AppModule providers.
  2. Inject in class constructor:
import { Controller, Logger } from '@nestjs/common';

@Controller('cats')
export class CatsController {
  constructor(private readonly logger: Logger) {}
}

But as a result I am getting next error:

Error: Nest can't resolve dependencies of the CatsController

After some investigation I found another way to use logger in class in official documentation here.

import { Logger, Injectable } from '@nestjs/common';

@Injectable()
class MyService {
  private readonly logger = new Logger(MyService.name);

  doSomething() {
    this.logger.log('Doing something...');
  }
}

And now everything is works. No need to add into providers list. So yeah.. Is it ok? Should documentation be updated?

Description

I've picked "Replacing the Nest logger (also for bootstrapping)" to setup logger.

In the end of section there are 2 requirements:

  1. Add Logger to AppModule providers.
  2. Inject in class constructor:
import { Controller, Logger } from '@nestjs/common';

@Controller('cats')
export class CatsController {
  constructor(private readonly logger: Logger) {}
}

But as a result I am getting next error:

Error: Nest can't resolve dependencies of the CatsController

After some investigation I found another way to use logger in class in official documentation here.

import { Logger, Injectable } from '@nestjs/common';

@Injectable()
class MyService {
  private readonly logger = new Logger(MyService.name);

  doSomething() {
    this.logger.log('Doing something...');
  }
}

And now everything is works. No need to add into providers list. So yeah.. Is it ok? Should documentation be updated?

Wouldn't this just be using the built in nest logger and not the custom logger?

@TroyMorvant No, winston is used. All steps are taken from readme.

I think secret is here:

  const app = await NestFactory.create(AppModule, {
    logger: WinstonModule.createLogger({
      // options (same as WinstonModule.forRoot() options)
    })

And then import from @nestjs/common should be used

Same here.

private readonly logger = new Logger(MyService.name);

Seems to work instead of trying to put it in the constructor.

Solve it in another way

app.module.ts

@global() // important
@module({
providers: [Logger],
exports: [Logger], // important
})
export class AppModule {}

This issue is stale because it has been open 30 days with no activity. Remove stale label or comment or this will be closed in 7 days.

This issue was closed because it has been stalled for 7 days with no activity.

drewB commented

I definitely think the documentation should be updated. Since most people will be migrating from the default nest way of using the logger which is exactly the same the @inferusvv solution it is much less effort.

tapqr commented

waste a week to solve this problem