middlewares dependencies

[monwolf]

Hi
@jake-kramer with the inclusion of #173
, you are effectively forcing all Pyroscope users to include both Fastify and Express in the same project. This introduces a set of unnecessary extra dependencies.

In my original approach, I implemented a conditional load, exporting only the objects required when the corresponding dependencies are present, avoiding this issue altogether.

await (async () => {
  try {
    const expressMiddleware = (await import('./middleware/express.js')).default;
    BaseImport.expressMiddleware = expressMiddleware;
  } catch (error) {
    console.debug('Error loading express middleware:', error);
  }

  try {
    const fastifyMiddleware = (await import('./middleware/fastify.js')).default;
    BaseImport.fastifyMiddleware = fastifyMiddleware;
  } catch (error) {
    console.debug('Error loading fastify middleware:', error);
  }
})();

With the current approach, we can't use the library without providing express:

node_modules/@pyroscope/nodejs/dist/esm/middleware/express.d.ts:1:49 - error TS2307: Cannot find module 'express' or its corresponding type declarations.

1 import { NextFunction, Request, Response } from 'express';
                                                  ~~~~~~~~~


Found 1 error.

npm notice
npm notice New minor version of npm available! 11.6.1 -> 11.7.0
npm notice Changelog: https://github.com/npm/cli/releases/tag/v11.7.0
npm notice To update run: npm install -g npm@11.7.0
npm notice

I think there are two possible approaches to address this:

• Extract the middlewares from the main repository and require users to install them ad hoc.
• Export the middlewares only when the required dependencies are satisfied. (this would be the best one for maintainability)

[jake-kramer]

Hi @monwolf, the initial approach copied above has a race condition in loading the dependency which is why it wasn't incorporated. I tried many approaches to resolve the race condition but was not able to -- let me know if you have an alternate approach.

I have a strong preference to keep the API the same, so changing the import path of the middleware is not ideal.