Pnpm and Pino Transports

[moshie]

Hi,

I have created a private package called @zopauk/nextjs-logger the idea of this package is nextjs applications can install it and have access to the logger however, the global configuration (transports and such) remain under the package it's self (to keep applications aligned with logging config).

The issue we have with this is that when installing @zopauk/nextjs-logger with pnpm on to the nextjs application the app errors with the following:

Error: Cannot find module 'pino-pretty'
Require stack:
- /Users/david.hewitt/code/loans-dashboard/.next/server/pages/_document.js
    at Function.Module._resolveFilename (node:internal/modules/cjs/loader:933:15)
    at Function.mod._resolveFilename (/Users/david.hewitt/code/loans-dashboard/node_modules/.pnpm/next@12.0.8_dfc0b93d99fba272c9564d3139a2581a/node_modules/next/dist/build/webpack/require-hook.js:179:28)
    at Function.resolve (node:internal/modules/cjs/helpers:108:19)
    at fixTarget (/Users/david.hewitt/code/loans-dashboard/node_modules/.pnpm/pino@7.6.3/node_modules/pino/lib/transport.js:129:30)
    at transport (/Users/david.hewitt/code/loans-dashboard/node_modules/.pnpm/pino@7.6.3/node_modules/pino/lib/transport.js:112:22)
    at normalizeArgs (/Users/david.hewitt/code/loans-dashboard/node_modules/.pnpm/pino@7.6.3/node_modules/pino/lib/tools.js:412:16)
    at pino (/Users/david.hewitt/code/loans-dashboard/node_modules/.pnpm/pino@7.6.3/node_modules/pino/pino.js:84:28)
    at PinoLogger (/Users/david.hewitt/code/loans-dashboard/node_modules/.pnpm/@zopauk+nextjs-logger@1.1.6_next@12.0.8/node_modules/@zopauk/nextjs-logger/dist/nextjs-logger.cjs.development.js:885:13)
    at eval (webpack-internal:///./src/utils/logger.ts:9:99)
    at Object../src/utils/logger.ts (/Users/david.hewitt/code/loans-dashboard/.next/server/pages/_document.js:220:1) {
  code: 'MODULE_NOT_FOUND',
  requireStack: [
    '/Users/david.hewitt/code/loans-dashboard/.next/server/pages/_document.js'
  ]
}

When installing the same package with npm it works fine presumably because npm hoists it's packages to the top level.

I could install pino-pretty on the nextjs application but it defeats the purpose if I want to introduce other transports in the future.

I noticed under the /node_modules/@zopauk/nextjs-logger/node_modules/folder that it's missing the dependencies for pino-pretty (I can see it has the bin file but not the code.)

Looking under the node_modules folder pnpm structures it's packages differently choosing to place the module here: /node_modules/.pnpm/@zopauk+nextjs-logger@1.1.6_next@12.0.8/node_modules/pino-pretty

My question is how come pino is unable to find the pino-pretty module when installing packages using pnpm?

I hope that this is enough information let me know if you need anything further.

[mcollina]

My question is how come pino is unable to find the pino-pretty module when installing packages using pnpm?

There is some difference layout on disk compared to pino and https://github.com/pinojs/pino/blob/master/lib/caller.js is actually not working as we thought it should.

Would you like to improve this?

[moshie]

I am not super familiar with the internals of this package but I am happy to give it a shot 👍

[moshie]

@mcollina Looks like the culprit is:

// caller.js
for (const entry of entries) {
    const file = entry ? entry.getFileName() : undefined

    if (isOutsideNodeModules(file)) {
      return file
    }
  }

When it loops through these Error entries:

FILE /Users/david.hewitt/code/loans-dashboard/node_modules/.pnpm/@zopauk+nextjs-logger@1.1.6_next@12.0.8/node_modules/@zopauk/nextjs-logger/dist/nextjs-logger.cjs.development.js
FILE undefined
FILE /Users/david.hewitt/code/loans-dashboard/.next/server/pages/_document.js

It's hitting the /Users/david.hewitt/code/loans-dashboard/.next/server/pages/_document.js one and returning it as a result of the isOutsideNodeModules check which isn't right.

It should be returning the first one.

How would you like me to approach this?

commenting out that for loop fixes it for me but I am guessing it's in there for a reason :)

[mcollina]

I think we should return an array instead and then try multiple callers until we find one that works when we resolve the dep later on.

[github-actions[bot]]

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.