fix: Suppress node 23 warnings about require-ing ESM MONGOSH-1895 #488
Conversation
|
|
||
| warningsSuppressed = true; | ||
| } | ||
| } |
There was a problem hiding this comment.
If this is the approach we're going with – which I think is pretty reasonable – then I'd do that in the mongosh CLI itself, like we already disable warnings for the case of the compiled executable in packages/cli-repl/src/run.ts, because that's where we're expecting overrides for Node.js behavior and other modifications to truly global state, how does that sound to you?
There was a problem hiding this comment.
Sure, that sounds reasonable to me! I guess originally I wanted to put it next to the code that does the requiring shenanigans, but you're right that the warnings being displayed or not is better left to the user-facing application.
There was a problem hiding this comment.
Yeah, I think there's also just going to be more code running into this problem in the future, the more popular ESM gets in our dependencies, not just the code here
|
Closing in favor of mongodb-js/mongosh#2258. |
Description
Starting with node 23, support for require-ing ES modules no longer throws but instead prints a warning in the output. This is problematic for homebrew users since homebrew aggressively upgrades node, regardless of what the preferred node version is for a package. This PR takes one approach, which is to suppress experimental warnings altogether for Node 23. This is only manually tested since I'm not convinced it's the approach we'd like to take (see Open Questions) - once we've settled on it, I'll need to figure out a way to test.
Open Questions
This feels a bit hacky and I'm not convinced this is the long term fix we'd like to use. Here are a few alternatives, ranked from easy to hard:
moduleResolutiontoNodeNextand fix the issues that this causes (unfortunately, I wan't able to trace what they were).Checklist