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

chore: update mongoose #370

Merged
merged 2 commits into from
Mar 14, 2023
Merged

chore: update mongoose #370

merged 2 commits into from
Mar 14, 2023

Conversation

backflip
Copy link
Contributor

This PR updates Mongoose to v7 where callback support has been dropped. It should fix #369 with minimal changes:

  • Replacing Document.save(cb) with Document.save().then(document => cb(null, document)).catch(cb)
  • Replacing Query.exec(cb) with Query.exec().then(document => cb(null, document)).catch(cb)

Is this a reasonable approach or would you want to remove callback support from the plugin methods, too?

@saintedlama
Copy link
Owner

I think this approach is feasible and then in a next step remove callback support and use async/await consistently. I updated the main branch to exclude node 12 from supported versions and adding node 18 to the test matrix. Could you merge main into this branch and check if the tests are green?

@backflip
Copy link
Contributor Author

@saintedlama, I have rebased on main. However, I'm not sure how to trigger a test run. https://github.com/saintedlama/passport-local-mongoose/actions/runs/4406499156 says Waiting for pending jobs. FWIW, I was able to run the tests locally with Node 14.21.3, 16.19.1 and 18.15.0.

@saintedlama
Copy link
Owner

Hey, I need to approve the action run for the PR. Did it right now...

@coveralls
Copy link

Coverage Status

Coverage: ?%. Remained the same when pulling 4a7f2f0 on backflip:main into 3ef9e74 on saintedlama:main.

@coveralls
Copy link

Coverage Status

Coverage: 97.727%. Remained the same when pulling 4a7f2f0 on backflip:main into 3ef9e74 on saintedlama:main.

@saintedlama
Copy link
Owner

saintedlama commented Mar 13, 2023

@saintedlama, I have rebased on main. However, I'm not sure how to trigger a test run. https://github.com/saintedlama/passport-local-mongoose/actions/runs/4406499156 says Waiting for pending jobs. FWIW, I was able to run the tests locally with Node 14.21.3, 16.19.1 and 18.15.0.

Ready to release? I'll make it a major update because of mongoose 7

@backflip
Copy link
Contributor Author

Ready to release? I'll make it a major update because of mongoose 7

Sounds good to me!

@saintedlama saintedlama merged commit a397c00 into saintedlama:main Mar 14, 2023
@saintedlama
Copy link
Owner

Thanks for the PR! New version released to npm 🎉

@kubawasikowski
Copy link

Thank You All for the update

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants