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.

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

Using transport with file with Next.js #81

Open
GodDrinkTeJAVA opened this issue Jan 9, 2023 · 14 comments
Open

Using transport with file with Next.js #81

GodDrinkTeJAVA opened this issue Jan 9, 2023 · 14 comments

Comments

@GodDrinkTeJAVA
Copy link

While importing PinoWebpackPlugin in Next.config.js, next.js always shows the message for not having CommonJsRequireDependency.

When I push PinoWebpackPlugin into array of plugins by this clause where currDir is simply a variable for __dirname

config.plugins.push(new PinoWebpackPlugin({transports: [`${currDir}/src/modules/logger/transport.mjs`]}));

The error for dependency always occurs, regardless of my TS-baed project, or new JS-based project created by create-next-app. next build always fail, and dev-server throws error for it.

The message below is shown by error page:

Failed to compile
./node_modules/pino/browser.js
No template for dependency: CommonJsRequireDependency

The message below is shown by terminal running server:

No template for dependency: CommonJsRequireDependency
{"level":30,"time":1673234383826,"pid":23900,"hostname":"<MyDevice>.local","msg":"info logger"} # this is the log I've created
Could not find files for / in .next/build-manifest.json
Could not find files for / in .next/build-manifest.json
<w> [webpack.cache.PackFileCacheStrategy] Skipped not serializable cache item 'Compilation/modules|javascript/dynamic|/Users/user/workspace/project/test/next-test/node_modules/pino/browser.js': No serializer registered for CommonJsRequireDependency
<w> while serializing webpack/lib/cache/PackFileCacheStrategy.PackContentItems -> webpack/lib/NormalModule -> Array { 3 items } -> CommonJsRequireDependency

I've searched for the CommonJsRequireDependency error cases and haven't found any clue for it. If further information is needed, let me know by leaving a comment for that.

@mcollina
Copy link
Member

mcollina commented Jan 9, 2023

@ShogunPanda wdyt?

@ShogunPanda
Copy link
Collaborator

I need to look into it.

@GodDrinkTeJAVA Can you please provide me a reproducible repository or a list of commands I can execute to reproduce this?

@GodDrinkTeJAVA
Copy link
Author

GodDrinkTeJAVA commented Jan 10, 2023

https://github.com/GodDrinkTeJAVA/pino-webpack-test

This is a simple example project made from create-next-app. In modules/logger.ts, Pino constructor actually has no transport property, and only adding PinoWebpackPlugin to plugin arrays from next.config.js causes the error. dev-server shows error information page when accessed, and next build fails with same error.

If any of my implementation is broken, I would appreciate a lot for letting me notified.

@ShogunPanda
Copy link
Collaborator

I tried cloning your repo and then run next build or next dev and never got any issue.
Have you tried deleting node_modules files and any lockfile, reinstall from scratch and build with Next again?

@GodDrinkTeJAVA
Copy link
Author

I've checked out the commit and you were right. I'll check more conditions and re-comment when the problem is correctly found.
Oh.. and.. could you tell me which directory should be given to transport.target when target is js file? I supposed it be moved to the root so it would be like '__dirname/{filename}' no matter where the file is. Would this be right?

@GodDrinkTeJAVA GodDrinkTeJAVA changed the title Compatibility Issue with Next.js Using transport with file with Next.js Jan 11, 2023
@ShogunPanda
Copy link
Collaborator

I'm not sure I understand the question. Can you please clarify?

@GodDrinkTeJAVA
Copy link
Author

Sorry if it confuses you. What I just tried to tell was to check out which way of directory should be given as transport.target property in pino() function call.

pino({
    transport: {
        target: PATH_TO_TRANSPORT
    }
})

README.md says each file specified in plugin constructor's transports option would be generated at the root of the dist folder. I thought file with pino() function call would be generated in top level, too. Sorry for confusing comment.

@ShogunPanda
Copy link
Collaborator

@mcollina I think you have more knowledge in this. Can you help?

@GodDrinkTeJAVA
Copy link
Author

https://github.com/GodDrinkTeJAVA/pino-webpack-test

Also, I found the way to reproduce the error. Now main branch shows error when npm build.
Commenting out lines which push PinoWebpackPlugin will fix the error, but when included back, it gives 'No template for dependency: CommonJsRequireDependency' error again.

@ristomatti
Copy link

Sorry about the somewhat off topic comment but I fail to understand what benefit using this would bring to a Next.js app? We're currently using Pino 7 with Next.js like any other library and everything seems to work as expected.

@mcollina
Copy link
Member

Sorry about the somewhat off topic comment but I fail to understand what benefit using this would bring to a Next.js app? We're currently using Pino 7 with Next.js like any other library and everything seems to work as expected.

This plugin allows you to use transports in a bundled server component. Not something that I would recommend, but it seems people are really interested in doing it.

@Xevion
Copy link

Xevion commented Sep 4, 2023

Has anyone here found a way forward for using Next.js with Pino transports? I've searched everywhere and come to no conclusion. #109 is the exact same issue as me and has received no attention from the PinoJS community in months.

What do Next.js developers do, as there doesn't seem to be any other great logging libraries out there...

@mcollina
Copy link
Member

mcollina commented Sep 4, 2023

#109 is the exact same issue as me and has received no attention from the PinoJS community in months.

Pull Requests are welcomed, especially for bugs like this.
Unfortunately it does not help that none of us are using Next.js :(.

@groozin
Copy link

groozin commented Oct 8, 2023

Hopefully the below helps in some following work on this, as I could not fix this problem.

  1. When next compiles it runs webpack with 2 targets: web and node. For node the pino module is pulled but for web it substitutes it with ./browser.js (as indicated in the pinos package.json in the browser prop). This means that if we include logger.info("asdf") as part of the client code the webpack will try and pull in the ./browser.js. And this somehow conflicts with this plugin.

  2. I am not 100% sure what we are trying to achieve here, by using the same logger configuration on both client and server, but there are some bullet points worth mentioning:

  • pino itself exposes browser compatible api that can be used to log on the client - this however means we need to use it differently than on the server see https://github.com/pinojs/pino/blob/master/docs/browser.md

  • not all transport plugins will work on the client - as some of them might use only node side modules

  • even though we include transports in the plugin configuration - we still need to configure the logger with them to work, so something like this might be necessary. Also the __dirname will be resolved to the path in the .next build folder - I think

import pino from "pino";

const logger = pino({
  transport: {
    target: `${__dirname}/modules/logger/transport.mjs`,
  },
});

export default logger;

Hope this helps for further investigation.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants