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

feat: re-export default Mongoose instance properties for ESM named imports support #12256

Merged
merged 3 commits into from
Aug 25, 2022

Conversation

vkarpov15
Copy link
Collaborator

Fix #12148

Summary

I did some reading in https://github.com/nodejs/node/pull/35249/files and https://simonplend.com/node-js-now-supports-named-imports-from-commonjs-modules-but-what-does-that-mean/, it looks like all we need to do to support ESM named imports in Node is to explicitly re-export all the properties on the Mongoose global.

This does come with some caveats. Most notably that this will no longer be an instance of Mongoose in all exported functions by default. This shouldn't break anything - we added support for destructured imports in a73c2ab. But it is still a risk.

I'm also not exporting Connection and Collection, because those are custom getters and overwriting those can break support for custom drivers.

Examples

@vkarpov15 vkarpov15 added this to the 6.6 milestone Aug 11, 2022
index.js Outdated Show resolved Hide resolved
Copy link
Collaborator

@Uzlopak Uzlopak left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

Copy link
Collaborator

@AbdelrahmanHafez AbdelrahmanHafez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One minor comment, otherwise LGTM. Thanks!

index.js Outdated
module.exports.deleteModel = mongoose.deleteModel;
module.exports.modelNames = mongoose.modelNames;
module.exports.plugin = mongoose.plugin;
// module.exports.connection = mongoose.connection;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not export the default connection, is it also because of "custom driver may overwrite this"?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes exactly. setDriver() can recreate the default connection.

@vkarpov15 vkarpov15 changed the base branch from master to 6.6 August 23, 2022 19:01
@vkarpov15 vkarpov15 merged commit df2ffb7 into 6.6 Aug 25, 2022
@vkarpov15 vkarpov15 removed this from the 6.6 milestone Aug 25, 2022
@hasezoey hasezoey added this to the 6.6 milestone Sep 1, 2022
@hasezoey hasezoey deleted the vkarpov15/gh-12148 branch September 1, 2022 11:33
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

Successfully merging this pull request may close these issues.

add esm support
5 participants