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

Type for mixinFn doesn't match docs - mixin doesn't actually get logger parameter #1787

Closed
jason-curtis opened this issue Aug 17, 2023 · 5 comments · Fixed by #1977
Closed

Comments

@jason-curtis
Copy link

The docs for the mixin option state that it has 3 arguments: https://getpino.io/#/docs/api?id=mixin-function
The first parameter is the value mergeObject or an empty object. The second parameter is the log level number. The third parameter is the logger or child logger itself

However, the type declaration only has two arguments:

pino/pino.d.ts

Line 31 in 91a3505

type MixinFn = (mergeObject: object, level: number) => object;

Which of these versions of reality is correct?

Here's a screenshot of the resultant type error in VS code:
image

Pino version: 8.15.0

@mcollina
Copy link
Member

mcollina commented Oct 5, 2023

I think the test are the best form of documentation:

pino/test/mixin.test.js

Lines 164 to 178 in 309cdfb

test('mixin receives logger as third parameter', async ({ ok, same }) => {
const stream = sink()
const instance = pino({
mixin (context, num, logger) {
ok(logger !== null, 'logger should be defined')
ok(logger !== undefined, 'logger should be defined')
same(logger, instance)
return { ...context, num }
}
}, stream)
instance.level = name
instance[name]({
message: '123'
}, 'test')
})

Could you check the behavior and make sure docs and types are correct?

@jason-curtis
Copy link
Author

Just as a heads up, my team has worked around this issue for now and I don't expect to have time to check and update the docs.

@andreiwow2
Copy link

I encountered the same problem.

Type '(obj: any, num: any, logger: any) => { tags: any; }' is not assignable to type 'MixinFn'.
  Target signature provides too few arguments. Expected 3 or more, but got 2.

@Downster
Copy link

When can we expect this fix to be released? What would be the best workaround for now?

Copy link

This issue has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jun 20, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants