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

export pino-params provider #441

Merged
merged 3 commits into from Mar 30, 2021

Conversation

dguyonvarch
Copy link

@dguyonvarch dguyonvarch commented Mar 24, 2021

With this PR we can now do :

// logger.service.ts
import { Inject, Injectable } from '@nestjs/common';
import { Logger, Params, PARAMS_PROVIDER_TOKEN, PinoLogger } from 'nestjs-pino';

@Injectable()
export class LoggerService extends Logger {
    private aProperty: string;
    
    constructor(
        pinoLogger: PinoLogger,
        @Inject(PARAMS_PROVIDER_TOKEN) params: Params,
        aProperty: string
    ) {
        super(pinoLogger, params);
        this.aProperty = aProperty
    }

    aMethod(): void {
        ...
    }
}

and

// logger.module.ts
import { Module } from "@nestjs/common";
import { LoggerModule  } from "nestjs-pino";
import { LoggerService } from "./logger.service";

@Module({
  exports: [LoggerService],
  providers: [LoggerService],
  imports: [
   LoggerModule.forRoot()
  ],
})
export class LoggerModule {}

@AckerApple
Copy link

I am in appreciation of this PR. I have a more tedious solution injected into a company code base and would like to see this PR merged

@jdt6
Copy link

jdt6 commented Mar 24, 2021

Yea how soon until this gets merged!???

@iamolegga
Copy link
Owner

Hi, looks like @jdt6 really need this feature 😀 And there colud be more users who need this too, so let's add extra section in docs with example and some basic test to be sure that this will not be broken in future. After that I can merge it and release. Thanks

@dguyonvarch dguyonvarch force-pushed the export-pino-params branch 3 times, most recently from eb87ea2 to bf27ca3 Compare March 29, 2021 12:55
@iamolegga iamolegga merged commit b68bd51 into iamolegga:master Mar 30, 2021
@iamolegga
Copy link
Owner

released in 1.4.0

@delai
Copy link

delai commented Nov 30, 2021

With this PR we can now do :

// logger.service.ts
import { Inject, Injectable } from '@nestjs/common';
import { Logger, Params, PARAMS_PROVIDER_TOKEN, PinoLogger } from 'nestjs-pino';

@Injectable()
export class LoggerService extends Logger {
    private aProperty: string;
    
    constructor(
        pinoLogger: PinoLogger,
        @Inject(PARAMS_PROVIDER_TOKEN) params: Params,
        aProperty: string
    ) {
        super(pinoLogger, params);
        this.aProperty = aProperty
    }

    aMethod(): void {
        ...
    }
}

and

// logger.module.ts
import { Module } from "@nestjs/common";
import { LoggerModule  } from "nestjs-pino";
import { LoggerService } from "./logger.service";

@Module({
  exports: [LoggerService],
  providers: [LoggerService],
  imports: [
   LoggerModule.forRoot()
  ],
})
export class LoggerModule {}

TS check error in logger.module.ts

Individual declarations in merged declaration 'LoggerModule' must be all exported or all local.ts(2395)
Import declaration conflicts with local declaration of 'LoggerModule'.ts(2440)

any suggestion? thanks a lot

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants