jochen-schweizer / express-prom-bundle

express middleware with standard prometheus metrics in one bundle
MIT License
303 stars 67 forks source link

@types/express should be marked as a peerDependency #123

Open TimHaboldt opened 1 year ago

TimHaboldt commented 1 year ago

What is the current behavior?

@types/express is marked as a development dependency.

 "devDependencies": {
    "@types/express": "^4.16.1",
    "coveralls": "^3.0.2",
    ...
  },

What is the expected behavior?

@types/express should be marked as a peerDependency.

  "peerDependencies": {
    "prom-client": ">=12.0.0",
    "@types/express": "^4.16.1"
  },

Why is this an issue?

express-prom-bundle is reexporting the types of express:

import { Request, RequestHandler, Response, Express } from 'express';

declare namespace express_prom_bundle {
  ...
  interface Opts {
    ...
    metricsApp?: Express;
    ...
  }
  ...
}

not all types of express are compatible with each other. e.g. (4.17.13 and 4.17.14)

Using PNPM in the context of a MonoRepository can lead to other services using a higher/lower version of the express library which can lead to express-prom-bundle using a different version of the express types than installed locally for the current workspace. This leads to a type missmatch.

Codesnipped that can leads to a reproduction:

import express from "express";
import prometheusMiddleware from "express-prom-bundle";
import { collectDefaultMetrics, Registry } from "prom-client";

const metricsServer = express();

const register = new Registry();
collectDefaultMetrics({ register });

const middleware = prometheusMiddleware({
    autoregister: false,
    buckets: [120],
    includePath: true,
    metricsApp: metricsServer,
    metricsPath: "/",
    promRegistry: register,
});

In this example prometheusMiddleware resolves the type of the key metricsApp as express internally. The types referenced via import express from "express"; is referencing another type version. Typescript will throw a type missmatch in this case.

That is why in our opinion the production dependency @types/express should be a peerDependency instead.