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.

Sign up for GitHub

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

Jump to bottom

fix: type regression on fastify #1246

Merged
merged 3 commits into from Dec 1, 2021
Merged

fix: type regression on fastify #1246

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
e393b91
fix: type regression on fastify
climba03003 Nov 30, 2021
5834906
refactor: remove unused import
climba03003 Nov 30, 2021
e74a8ee
test: fix failing test
climba03003 Dec 1, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
183 changes: 95 additions & 88 deletions pino.d.ts
View file
Open in desktop
Expand Up @@ -18,7 +18,7 @@

import type { EventEmitter } from "events";
// @ts-ignore -- gracefully falls back to `any` if not installed
import type { PrettyOptions } from "pino-pretty";
import type { PrettyOptions as PinoPrettyOptions } from "pino-pretty";
import type { SonicBoom, SonicBoomOpts } from "sonic-boom";
import type { WorkerOptions } from "worker_threads";

Expand All @@ -27,9 +27,6 @@ import * as pinoStdSerializers from "pino-std-serializers";

//// Non-exported types and interfaces

type SerializerFn = (value: any) => any;
type WriteFn = (o: object) => void;

// ToDo https://github.com/pinojs/thread-stream/issues/24
type ThreadStream = any

Expand All @@ -42,90 +39,6 @@ interface redactOptions {
remove?: boolean;
}

interface BaseLogger {
/**
* Set this property to the desired logging level. In order of priority, available levels are:
*
* - 'fatal'
* - 'error'
* - 'warn'
* - 'info'
* - 'debug'
* - 'trace'
*
* The logging level is a __minimum__ level. For instance if `logger.level` is `'info'` then all `'fatal'`, `'error'`, `'warn'`,
* and `'info'` logs will be enabled.
*
* You can pass `'silent'` to disable logging.
*/
level: pino.LevelWithSilent | string;

/**
* Log at `'fatal'` level the given msg. If the first argument is an object, all its properties will be included in the JSON line.
* If more args follows `msg`, these will be used to format `msg` using `util.format`.
*
* @typeParam T: the interface of the object being serialized. Default is object.
* @param obj: object to be serialized
* @param msg: the log message to write
* @param ...args: format string values when `msg` is a format string
*/
fatal: pino.LogFn;
/**
* Log at `'error'` level the given msg. If the first argument is an object, all its properties will be included in the JSON line.
* If more args follows `msg`, these will be used to format `msg` using `util.format`.
*
* @typeParam T: the interface of the object being serialized. Default is object.
* @param obj: object to be serialized
* @param msg: the log message to write
* @param ...args: format string values when `msg` is a format string
*/
error: pino.LogFn;
/**
* Log at `'warn'` level the given msg. If the first argument is an object, all its properties will be included in the JSON line.
* If more args follows `msg`, these will be used to format `msg` using `util.format`.
*
* @typeParam T: the interface of the object being serialized. Default is object.
* @param obj: object to be serialized
* @param msg: the log message to write
* @param ...args: format string values when `msg` is a format string
*/
warn: pino.LogFn;
/**
* Log at `'info'` level the given msg. If the first argument is an object, all its properties will be included in the JSON line.
* If more args follows `msg`, these will be used to format `msg` using `util.format`.
*
* @typeParam T: the interface of the object being serialized. Default is object.
* @param obj: object to be serialized
* @param msg: the log message to write
* @param ...args: format string values when `msg` is a format string
*/
info: pino.LogFn;
/**
* Log at `'debug'` level the given msg. If the first argument is an object, all its properties will be included in the JSON line.
* If more args follows `msg`, these will be used to format `msg` using `util.format`.
*
* @typeParam T: the interface of the object being serialized. Default is object.
* @param obj: object to be serialized
* @param msg: the log message to write
* @param ...args: format string values when `msg` is a format string
*/
debug: pino.LogFn;
/**
* Log at `'trace'` level the given msg. If the first argument is an object, all its properties will be included in the JSON line.
* If more args follows `msg`, these will be used to format `msg` using `util.format`.
*
* @typeParam T: the interface of the object being serialized. Default is object.
* @param obj: object to be serialized
* @param msg: the log message to write
* @param ...args: format string values when `msg` is a format string
*/
trace: pino.LogFn;
/**
* Noop function.
*/
silent: pino.LogFn;
}

interface LoggerExtras extends EventEmitter {
/**
* Exposes the Pino package version. Also available on the exported pino function.
Expand Down Expand Up @@ -192,10 +105,98 @@ interface LoggerExtras extends EventEmitter {

declare namespace pino {
//// Exported types and interfaces

interface BaseLogger {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why move it from top level into a namespace?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because it never export outside and nobody can use.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can't we export it instead?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then the old way of import is missing this exportation.
e.g.

import pino from 'pino'
pino.BaseLogger

Top-level export is mapped on line 768

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We generally want to move away from relying on namespace. See #1193 (comment)
I agree that types should be fixed if we lost the way to do something we could do before, though.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree that it should move away from namespace, but the current version is too late to do it.
Current version should support the import syntax that can be used in 7.0.0.

The complete removal should be happen in next major 8.0.0.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I understand that we can't remove it yet, but can we at least already provide alternative for people who want to do direct named imports? That was the gist of #1193 and it seems like the right direction.
Other than that, PR LGTM.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As I mention on line 768, it has the top-level export mapped.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR already provide the type import for below format.

// named import
import { BaseLogger } from 'pino'
// namespace import / default import
import pino from 'pino'
pino.BaseLogger
// pino named import
import { pino } from 'pino'
pino.BaseLogger
// old P named import
import { P } from 'pino'
P.BaseLogger

If anything is missing, please let me know.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for clarification!

/**
* Set this property to the desired logging level. In order of priority, available levels are:
*
* - 'fatal'
* - 'error'
* - 'warn'
* - 'info'
* - 'debug'
* - 'trace'
*
* The logging level is a __minimum__ level. For instance if `logger.level` is `'info'` then all `'fatal'`, `'error'`, `'warn'`,
* and `'info'` logs will be enabled.
*
* You can pass `'silent'` to disable logging.
*/
level: pino.LevelWithSilent | string;

/**
* Log at `'fatal'` level the given msg. If the first argument is an object, all its properties will be included in the JSON line.
* If more args follows `msg`, these will be used to format `msg` using `util.format`.
*
* @typeParam T: the interface of the object being serialized. Default is object.
* @param obj: object to be serialized
* @param msg: the log message to write
* @param ...args: format string values when `msg` is a format string
*/
fatal: pino.LogFn;
/**
* Log at `'error'` level the given msg. If the first argument is an object, all its properties will be included in the JSON line.
* If more args follows `msg`, these will be used to format `msg` using `util.format`.
*
* @typeParam T: the interface of the object being serialized. Default is object.
* @param obj: object to be serialized
* @param msg: the log message to write
* @param ...args: format string values when `msg` is a format string
*/
error: pino.LogFn;
/**
* Log at `'warn'` level the given msg. If the first argument is an object, all its properties will be included in the JSON line.
* If more args follows `msg`, these will be used to format `msg` using `util.format`.
*
* @typeParam T: the interface of the object being serialized. Default is object.
* @param obj: object to be serialized
* @param msg: the log message to write
* @param ...args: format string values when `msg` is a format string
*/
warn: pino.LogFn;
/**
* Log at `'info'` level the given msg. If the first argument is an object, all its properties will be included in the JSON line.
* If more args follows `msg`, these will be used to format `msg` using `util.format`.
*
* @typeParam T: the interface of the object being serialized. Default is object.
* @param obj: object to be serialized
* @param msg: the log message to write
* @param ...args: format string values when `msg` is a format string
*/
info: pino.LogFn;
/**
* Log at `'debug'` level the given msg. If the first argument is an object, all its properties will be included in the JSON line.
* If more args follows `msg`, these will be used to format `msg` using `util.format`.
*
* @typeParam T: the interface of the object being serialized. Default is object.
* @param obj: object to be serialized
* @param msg: the log message to write
* @param ...args: format string values when `msg` is a format string
*/
debug: pino.LogFn;
/**
* Log at `'trace'` level the given msg. If the first argument is an object, all its properties will be included in the JSON line.
* If more args follows `msg`, these will be used to format `msg` using `util.format`.
*
* @typeParam T: the interface of the object being serialized. Default is object.
* @param obj: object to be serialized
* @param msg: the log message to write
* @param ...args: format string values when `msg` is a format string
*/
trace: pino.LogFn;
/**
* Noop function.
*/
silent: pino.LogFn;
}

type Bindings = Record<string, any>;

type Level = "fatal" | "error" | "warn" | "info" | "debug" | "trace";
type LevelWithSilent = pino.Level | "silent";

type SerializerFn = (value: any) => any;
type WriteFn = (o: object) => void;

type LevelChangeEventListener = (
lvl: LevelWithSilent | string,
Expand Down Expand Up @@ -278,6 +279,8 @@ declare namespace pino {
(msg: string, ...args: any[]): void;
}

interface PrettyOptions extends PinoPrettyOptions {}

interface LoggerOptions {
transport?: TransportSingleOptions | TransportMultiOptions | TransportPipelineOptions
/**
Expand Down Expand Up @@ -756,10 +759,13 @@ export type LevelChangeEventListener = pino.LevelChangeEventListener;
export type LogDescriptor = pino.LogDescriptor;
export type Logger = pino.Logger;
export type SerializedError = pino.SerializedError;
export type SerializerFn = pino.SerializerFn;
export type SerializedRequest = pino.SerializedRequest;
export type SerializedResponse = pino.SerializedResponse;
export type WriteFn = pino.WriteFn;

// Interfaces
export interface BaseLogger extends pino.BaseLogger {}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@kibertoad I think here is what you mention?

export interface ChildLoggerOptions extends pino.ChildLoggerOptions {}
export interface DestinationStream extends pino.DestinationStream {}
export interface LevelMapping extends pino.LevelMapping {}
Expand All @@ -768,6 +774,7 @@ export interface LogFn extends pino.LogFn {}
export interface LoggerOptions extends pino.LoggerOptions {}
export interface MultiStreamOptions extends pino.MultiStreamOptions {}
export interface MultiStreamRes extends pino.MultiStreamRes {}
export interface PrettyOptions extends pino.PrettyOptions {}
export interface StreamEntry extends pino.StreamEntry {}
export interface TransportBaseOptions extends pino.TransportBaseOptions {}
export interface TransportMultiOptions extends pino.TransportMultiOptions {}
Expand Down
19 changes: 19 additions & 0 deletions test/types/pino.test-d.ts
View file
Open in desktop
Expand Up @@ -277,3 +277,22 @@ const logLine: pino.LogDescriptor = {
interface CustomLogger extends pino.Logger {
customMethod(msg: string, ...args: unknown[]): void;
}

const serializerFunc: pino.SerializerFn = () => {}
const writeFunc: pino.WriteFn = () => {}

interface CustomBaseLogger extends pino.BaseLogger {
child(): CustomBaseLogger
}

const customBaseLogger: CustomBaseLogger = {
level: 'info',
fatal() {},
error() {},
warn() {},
info() {},
debug() {},
trace() {},
silent() {},
child() { return this }
}