Add stricter compilation options and types

Author: Michal Vlasák

package.json

@@ -4,24 +4,33 @@
   "description": "Ackee Node Logger",
   "main": "./dist/index.js",
   "scripts": {
-    "build": "tsc",
+    "build": "tsc --strict --noImplicitReturns --noUnusedLocals --noUnusedParameters",
     "lint-staged": "lint-staged",
     "test": "jest",
     "test-watch": "jest --watch",
-    "lint": "tslint -p . --force --format verbose; exit 0",
+    "lint": "tslint -p . --force --format codeFrame; exit 0",
     "pretty": "prettier --write 'src/*.ts'",
     "precommit": "lint-staged && npm test",
     "check": "npm-check -i components -i config -i controllers -i enums -i errors -i middleware -i routes -i repositories -i services -i husky & exit 0"
   },
   "lint-staged": {
-    "*.js": ["eslint", "git add"]
+    "*.js": [
+      "eslint"
+    ]
   },
   "author": "Michal Vlasák <michal.vlasak@ackee.cz>",
   "license": "ISC",
   "engines": {
     "node": "6.14.3"
   },
   "dependencies": {
+    "@types/express": "^4.16.0",
+    "@types/lodash.isobject": "^3.0.4",
+    "@types/lodash.isstring": "^4.0.4",
+    "@types/on-finished": "^2.3.1",
+    "@types/on-headers": "^1.0.0",
+    "@types/pino": "^5.6.1",
+    "@types/pino-multi-stream": "^3.1.1",
     "lodash.difference": "^4.5.0",
     "lodash.foreach": "^4.5.0",
     "lodash.isempty": "^4.4.0",

src/declarations.ts

@@ -0,0 +1,32 @@
+declare module 'omit-deep' {
+    function omitDeep(value: object | object[], keys: string | string[]): object;
+    export = omitDeep;
+}
+
+declare module 'pick-deep' {
+    function pickDeep(obj: object, paths: string | string[], separator?: string): object;
+    export = pickDeep;
+}
+
+// there is a @typed/pino-multi-stream package, but it has wrong type in its Streams definition. So until its fixed, we use this
+declare module 'pino-multi-stream' {
+    import {
+        LevelWithSilent as PinoLevel,
+        Logger as PinoLogger,
+        LoggerOptions as PinoLoggerOptions,
+        stdSerializers as pinoStdSerializers,
+    } from 'pino';
+    import stream = require('stream');
+
+    type Streams = Array<{ stream: NodeJS.WritableStream; level?: Level }>;
+    interface LoggerOptions extends PinoLoggerOptions {
+        streams?: Streams;
+    }
+    const stdSerializers: typeof pinoStdSerializers;
+
+    function multistream(streams: Streams): stream.Writable;
+    type Level = PinoLevel;
+    type Logger = PinoLogger;
+
+    function pinoms(options: LoggerOptions): Logger;
+}

src/express.ts

@@ -1,21 +1,37 @@
+import { ErrorRequestHandler, Request, RequestHandler, Response } from 'express';
 import * as onFinished from 'on-finished';
-import * as onHeaders from 'on-headers';
+import onHeaders = require('on-headers');
+import { AckeeLogger } from '.';
 
-const expressMiddleware = function(req, response, next) {
+const errorSymbol = Symbol.for('error');
+
+export type AckeeLoggerExpressMiddleware = (
+    this: AckeeLogger,
+    req: Request & { _startAt?: [number, number]; ackId?: string },
+    response: Response & { _startAt?: [number, number]; time?: string; out?: object; [errorSymbol]?: any },
+    next: any
+) => void;
+
+const expressMiddleware: RequestHandler = function(
+    this: AckeeLogger,
+    req: Request & { _startAt?: [number, number]; ackId?: string },
+    response: Response & { _startAt?: [number, number]; time?: string; out?: object; [errorSymbol]?: any },
+    next: any
+) {
     const reqIn = `--- ${req.method} ${req.originalUrl} ${req.headers['user-agent']}`;
     this.debug({ req, ackId: req.ackId }, `${reqIn} - Request accepted`);
     req._startAt = process.hrtime();
     onHeaders(response, () => {
         response._startAt = process.hrtime();
-        const diffFromSeconds = (response._startAt[0] - req._startAt[0]) * 1e3;
-        const diffFromNanoseconds = (response._startAt[1] - req._startAt[1]) * 1e-6;
+        const diffFromSeconds = (response._startAt[0] - req._startAt![0]) * 1e3;
+        const diffFromNanoseconds = (response._startAt[1] - req._startAt![1]) * 1e-6;
         const ms = diffFromSeconds + diffFromNanoseconds;
         response.time = ms.toFixed(3);
     });
-    onFinished(response, (err, res) => {
-        const error = res[Symbol.for('error')];
+    onFinished(response, (_err, res) => {
+        const error = res[errorSymbol];
         const reqOut = `${res.statusCode} ${req.method} ${req.originalUrl} ${res.time} ms ${req.headers['user-agent']}`;
-        if (this.options.ignoredHttpMethods.includes(req.method)) {
+        if (this.options.ignoredHttpMethods && this.options.ignoredHttpMethods.includes(req.method)) {
             return;
         }
         if (error) {
@@ -29,9 +45,10 @@ const expressMiddleware = function(req, response, next) {
     next();
 };
 
-const expressErrorMiddleware = (error, req, res, next) => {
-    res[Symbol.for('error')] = error;
+const expressErrorMiddleware: ErrorRequestHandler = (error, _req, res, next) => {
+    (res as any)[Symbol.for('error')] = error;
     next(error);
 };
 
 export { expressErrorMiddleware, expressMiddleware };
+

src/index.ts

@@ -1,27 +1,35 @@
-import * as isObject from 'lodash.isobject';
-import * as isString from 'lodash.isstring';
+import { ErrorRequestHandler } from 'express';
+import isObject = require('lodash.isobject');
+import isString = require('lodash.isstring');
 import * as pino from 'pino';
-import { multistream } from 'pino-multi-stream';
-import { expressErrorMiddleware, expressMiddleware } from './express';
-import { levels } from './levels';
+import { BaseLogger as PinoLogger } from 'pino';
+import { Level, LoggerOptions, multistream } from 'pino-multi-stream';
+import { AckeeLoggerExpressMiddleware, expressErrorMiddleware, expressMiddleware } from './express';
 import * as serializers from './serializers';
 import { StackDriverFormatStream } from './stackdriver';
-import { decorateStreams, DefaultTransformStream } from './streams';
-
-interface LoggerOptions {
-    disableFields: string[];
-    enableFields: string[];
-    defaultLevel: string;
-    disableStackdriverFormat: boolean;
-    streams: any[];
-    ignoredHttpMethods: string[];
-    config: any;
-    pretty: boolean;
+import { AckeeLoggerStream, decorateStreams, DefaultTransformStream } from './streams';
+
+export interface AckeeLoggerOptions {
+    disableFields?: string[];
+    enableFields?: string[];
+    defaultLevel?: Level;
+    disableStackdriverFormat?: boolean;
+    streams?: AckeeLoggerStream[];
+    ignoredHttpMethods?: string[];
+    config?: LoggerOptions;
+    pretty?: boolean;
+}
+
+export interface AckeeLogger extends PinoLogger {
+    warning: pino.LogFn;
+    options: AckeeLoggerOptions;
+    express: AckeeLoggerExpressMiddleware;
+    expressError: ErrorRequestHandler;
 }
 
 // This is a custom slightly edited version of pino-multistream's wirte method, whch adds support for maximum log level
 // The original version was pino-multistream 3.1.2 (commit 71d98ae) - https://github.com/pinojs/pino-multi-stream/blob/71d98ae191e02c56e39e849d2c30d59c8c6db1b9/multistream.js#L43
-const maxLevelWrite = function(data) {
+const maxLevelWrite: pino.WriteFn = function(this: any, data: object): void {
     let stream;
     const needsMetadata = Symbol.for('needsMetadata');
     const level = this.lastLevel;
@@ -44,7 +52,7 @@ const maxLevelWrite = function(data) {
     }
 };
 
-const defaultLogger = (options: LoggerOptions = ({} as any) as LoggerOptions) => {
+const defaultLogger = (options: AckeeLoggerOptions = {}): AckeeLogger => {
     const pretty = pino.pretty();
     pretty.pipe(process.stdout);
     const prettyErr = pino.pretty();
@@ -54,7 +62,7 @@ const defaultLogger = (options: LoggerOptions = ({} as any) as LoggerOptions) =>
     serializers.enablePaths(options.enableFields);
 
     const isTesting = process.env.NODE_ENV === 'test';
-    let defaultLevel = 'debug';
+    let defaultLevel: Level = 'debug';
 
     if (isTesting) {
         defaultLevel = 'silent';
@@ -64,20 +72,17 @@ const defaultLogger = (options: LoggerOptions = ({} as any) as LoggerOptions) =>
         defaultLevel = options.defaultLevel;
     }
 
-    let streams;
+    let streams: AckeeLoggerStream[];
     let defaultMessageKey = 'message'; // best option for Google Stackdriver
     if (options.streams) {
         streams = options.streams;
     } else if (options.pretty) {
-        streams = [
-            { level: defaultLevel, stream: pretty, maxLevel: levels.warn },
-            { level: levels.warn, stream: prettyErr },
-        ];
+        streams = [{ level: defaultLevel, maxLevel: 'warn', stream: pretty }, { level: 'warn', stream: prettyErr }];
         defaultMessageKey = 'msg'; // default pino - best option for pretty print
     } else {
         streams = [
-            { level: defaultLevel, stream: process.stdout, maxLevel: levels.warn },
-            { level: levels.warn, stream: process.stderr },
+            { level: defaultLevel, maxLevel: 'warn', stream: process.stdout },
+            { level: 'warn', stream: process.stderr },
         ];
     }
     if (!options.disableStackdriverFormat) {
@@ -106,28 +111,27 @@ const defaultLogger = (options: LoggerOptions = ({} as any) as LoggerOptions) =>
         multistream(streams)
     );
     logger.warning = logger.warn;
-    logger.options = options;
+    (logger as any).options = options;
 
     // Add maxLevel support to pino-multi-stream
     // This could be replaced with custom pass-through stream being passed to multistream, which would filter the messages
-    logger.stream.write = maxLevelWrite.bind(logger.stream);
-
+    (logger as any).stream.write = maxLevelWrite.bind(logger.stream);
     logger.express = expressMiddleware.bind(logger);
-    logger.expressError = expressErrorMiddleware;
+    logger.expressError = expressErrorMiddleware as any;
 
-    return logger;
+    return (logger as any) as AckeeLogger;
 };
 
-let rootLogger;
+let rootLogger: AckeeLogger;
 
-const loggerFactory = (data = {}) => {
-    let moduleName;
-    let options;
+const loggerFactory = (data: string | AckeeLoggerOptions = {}): AckeeLogger => {
+    let moduleName: string | undefined;
+    let options: AckeeLoggerOptions = {};
     if (data) {
         if (isString(data)) {
-            moduleName = data;
+            moduleName = data as string;
         } else if (isObject(data)) {
-            options = data;
+            options = data as AckeeLoggerOptions;
         } else {
             throw new TypeError(`Invalid argument of type ${typeof data}`);
         }
@@ -139,11 +143,11 @@ const loggerFactory = (data = {}) => {
     if (!moduleName) {
         return rootLogger;
     }
-    return rootLogger.child({ name: moduleName });
+    return (rootLogger.child({ name: moduleName }) as any) as AckeeLogger;
 };
 
 const factoryProxy = new Proxy(loggerFactory, {
-    get: (target, key) => target()[key],
+    get: (target, key) => (target() as any)[key],
 });
 
 export default factoryProxy;

src/serializers.ts

@@ -1,18 +1,21 @@
-import * as forEach from 'lodash.foreach';
-import * as omit from 'omit-deep';
-import * as pick from 'pick-deep';
+import { Dictionary } from 'lodash';
+import forEach = require('lodash.foreach');
+import omit = require('omit-deep');
+import pick = require('pick-deep');
 import { removeEmpty } from './utils';
 
-const serializers = {
-    error(obj) {
+type SerializerFn = (obj: Dictionary<any>) => Dictionary<any>;
+
+const serializers: Dictionary<SerializerFn> = {
+    error(obj: Dictionary<any>): Dictionary<any> {
         return {
             code: obj.code,
             data: obj.data,
             message: obj.message,
             stack: obj.stack,
         };
     },
-    process(obj) {
+    process(obj: Dictionary<any>): Dictionary<any> {
         if (!obj.env) {
             return obj;
         }
@@ -23,7 +26,7 @@ const serializers = {
         omit(rest, 'env');
         return removeEmpty(Object.assign({}, filteredEnv, rest));
     },
-    req(obj) {
+    req(obj: Dictionary<any>): Dictionary<any> {
         const pickHeaders = ['x-deviceid', 'authorization', 'user-agent'];
         const [body, query] = ['body', 'query'].map(name => {
             const source = obj[name];
@@ -43,51 +46,53 @@ const serializers = {
             url: obj.originalUrl || obj.url,
         });
     },
-    res(obj) {
+    res(obj: Dictionary<any>): Dictionary<any> {
         return {
             out: obj.out,
             time: obj.time,
         };
     },
 };
 
-const disablePaths = paths => {
+const disablePaths = (paths: string[] | undefined) => {
     if (!paths || paths.length <= 0) {
         return;
     }
     forEach(serializers, (value, key) => {
         const matcher = new RegExp(`^${key}.(.*)`);
-        const affectedFields = [];
+        const affectedFields: string[] = [];
         paths.forEach(field => {
-            field.replace(matcher, (match, p1) => {
-                affectedFields.push(p1);
-            });
+            const res = field.match(matcher);
+            if (res !== null) {
+                affectedFields.push(res[1]);
+            }
         });
 
         if (affectedFields.length > 0) {
-            const newSerializer = obj => {
+            const newSerializer: SerializerFn = (obj: Dictionary<any>) => {
                 return omit(value(obj), affectedFields);
             };
             serializers[key] = newSerializer;
         }
     });
 };
 
-const enablePaths = paths => {
+const enablePaths = (paths: string[] | undefined) => {
     if (!paths || paths.length <= 0) {
         return;
     }
     forEach(serializers, (value, key) => {
         const matcher = new RegExp(`^${key}.(.*)`);
-        const affectedFields = [];
+        const affectedFields: string[] = [];
         paths.forEach(field => {
-            field.replace(matcher, (match, p1) => {
-                affectedFields.push(p1);
-            });
+            const res = field.match(matcher);
+            if (res !== null) {
+                affectedFields.push(res[1]);
+            }
         });
 
         if (affectedFields.length > 0) {
-            const newSerializer = obj => {
+            const newSerializer: SerializerFn = (obj: Dictionary<any>) => {
                 const newFields = pick(obj, affectedFields);
                 const originalResult = value(obj);
                 return Object.assign({}, originalResult, newFields);

src/stackdriver.ts

@@ -1,6 +1,6 @@
 import { Transform } from 'stream';
 
-const PINO_TO_STACKDRIVER = {
+const PINO_TO_STACKDRIVER: { [key: number]: string } = {
     10: 'DEBUG',
     20: 'DEBUG',
     30: 'INFO',
@@ -10,7 +10,7 @@ const PINO_TO_STACKDRIVER = {
 };
 
 class StackDriverFormatStream extends Transform {
-    public _transform(chunk, encoding, callback) {
+    public _transform(chunk: any, _encoding: string, callback: (error?: Error | undefined, data?: any) => void) {
         const obj = JSON.parse(chunk);
         obj.severity = PINO_TO_STACKDRIVER[obj.level] || 'UNKNOWN';