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

Allow importing Mongoose without any MongoDB specifics, take 2 #13905

Merged
merged 5 commits into from Oct 4, 2023

Conversation

vkarpov15
Copy link
Collaborator

Summary

#13902, but with cleaner git history.

We've been using this branch to make Mongoose playground work: https://github.com/mongoosejs/mongoose-playground/blob/b6032ccbbf11908ae45cfc8eb28077a5559bfbdf/package.json#L13 because otherwise there's no way to import fully fledged Mongoose in the browser.

This change is more a refactor than anything, but it is sufficiently significant that I wanted to hold off until Mongoose 8.

Examples

Copy link
Collaborator

@hasezoey hasezoey left a comment

Choose a reason for hiding this comment

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

LGTM, a lot cleaner git history, though it seems like comparing HEAD to 8.0 still does not recognize the rename and making it more annoying to compare (but going commit by commit works)

just to confirm, the changes in lib/query.js are to make things consistent or was there a specific problem for this change? (the _collection.collection -> mongooseCollection change)

PS: should the old PR branch be deleted once this is merged?

@vkarpov15
Copy link
Collaborator Author

"just to confirm, the changes in lib/query.js are to make things consistent or was there a specific problem for this change? (the _collection.collection -> mongooseCollection change)" I don't remember the exact reasoning. The difference is that this._collection is an instance of mquery.Collection, whereas this.mongooseCollection is an instance of Mongoose's collection class. So I suspect there was some quirk with mquery collections that I didn't want to work around. Either way, I think using mongooseCollection is a maintainability improvement as we try to move away from mquery.

@vkarpov15 vkarpov15 merged commit 65aef58 into 8.0 Oct 4, 2023
34 checks passed
@hasezoey hasezoey deleted the vkarpov15/gh-4292-2 branch October 5, 2023 10:22
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants