-
-
Notifications
You must be signed in to change notification settings - Fork 3.9k
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(mongoose): add setDriver()
function to allow overwriting driver
in a more consistent way
#11900
Conversation
…er` in a more consistent way
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.
Can you think of a way we can mimic having different drivers and setting them to verify that this works?
Would deep cloning the driver, modify some arbitrary property, then setting it, and asserting that get
ting the driver, we'll have the arbitrary property be enough testing?
Otherwise, LGTM.
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.
I dont know how to test this. Maybe creating a mock with a Proxy around the actual mongodb driver, and then checking if the Proxy is set after using setDriver. And when we are successful we set back the original driver.
|
||
const openConnection = _mongoose.connections && _mongoose.connections.find(conn => conn.readyState !== STATES.disconnected); | ||
if (openConnection) { | ||
const msg = 'Cannot modify Mongoose driver if a connection is already open. ' + |
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.
nitpick: why do we need a one time variable? We could directly pass it to MongooseError
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.
Better readability IMO. Easier to break up the long message if there's a temporary variable rather than putting it as a param.
Co-authored-by: Uzlopak <[email protected]>
Co-authored-by: Uzlopak <[email protected]>
…se into vkarpov15/driver-fixes
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
Summary
Right now overwriting the Mongoose driver is a bit messy, because 1)
Connection
andCollection
aren't changed when you domongoose.driver.set()
, and 2) driver is always global, so no way to have 2 Mongoose instances with the same driver in the same process.Examples
You can now do
mongoose.setDriver()
without any extra work to use a different driver.