-
Notifications
You must be signed in to change notification settings - Fork 878
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
chore: use node: imports #2003
chore: use node: imports #2003
Conversation
I'm not clear what this offers. Will it result in issues from people bundling things? |
There have been quite some discussions on the nodejs repo when the feature was introduced. Recently added modules don't have unprefixed versions available, like If you are worried about deno and bun support (don't know if pino runs as is currently?) they handle the Disclaimer: I am fully aware all of these reasons are opinionated and none are hard hitting. I do however feel obliged to suggest whatever nodejs proper/core brings out to help move the ecosystem forward. |
Yes, all of that is true. I am asking what benefit it provides this code base. Any usage of modules that require the prefix will, of course, utilize the prefix. We know that people run this dependency, |
Ah, sorry I misunderstood. I will do some research. |
In Bun, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
Just a data point: This indirectly breaks fastify@4 with Node.js v14 versions before 14.18. |
Nice. I suspected we'd see issues. We'll revert. |
Please don't feel obliged just for me. I am able to cope in my usages. I just have some repos that try to maintain 14.17 as a min-supported Node. In my case I just needed to limit the scope of testing. My runtime deps haven't been broken. |
I also forgot that this is in the v9 release line and we do not support v14 with Line 46 in 8aafa88
|
So, perhaps it is an issue on fastify@4 that it took a PR to bump its pino dep from 8 to 9? This PR (which was merged ... without any review?) fastify/fastify#5431 https://github.com/fastify/fastify/blob/4.x/docs/Reference/LTS.md#schedule |
😖 I have feelings about Dependabot. |
(available since node 14.18)